(Follows on from http://bugs.webkit.org/show_bug.cgi?id=16748) // DOM Range function () { // test 11: basic ranges tests var r = document.createRange(); assert(r, "range not created"); assert(r.collapsed, "new range wasn't collapsed"); assert(r.commonAncestorContainer == document, "new range's common ancestor wasn't the document"); assert(r.startContainer == document, "new range's start container wasn't the document"); assert(r.startOffset == 0, "new range's start offset wasn't zero"); assert(r.endContainer == document, "new range's end container wasn't the document"); assert(r.endOffset == 0, "new range's end offset wasn't zero"); assert(r.cloneContents(), "cloneContents() didn't return an object"); assert(r.cloneContents().childNodes.length == 0, "nothing cloned was more than nothing"); assert(r.cloneRange().toString() == "", "nothing cloned stringifed to more than nothing"); r.collapse(true); // no effect assert(r.compareBoundaryPoints(r.START_TO_END, r.cloneRange()) == 0, "starting boundary point of range wasn't the same as the end boundary point of the clone range"); r.deleteContents(); // no effect assert(r.extractContents().childNodes.length == 0, "nothing removed was more than nothing"); r.insertNode(document.createComment("commented inserted to test ranges")); assert(!r.collapsed, "range with inserted comment isn't collapsed"); assert(r.commonAncestorContainer == document, "range with inserted comment has common ancestor that isn't the document"); assert(r.startContainer == document, "range with inserted comment has start container that isn't the document"); assert(r.startOffset == 0, "range with inserted comment has start offset that isn't zero"); assert(r.endContainer == document, "range with inserted comment has end container that isn't the document"); assert(r.endOffset == 1, "range with inserted comment has end offset that isn't after the comment"); return 1; }, Fails at: assert(!r.collapsed, "range with inserted comment isn't collapsed");
Created attachment 18299 [details] Proposed patch Move the endOffset as required when new items are inserted.
I'm not sure that this is right approach - ranges should be fixed for any DOM mutations, not just those that happen to be made via DOMRange interface. See <http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Mutation>.
Comment on attachment 18299 [details] Proposed patch Usually a count would be named with a noun. The word "move" is more like a verb, so "offsetMove" is not a great variable name. Maybe offsetDelta or endOffsetAdjustment? + if (m_startContainer == m_endContainer) + m_endOffset += offsetMove; There's no if statement needed here -- you already ensured that the variable will be 0 in the case where the two are not equal. r=me
(In reply to comment #2) > I'm not sure that this is right approach - ranges should be fixed for any DOM > mutations, not just those that happen to be made via DOMRange interface. See > <http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Mutation>. I agree with Alexey. This approach is incorrect. However, I see no major harm in having this code which fixes only one particular case checked in, in the mean time. We can do the real fix later and remove this one special case -- the test case here will ensure we don't break this particular case.
Comment on attachment 18299 [details] Proposed patch Landed in r29214 I agree with Alexy on this, so I'll leave this bug open and clear the review flag on this patch.
Calling this one fixed as I made a more general bug for this at http://bugs.webkit.org/show_bug.cgi?id=16766
Mass moving XML DOM bugs to the "DOM" Component.