Bug 5629

Summary:

REGRESSION: appendChild() does not remove nodes from source nodelist when inserting into destination

Product:

WebKit

Reporter:

Daniel Udey <dan>

Component:

DOM

Assignee:

Geoffrey Garen <ggaren>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andito, jon, mitz

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
Reduced testcase for Shipley Beachball bug	none
Dispatch old parent's subtree modified event from appendChild()	harrison: review-
Allow dispatch in more places	harrison: review+

Daniel Udey

Reported 2005-11-04 13:37:23 PST

When the DOM is used to acquire a nodelist (for example, a list of P nodes obtained via getElementByTagName()) and appendChild() is used to append those nodes into another node, they are not removed from the original nodelist (or at least, the nodelist.length property does not reflect this), and any loops that make use of this will loop infinitely.

Attachments
Reduced testcase for Shipley Beachball bug (904 bytes, text/html) 2005-11-04 13:38 PST, Daniel Udey	no flags	Details
Dispatch old parent's subtree modified event from appendChild() (728 bytes, patch) 2005-11-17 14:49 PST, mitz	harrison: review-	Details Formatted Diff Diff
Allow dispatch in more places (12.13 KB, patch) 2005-11-18 13:05 PST, David Harrison	harrison: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Daniel Udey

Comment 1 2005-11-04 13:38:51 PST

Created attachment 4600 [details] Reduced testcase for Shipley Beachball bug Reduced testcase for the bug; beachballs ToT as of November 4th.

mitz

Comment 2 2005-11-12 14:21:38 PST

*** Bug 5718 has been marked as a duplicate of this bug. ***

Geoffrey Garen

Comment 3 2005-11-12 17:39:31 PST

appendChild (w3.org): Adds the node newChild to the end of the list of children of this node. If the newChild is already in the tree, it is first removed.

mitz

Comment 4 2005-11-17 14:32:27 PST

Here's the problem: when the node is removed from the tree, its parent doesn't get the subtreeModifiedEvent from removeChild() since event dispatch is forbidden by appendChild() until it's done. Subsequently, the old parent's nodelists aren't notified of the change. Still no idea how to fix this.

mitz

Comment 5 2005-11-17 14:49:25 PST

Created attachment 4715 [details] Dispatch old parent's subtree modified event from appendChild()

David Harrison

Comment 6 2005-11-17 17:22:07 PST

Comment on attachment 4715 [details] Dispatch old parent's subtree modified event from appendChild() Good catch.

David Harrison

Comment 7 2005-11-18 10:22:28 PST

Comment on attachment 4715 [details] Dispatch old parent's subtree modified event from appendChild() Actually, there is more to it than this. Am coming up with a new patch.

David Harrison

Comment 8 2005-11-18 13:05:28 PST

Created attachment 4726 [details] Allow dispatch in more places Attached patch enables event dispatch when calling removeChild() in loops. That it was disabled previously was wrong because the DOM is not fragile at that point. This makes the event dispatch forbidding a debug-only check (yay).

David Harrison

Comment 9 2005-11-18 13:27:24 PST

Comment on attachment 4726 [details] Allow dispatch in more places r=Darin

David Harrison

Comment 10 2005-11-18 13:38:29 PST

Committed.

Note You need to log in before you can comment on or make changes to this bug.