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

RESOLVED FIXED in Firefox 44

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: ayg, Assigned: ayg)

Tracking

(Depends on: 2 bugs)

unspecified
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(1 attachment)

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/.
Created attachment 8646370 [details] [diff] [review]
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
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.
Depends on: 1194860
Depends on: 1178754
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
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44

Updated

2 years ago
Depends on: 1314549

Updated

2 years ago
No longer depends on: 1314549
You need to log in before you can comment on or make changes to this bug.