Bug 16764 – DOMRange doesn't correctly re-size when inserting items (Acid3)

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Andrew Wellington]

(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");

[Andrew Wellington]

Created attachment 18299 [details]
Proposed patch

Move the endOffset as required when new items are inserted.

[Alexey Proskuryakov]

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>.

[Darin Adler]

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

[Darin Adler]

(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.

[Andrew Wellington]

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.

[Andrew Wellington]

Calling this one fixed as I made a more general bug for this at http://bugs.webkit.org/show_bug.cgi?id=16766

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.