Bug 207875

Summary: Crash when Node::normalize() triggers mutation event that modifies child order
Product: WebKit Reporter: Sunny He <sunny_he>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ajuma, bfulgham, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, rniwa, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Sunny He 2020-02-17 18:30:33 PST 
rdar://58976682
Comment 1 Sunny He 2020-02-17 18:32:42 PST 
Created attachment 391017 [details]
Patch
Comment 2 Sunny He 2020-02-18 15:15:22 PST 
Created attachment 391101 [details]
Patch
Comment 3 Sunny He 2020-02-18 15:24:49 PST 
After playing around with normalize() and DOMSubtreeModified event, I'm not so sure about that FIXME. If I log in the eventhandler, I see Chrome also calls the event handler multiple times if multiple text nodes were merged. Am I reading the DOM spec correctly (https://dom.spec.whatwg.org/#dom-node-normalize)?
Comment 4 Ryosuke Niwa 2020-02-18 18:15:33 PST 
Comment on attachment 391101 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391101&action=review

> Source/WebCore/dom/Node.cpp:674
> +            // Update start/end for any affected Ranges

I don’t think this comment is necessary since the code says that.
If anything, we should explain why we need call this before appendData instead.

> LayoutTests/ChangeLog:17
> +        * fast/dom/Node/normalize_mutation_event.html: Added.

Please use - instead of _ in file names
Comment 5 Sunny He 2020-02-19 14:35:14 PST 
Created attachment 391201 [details]
Patch
Comment 6 Ryosuke Niwa 2020-02-19 16:19:03 PST 
Comment on attachment 391201 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391201&action=review

> Source/WebCore/ChangeLog:5
> +        rdar://58976682

Please wrap radar URL in < & >.

> Source/WebCore/dom/Node.cpp:680
> +            

Nit: whitespace.
Comment 7 Sunny He 2020-02-19 16:45:43 PST 
Created attachment 391221 [details]
Patch
Comment 8 WebKit Commit Bot 2020-02-19 19:36:58 PST 
Comment on attachment 391221 [details]
Patch

Clearing flags on attachment: 391221

Committed r257036: <https://trac.webkit.org/changeset/257036>
Comment 9 WebKit Commit Bot 2020-02-19 19:37:01 PST 
All reviewed patches have been landed.  Closing bug.
Comment 10 Ryosuke Niwa 2020-03-04 19:44:31 PST 
*** Bug 208314 has been marked as a duplicate of this bug. ***