Bug 16748

Summary: DOMRange.cloneContents does not work (Acid3 bug)
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.4   
Attachments:
Description Flags
Proposed patch darin: review+

Description Eric Seidel (no email) 2008-01-05 14:12:12 PST
DOMRange.cloneContents does not work (Acid3 bug)

    // 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;
    },
Comment 1 Andrew Wellington 2008-01-06 01:13:36 PST
Created attachment 18297 [details]
Proposed patch

The first part of this bug is due to not returning a DocumentFragment for an empty range.

(This won't fix the full Acid3 test, we fail at "range with inserted comment isn't collapsed" after this patch)
Comment 2 Darin Adler 2008-01-06 03:04:05 PST
Comment on attachment 18297 [details]
Proposed patch

There's no need to call fragment.release() if you're not returning the fragment. A plain old "return 0" will do, and it's more efficient.

Otherwise looks great.

I'll say review+, but really those extra fragment.release() calls should not be in there.
Comment 3 Andrew Wellington 2008-01-06 03:30:17 PST
Landed in r29207