cloneNode() doesn't work for DocumentFragment

VERIFIED FIXED in mozilla1.0

Status

()

Core
DOM: Core & HTML
VERIFIED FIXED
17 years ago
4 years ago

People

(Reporter: david, Assigned: jst)

Tracking

Trunk
mozilla1.0
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [HAVE FIX])

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
The cloneNode() method does not copy the children of a DocumentFragment.

You can demonstrate this as follows:

    var fragment = document.createDocumentFragment();
    fragment.appendChild(document.createTextNode("This is "));
    var only = fragment.appendChild(document.createElement("I"));
    only.appendChild(document.createTextNode("only"));
    fragment.appendChild(document.createTextNode(" a test"));

    alert(fragment.childNodes.length);   // Displays 3
    var f = fragment.cloneNode(true);
    alert(f.childNodes.length);          // Displays 0

Comment 1

17 years ago
Created attachment 31775 [details]
Testcase based on David Flanagan's code

Comment 2

17 years ago
Confirmed via the attached testcase. Marking NEW (not a dup as far as I can tell).
Seeing on win2k as well -> OS:ALL
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
(Assignee)

Updated

17 years ago
Target Milestone: --- → mozilla1.0

Comment 3

17 years ago
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala

Comment 4

17 years ago
I confirmed that this is broken on Mozilla 0.9.3 for MacOS 9.1.  

I tested using the Netscape DOM Level 2 Conformance test "tc_hdoc501m Test
Document Modifying Properties".  This test is available by clicking on the link
for DOM HTML Level 2 Conformance on this page:

http://developer.netscape.com/evangelism/tools/testsuites/

Comment 5

16 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 6

16 years ago
Created attachment 67580 [details] [diff] [review]
Proposed fix.

Patch has been tested and fixes the attached testcase. It follows the same
logic as nsXMLDocument::CloneNode(). When reviewing, please make sure I don't
leak anything (it shouldn't, but you never know)

Comment 7

16 years ago
bz or peterv, would you review please? thanks in advance
Whiteboard: [HAVE FIX]
Target Milestone: mozilla1.0.1 → mozilla1.0
Comment on attachment 67580 [details] [diff] [review]
Proposed fix.

>+  NS_ENSURE_ARG_POINTER(aReturn);

Don't bother with that.

>+      for (index = 0; index < count; index++) {

use ++index, not index++

>+  CallQueryInterface(newFragment, aReturn);
>   return NS_OK;

return CallQueryInterface(newFragment, aReturn);

Otherwise QI failures will do odd things... :)

With those, r=bzbarsky

No need to attach a new patch for those nits.
Attachment #67580 - Flags: review+
(Assignee)

Comment 9

16 years ago
Comment on attachment 67580 [details] [diff] [review]
Proposed fix.

sr=jst
Attachment #67580 - Flags: superreview+
(Assignee)

Comment 10

16 years ago
Nits fixed and patch checked in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 11

16 years ago
verified fixed. 2002-03-07-10
Status: RESOLVED → VERIFIED

Updated

10 years ago
Component: DOM: Core → DOM: Core & HTML
QA Contact: stummala → general
You need to log in before you can comment on or make changes to this bug.