Bug 16748 - DOMRange.cloneContents does not work (Acid3 bug)
Summary: DOMRange.cloneContents does not work (Acid3 bug)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-05 14:12 PST by Eric Seidel (no email)
Modified: 2008-01-06 03:30 PST (History)
1 user (show)

See Also:


Attachments
Proposed patch (4.63 KB, patch)
2008-01-06 01:13 PST, Andrew Wellington
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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