1192855 - Range.insertNode/.surroundContents should check whether they will succeed before splitting text nodes

Status: RESOLVED FIXED

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

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Current spec at <https://dom.spec.whatwg.org/#dom-range-insertnode>.  It now says to run validity checks before modifying the DOM.  We sometimes split text nodes before we run all our checks, because we let InsertNode do some of the checks.  After <https://github.com/w3c/web-platform-tests/pull/2063> is accepted, this will be tested by <https://w3c-test.org/dom/ranges/Range-insertNode.html> and <https://w3c-test.org/dom/ranges/Range-surroundContents.html>, which we have imported under testing/web-platform/tests/dom/ranges/.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch

Try is green, except for expected failures in upstream tests that have not yet been updated: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e49abfc0d32f

Comment 2

[Henri Sivonen (:hsivonen)]

Comment on attachment 8646370 [details] [diff] [review]
Patch

It's a bit unclear to me if we can just land this or if we also now need to land something that flips some test expectations.

Comment 3

[Henri Sivonen (:hsivonen)]

Oops. Reading previous comments more carefully now. So, yes, we do need to update the tests before landing. I assume we should just remove the mochitest and update the Web Platform test.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Try with some test changes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=593a1b89bb12

The test changes had a bug that caused some failures, which I've fixed locally.  So I'll land it when inbound reopens.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b0175e11f26

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7b0175e11f26