1211894 - Throw on range.insertNode(range.startContainer)

Status: RESOLVED FIXED

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

Description

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

See discussion at <https://github.com/whatwg/dom/issues/63>.  The spec was just updated <https://github.com/whatwg/dom/commit/abad55d6bd76eda7b765a1eb5f6572e7a8af92dc> to change the behavior here, and a test update upstream is pending <https://github.com/w3c/web-platform-tests/pull/2227>.

We should change to match the spec.  The updated spec matches Chrome, so no compat issues are expected.

Comment 1

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

Attachment: Patch

Try run underway: https://treeherder.mozilla.org/#/jobs?repo=try&revision=349350e88355

Comment 2

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

Try is green except for unexpected passes (fixed locally).

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(need to review the spec change. not yet convinced it is a good change.)

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

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

I disagree with the spec change.

Comment 5

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

I understand you disagree, but I think the exact behavior here is less important than getting to interop.  IE has said they want to change, and WebKit/Blink already behave this way, so if we change we'll converge on common behavior.  If we don't change then we have to persuade IE, WebKit, and Blink to match us, which doesn't seem like it's worth the effort.  So could we please just follow the majority here and match the current spec even if you don't like it as much?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Has Edge or IE changed the behavior? If not, we should push for the better and consistent API and not do this change.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

But if Edge has already changed the behavior, then fine, let's follow others.

Comment 8

[Decky Coss (:deckycoss)]

these tests currently fail in Edge, so apparently the behaviour has not yet changed: http://w3c-test.org/dom/ranges/Range-insertNode.html

example: http://i.imgur.com/d4yu2Bd.png

Comment 9

[(no longer active)]

Olli, given <https://github.com/whatwg/dom/issues/63#issuecomment-238285079>, do you think we should change our behavior here?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

I can live with the change, and  MS hasn't indicated they wouldn't change the behavior at some point.

Comment 11

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

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

New try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e0ca79e78b2636c2c9b90fca2671eff67a8f06f

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c11b9ebe7a2e
Throw on range.insertNode(range.startContainer); r=smaug

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c11b9ebe7a2e