Closed Bug 1192855 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: ayg, Assigned: ayg)

References

Details

Attachments

(1 file)

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/.
Attached patch PatchSplinter Review
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
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Attachment #8646370 - Flags: review?(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.
Attachment #8646370 - Flags: review?(hsivonen) → review+
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.
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.
https://hg.mozilla.org/mozilla-central/rev/7b0175e11f26
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1314549
No longer depends on: 1314549
You need to log in before you can comment on or make changes to this bug.