[Bug 17541] Carefully think through how Range.insertNode should behave when the node to be inserted is already at the beginning of the range

https://www.w3.org/Bugs/Public/show_bug.cgi?id=17541

Aryeh Gregor <ayg@aryeh.name> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |annevk@annevk.nl,
                   |                            |bzbarsky@mit.edu,
                   |                            |Ms2ger@gmail.com

--- Comment #2 from Aryeh Gregor <ayg@aryeh.name> ---
It doesn't matter where in the text node it falls, no.  Why should it?

What the spec currently requires is this:

Step 3: Split the text node, set referenceNode to the second one ("cd").  The
selection is at the end of the first node ("ab"), which is <var>node</var>.

Step 5: parent is <p>.

Step 6: newOffset is 1.

Step 7: newOffset is 2.  This is wrong -- it's meant to adjust for the fact
that we're about to add a node, but actually we remove a node first.

Step 8: We remove the "ab" node, which causes the range to jump up to (<p>, 0).
 This is not anticipated.  Then we reinsert the node, and the range is still at
(<p>, 0).  This means that the range has jumped from "ab[]cd" to "{}abcd".

Step 9: Set the end of the parent to (<p>, 2), so it's "{abcd}".

I assume that's what Gecko does, without looking at the source.


What's expected behavior in this case?  There are two bugs here, I think:

1) We set the new position of the range (step 7) without noticing if we're
going to remove a node.

2) We split the node we were given, so we only wind up moving half of it.

As I noted, these happen to more or less cancel, but that's accidental.  You
could have a non-text node case where they don't, like:

data:text/html,<!doctype html>
<p><b>foo</b><i>bar</i></p>
<script>
var range = document.createRange();
/* Collapse range at end of <b> */
range.setStart(document.querySelector("b"), 1);
range.insertNode(range.startContainer.firstChild);
document.body.textContent =
range.startContainer.nodeName + " " +
range.startContainer.nodeValue + " " +
range.startOffset + " " +
range.endContainer.nodeName + " " +
range.endContainer.nodeValue + " " +
range.endOffset;
</script>

In this case the spec would say to throw an IndexSizeError, which is what Gecko
in fact does, because the last step of the algorithm tries to set the range
beyond the offset of the parent.  Again, step 7 increases the length to account
for a node that is not in fact going to be removed.


I think the simplest way to fix this is just to remove <var>node</var> from its
parent before doing anything else.  I think that would mean that the example
from comment #0 would result in "{abcd}" with the text node not split, and the
second case I give would result in "<b>{foo}</b><i>bar</i>", both of which
strike me as quite expected.

Opinions?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

Received on Wednesday, 19 February 2014 12:20:00 UTC