Problem/Motivation

Not all P items are being process.

This issue stood out with the new filter option and the last image not being processed, but debugging within media_wysiwyg_filter_paragraph_fix() showed that this too had the issued.

Root cause is the modification of the DOMNodeList during the iteration of the list items.

Proposed resolution

Store the DOMNodes during the iteration and remove afterwards.

Remaining tasks

More testing ;)

User interface changes

None

CommentFileSizeAuthor
#6 media-2871960-6-filters-fails-if-update-during-domnodelist-iteration-with-coding-std-fixes.patch8.5 KBAlan D.
#5 media-2871960-4-filters-fails-if-update-during-domnodelist-iteration-with-coding-std-fixes.patch8.89 KBAlan D.
#2 media-2871960-2-filters-fails-if-update-during-domnodelist-iteration.patch7.84 KBAlan D.
#2 media-2871960-2-filters-fails-if-update-during-domnodelist-iteration-test-only.patch5.79 KBAlan D.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Alan D. created an issue. See original summary.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Active » Needs work
FileSize
media-2871960-2-filters-fails-if-update-during-domnodelist-iteration-test-only.patch5.79 KB
media-2871960-2-filters-fails-if-update-during-domnodelist-iteration.patch7.84 KB
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs work » Needs review

The last submitted patch, 2: media-2871960-2-filters-fails-if-update-during-domnodelist-iteration-test-only.patch, failed testing.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
FileSize
media-2871960-4-filters-fails-if-update-during-domnodelist-iteration-with-coding-std-fixes.patch8.89 KB

Sorry, with the coding std issues resolved.

modules/media_wysiwyg/tests/media_wysiwyg.paragraph_fix_filter.test
line 14	Expected 3 space(s) before asterisk; 4 found
15	Expected 3 space(s) before asterisk; 4 found
16	Expected 3 space(s) before asterisk; 4 found
17	Expected 3 space(s) before asterisk; 4 found
18	Line indented incorrectly; expected 2 spaces, found 3
18	Class property $tag_regexp should use lowerCamel naming without underscores
21	Expected 3 space(s) before asterisk; 4 found
22	Expected 3 space(s) before asterisk; 4 found
23	Expected 3 space(s) before asterisk; 4 found
24	Expected 3 space(s) before asterisk; 4 found
25	Line indented incorrectly; expected 2 spaces, found 3
25	Class property $p_wrapped_regexp should use lowerCamel naming without underscores
28	Expected 3 space(s) before asterisk; 4 found
29	Expected 3 space(s) before asterisk; 4 found
30	Expected 3 space(s) before asterisk; 4 found
31	Expected 3 space(s) before asterisk; 4 found
32	Line indented incorrectly; expected 2 spaces, found 3
32	Class property $replace_regexp should use lowerCamel naming without underscores
46	Missing function doc comment
50	If the line declaring an array spans longer than 80 characters, each element should be broken into its own line
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
FileSize
media-2871960-6-filters-fails-if-update-during-domnodelist-iteration-with-coding-std-fixes.patch8.5 KB

duh, the do {} while(); too. Yuk, personally prefer the expanded version...

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

wow @Alan D , nice work!

I will put this in 7.x-3.x dev

  • Alan D. authored 33415a3 on 7.x-3.x 
    Issue #2871960 by Alan D.: Paragraph filter misses some DOM Nodes
joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Status: Needs review » Fixed

fixed in 7.x-3.x-dev

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Version: 7.x-3.x-dev » 7.x-2.x-dev
Status: Fixed » Needs review
Issue tags: +minor version target

setting to needs review for 7.x-2.x-dev

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Assigned: Unassigned » joseph.olstad

  • Alan D. authored 33415a3 on 7.x-2.x 
    Issue #2871960 by Alan D.: Paragraph filter misses some DOM Nodes
joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Status: Needs review » Fixed

This was just merged into 7.x-2.x and is included in release 7.x-2.6

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Issue tags: -minor version target

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.