DOM insertion mutation events should dispatch after a node is attached to the render tree

• Component: DOM
• Status: RESOLVED FIXED

[Antoine Quint]

The insertion mutation events, such as DOMNodeInserted and DOMNodeInsertedIntoDocument, are dispatched before ->attach() is called on the inserted node. This causes computed style to be meaningless in handlers for such events and differs from the Firefox and Opera implementations that provide such useful information.

It's worth noting that DOMSubtreeModified is dispatched _after_ the element was attached to the render tree.

[Antoine Quint]

Created attachment 49819 [details]
Testcase showing the width value of computedStyle for an appended element after various mutation events are dispatched

[Antoine Quint]

rdar://problem/7707492

[Antoine Quint]

Created attachment 49910 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 49910 [details]
Patch

The code changes look good. But I think you should test insertBefore and replaceChild, too, since you modified those.

[Antoine Quint]

Created attachment 49912 [details]
Patch

[Darin Adler]

Comment on attachment 49912 [details]
Patch

Moving the childrenChanged call after the attach call might lead to bugs like the one fixed in <http://trac.webkit.org/changeset/55462> because all the code run inside attach will be done while functions like HTMLSelectElement::childrenChanged have not yet been called. For example, the select element's list of items will still be wrong. And Element::childrenChanged won't run, which has something to do with updating style sharing data structures.

The test cases check only the mutation event aspect of this change, so if we had not moved the childrenChanged function calls, the test would still pass. The test doesn't show us possible problems created or bugs fixed by moving the childrenChanged calls.

[Antoine Quint]

I've moved only the dispatchChildInsertionEvents() calls after the render tree attachments, and left the childrenChanged() calls where they were which still fixes the original issue, but breaks a fair few layout tests, specifically for app cache.

By the looks of it dispatchChildInsertionEvents() actually does a bit more than dispatching just the insertion DOM events and some of its calls probably alter the process here. My guess is that dispatchChildInsertionEvents() should split off the actual event dispatching code to be on its own so that part only can be moved post render tree attachment.

[Antoine Quint]

Created attachment 50029 [details]
Patch

[Antoine Quint]

So I split off the internal-to-WebCore notification bits from
dispatchChildInsertionEvents() into a new static function
notifyChildInserted(), and now dispatchChildInsertionEvents() is called _after_
attachment of the node to the render tree.

[WebKit Commit Bot]

Comment on attachment 50029 [details]
Patch

Clearing flags on attachment: 50029

Committed r55532: <http://trac.webkit.org/changeset/55532>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.