1931301 - Re-enable nsINode::IsRootElement assertion.

Status: NEW

(Core :: DOM: Core & HTML, task)

Description

[Emilio Cobos Álvarez (:emilio)]

In bug 1914321 I added an nsINode::IsRootElement helper which is faster than comparing to GetRootElement(). However that catches an interesting case... We notify of ContentRemoved before UnbindFromTree(), but after disconnecting the child node from the document.

We want IsRootElement() to return true there (to return all the anonymous content we're destroying), but the assert fired because we didn't use to do that.

We should either remove that assertion, or ideally fix the notification so that it matches and re-enable it.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1931301 - Notify of removals on a stable state, not mid-remove. r=smaug,peterv,#dom-core

This simplifies some observers, and makes others a bit more subtle, but I think
over-all it is an improvement.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1931301 - Re-enable assert. r=smaug,peterv,#dom-core

This should generally now pass (except during UnbindFromTree).

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Masayuki, with the patch above I get some failures in widget/tests/test_composition_text_querycontent.xhtml, like:

  FAIL runIMEContentObserverTest with contenteditable (defaultParagraphSeparator is br): deleting child nodes (between same level descendants) with pressing Delete key should cause text change notification whose addedLength is 4 - got 5, expected 4
SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:509:14
is@chrome://mochitests/content/chrome/widget/tests/window_composition_text_querycontent.xhtml:60:34
checkTextChangeNotification@chrome://mochitests/content/chrome/widget/tests/window_composition_text_querycontent.xhtml:9689:7
testWithHTMLEditor@chrome://mochitests/content/chrome/widget/tests/window_composition_text_querycontent.xhtml:10433:32
async*runIMEContentObserverTest@chrome://mochitests/content/chrome/widget/tests/window_composition_text_querycontent.xhtml:10626:9
async*runTest@chrome://mochitests/content/chrome/widget/tests/window_composition_text_querycontent.xhtml:10990:9
async*SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13

I'm sure I'm doing something dumb.

The TLDR of the change is that there's no longer an special state where the child still points to its parent but the parent doesn't point to the child. That means that some callers need to be adjusted.

IMEContentObserver is rather tricky and I tried to preserve the current behavior, but I'm clearly missing something... Can you spot it off hand? Otherwise I'll dig with rr tomorrow and see where things go differently with my patch.

Thanks!