Created attachment 547390 [details]
example of multiple selection bug
In case of multiple selection on the same node, calling `range.insertNode(node)` after a `range.deleteContents()`, will collapse all the ranges below the one modified, to the start offset of the modified range.
In addition, calling `range.selectNode(node)`, will corrupting the selection in the page.
Steps to Reproduce:
1. Select text in the same node to have multiple selection, let's say four.
2. Get the range for the 2nd one
3. Call `range.deleteContents()` to delete the current contents, and call `range.insertNode(node)` to add a new node (where `node` is a DOM node)
4. Call `range.selectNode(node)`
The 2nd range will shows the new content, however the 2nd, 3rd and 4th selection are disappeared: the 3rd and the 4th are collapsed to the start offset of the 2nd selection.
In addition, the selection is now corrupted, showing everything selected from the start of the container node to the start of the 2nd selection offset.
The text in the all four selection should be still selected, and the `range` of 2nd, 3rd and 4th selections should be updated with new start / end offset values, in accordance with the new content of 2nd range. The 1st range should be remain the same.
For more details, see the "example of multiple selection bug" attachment for the actual result; and the "example of multiple selection workaround" for the expected result.
Created attachment 547391 [details]
example of multiple selection workaround
(In reply to comment #0)
> Actual Result:
> The 2nd range will shows the new content, however the 2nd, 3rd and 4th
> selection are disappeared: the 3rd and the 4th are collapsed to the start
> offset of the 2nd selection.
This is what DOM 2 Range spec specifies.
> In addition, the selection is now corrupted, showing everything selected
> from the start of the container node to the start of the 2nd selection
This looks indeed like a bug.
(In reply to comment #2)
> > Actual Result:
> > The 2nd range will shows the new content, however the 2nd, 3rd and 4th
> > selection are disappeared: the 3rd and the 4th are collapsed to the start
> > offset of the 2nd selection.
> This is what DOM 2 Range spec specifies.
Could you please give to me a link about that? I was looking to http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html but I wasn't able to find the description of this behavior. Maybe I missed it.
I asked because I'm working on Addon SDK selection module, and we're thinking to have a different behavior in this scenario. Any reference about this specific behavior in DOM 2 Range specs could be useful.
2.12 says that changes to Text nodes can be thought as splitText() and normalize(). So in this case the text node is split and a new one is created
which contains the data after the 2nd range. The other ranges don't automatically
move to that new text node.
Note, there is a new draft for range which seems to change the behavior of
deleteContents, but I'm not sure that the draft is correct yet, or compatible with
Hmm, looking at the code, looks like our implementation should do what the
new draft says... investigating.
Oh, yes, the behavior is correct even based on the new draft.
You call insertNode which ends up splitting the text node, and the ranges
after the split point will have their container pointing to the original
Created attachment 547645 [details]
This seems to work as expected.
The new draft is incompatible with the old spec when handling insertNode
with text nodes.
Investigating some more.
(In reply to comment #5)
> The other ranges don't automatically move to that new text node.
Hmm, why not exactly?
2.12 defines two general principles, one is "all Ranges will select the same portion of the document after any mutation operation", our current behavior
seems to violate this principle.
Well, the question is what is "same portion". Adding a new text node isn't the
same portion as before.
DOM 2 Range doesn't specify this case too well.
The new draft does, but I'm not sure it is correct.
It's not the identical nodes, no, but it's still the "same portion" as I see
it. I think the intent of the second principle is clear and that is that we
should update ranges that refers to text nodes that are removed as a result
of the normalize() step so that they cover the same portion of the text
When the spec describes how to handle mutations to Document, it is somewhat
clear that only changes to offsets are expected, not changes to
I've read 2.12 again, carefully, and I'm even more convinced this is a bug.
The operations in the attached testcase is actually covered by the
examples in the spec. Let's look at each part of replaceRange():
If I comment out the range.insertNode/selectNode parts, the end
result is that the other ranges select the same text they did before
(ie. their start/end offsets were updated)
-- I think we can agree that this is correct per 2.12.2
2. the range is now collapsed at its former start index
-- correct per 2.12.2
we're inserting into a text node at a position that isn't part of
any other range. A range after the insertion position is covered by
example 1 in 2.11.1 -- the example range ("Y blah i") has been adjusted
so that it covers the same text as before the insertion.
I think this makes it clear that this is what should happen for
"genera" and "tall" in our testcase.
Bug 191864 seems to be about the same thing.
Hacking nsGenericDOMDataNode::SplitData to propagate the new text node
in the CharacterDataChangeInfo struct, and adjusting the start/end node
in nsRange::CharacterDataChanged fixes this bug.
So fixing the splitText() case looks relatively easy, at first glance.
Yeah, we could do that (although I do hate special casing range handling).
Seems like Webkit and IE9 do update start/end node, Opera works like Gecko.
I don't understand (In reply to comment #14)
> 3. range.insertNode()
> we're inserting into a text node at a position that isn't part of
> any other range. A range after the insertion position is covered by
> example 1 in 2.11.1 -- the example range ("Y blah i") has been adjusted
> so that it covers the same text as before the insertion.
> I think this makes it clear that this is what should happen for
> "genera" and "tall" in our testcase.
I don't read that example the same way. If the text is inserted to existing
Text node using insertData or similar, then the range after it get updated,
per 2.12.1. But if you split the node and add something between the two nodes, then you have created two new nodes in the document tree.
The patches in bug 191864 fixes this bug.
Mats, the patches in that bug fixes also the `selectNode` behavior said in the description?
Yes. However, part 2 is unlikely to land so there may still be some
painting errors in your testcase without that. Part 1 maintains the
correct ranges though so it should be easy to do some workaround for
the repaint problem. Please try a Nightly build once it lands.
Created attachment 550425 [details]
example of multiple selection with fragment
Good! Thanks Mats, I will try with a Nightly once it lands.
We discussed about text node insertion, I just wonder in case of adding a fragment what is the expected behavior. I added a test case for it.
Probably in the Addon SDK will not follow the specs for the selection module, in this specific case, but it's good to know.
Mats, I tested with the Nightly and it works exactly as you said. The ranges are correctly maintained, but they aren't painted except for the first one.
Any plans to fix the repaint problem as well? You said that the part 2 is unlikely to land, but I hope will manage this issue soon or later.
All the testcases seems to work for me in:
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111025 Firefox/10.0a1
Bug 619273 will fix the remaining repaint problem.
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #22)
> We discussed about text node insertion, I just wonder in case of adding a
> fragment what is the expected behavior. I added a test case for it.
Your document fragment test works as expected. It has the same behavior
as the first test where you inserted a text node - the only reason that
the inserted text is selected in the first example is the explicit
range.selectNode(node) call, which the fragment test doesn't have.
AFAICT, IE9 has compatible behavior (if you workaround the missing
All issues should be fixed by bug 619273 / bug 698237 in the latest Nightly builds:
Please file a new bug and CC me if you still see some error. Thanks for your report!