Bug 116073
| Summary: | Fix wrong assertions in Range::textNodeSplit | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> |
| Component: | DOM | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | Normal | CC: | ahmad.saleem792, ap, bfulgham, rniwa |
| Priority: | P2 | Keywords: | BlinkMergeCandidate |
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Ryosuke Niwa
We should probably merge
https://chromium.googlesource.com/chromium/blink/+/107e79cd424ad62249584a91fc55bded5bcd89ab
Fix wrong assertions in Range::textNodeSplit
Text::splitText calls Node::insertBefore, in which a DOM mutatino event handler
can update the DOM structure. The function calls Document::textNodeSplit too,
and Range::textNodeSplit called by Document::textNodeSplit assumed no one
updated the DOM structure.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Ahmad Saleem
I took the test case from Chromium patch and changed it into JSFiddle:
Link - https://jsfiddle.net/foea2v5g/show
*** Safari Technology Preview 151 ***
No assertion failures even if an DOM mutation event handler updates the new node created by Text::splitText.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
FAIL r.endContainer should be [object Text]. Was [object HTMLDivElement].
FAIL r.endOffset should be 2. Was 1.
PASS successfullyParsed is true
TEST COMPLETE
*** Firefox Nightly 105 ***
No assertion failures even if an DOM mutation event handler updates the new node created by Text::splitText.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
FAIL r.endContainer should be [object Text]. Was [object HTMLDivElement].
FAIL r.endOffset should be 2. Was 1.
PASS successfullyParsed is true
TEST COMPLETE
*** Chrome Canary 106 ***
No assertion failures even if an DOM mutation event handler updates the new node created by Text::splitText.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
FAIL r.endContainer should be [object Text]. Was [object HTMLDivElement].
FAIL r.endOffset should be 2. Was 1.
PASS successfullyParsed is true
TEST COMPLETE
_________
I can find "nextSibling", which Chrome patch removed in Webkit Github source bit differently:
https://github.com/WebKit/WebKit/blob/main/Source/WebCore/dom/Range.cpp#L997
https://github.com/WebKit/WebKit/blob/main/Source/WebCore/dom/Range.cpp#L979
rniwa@webkit.org - Is this need anymore? Thanks!
Alexey Proskuryakov
These assertions failures would only be reproducible in a debug build. Guessing that this isn't relevant any more, but Ryosuke will know for certain.
Ahmad Saleem
It seems this was reverted in following commit in favor of other solution:
https://src.chromium.org/viewvc/blink?view=revision&revision=150483
and other alternative solution:
https://src.chromium.org/viewvc/blink?view=revision&revision=150493
^ I can do PR based on both, which is suggested approach? @rniwa - Thanks!
Ahmad Saleem
*** This bug has been marked as a duplicate of bug 116509 ***