Bug 5629 - REGRESSION: appendChild() does not remove nodes from source nodelist when inserting into destination
Summary: REGRESSION: appendChild() does not remove nodes from source nodelist when ins...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
: 5718 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-11-04 13:37 PST by Daniel Udey
Modified: 2005-12-26 14:08 PST (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Udey 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.
Comment 1 Daniel Udey 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.
Comment 2 mitz 2005-11-12 14:21:38 PST
*** Bug 5718 has been marked as a duplicate of this bug. ***
Comment 3 Geoffrey Garen 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.
Comment 4 mitz 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.
Comment 5 mitz 2005-11-17 14:49:25 PST
Created attachment 4715 [details]
Dispatch old parent's subtree modified event from appendChild()
Comment 6 David Harrison 2005-11-17 17:22:07 PST
Comment on attachment 4715 [details]
Dispatch old parent's subtree modified event from appendChild()

Good catch.
Comment 7 David Harrison 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.
Comment 8 David Harrison 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).
Comment 9 David Harrison 2005-11-18 13:27:24 PST
Comment on attachment 4726 [details]
Allow dispatch in more places

r=Darin
Comment 10 David Harrison 2005-11-18 13:38:29 PST
Committed.