From b21e31e94dbac345e37191a287f60a462e2a2fd3 Mon Sep 17 00:00:00 2001 From: "bzbarsky@mit.edu" Date: Sun, 27 Jan 2008 21:31:20 -0800 Subject: [PATCH] Fix bug 342954 by making sure to update our insertion point data when removing nodes from the DOM. r=smaug, sr=sicking --- content/xbl/src/nsBindingManager.cpp | 60 +++++++++++++++++++++------- content/xbl/src/nsBindingManager.h | 9 ++++- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/content/xbl/src/nsBindingManager.cpp b/content/xbl/src/nsBindingManager.cpp index a23a4c869962..cadf843133e8 100644 --- a/content/xbl/src/nsBindingManager.cpp +++ b/content/xbl/src/nsBindingManager.cpp @@ -1384,8 +1384,7 @@ nsBindingManager::ContentAppended(nsIDocument* aDocument, &isAnonymousContentList); if (nodeList && isAnonymousContentList) { - // Find a non-pseudo-insertion point and just jam ourselves in. - // This is not 100% correct. Hack city, baby. + // Find the one non-pseudo-insertion point and just add ourselves. nsAnonymousContentList* contentList = static_cast(nodeList.get()); @@ -1424,13 +1423,33 @@ nsBindingManager::ContentInserted(nsIDocument* aDocument, } } +static void +RemoveChildFromInsertionPoint(nsAnonymousContentList* aInsertionPointList, + nsIContent* aChild, + PRBool aRemoveFromPseudoPoints) +{ + // We need to find the insertion point that contains aChild and remove it + // from that insertion point. Sadly, we don't know which point it is, or + // when we've hit it, but just trying to remove from all the pseudo or + // non-pseudo insertion points, depending on the value of + // aRemoveFromPseudoPoints, should work. + PRInt32 count = aInsertionPointList->GetInsertionPointCount(); + for (PRInt32 i = 0; i < count; i++) { + nsXBLInsertionPoint* point = + aInsertionPointList->GetInsertionPointAt(i); + if ((point->GetInsertionIndex() == -1) == aRemoveFromPseudoPoints) { + point->RemoveChild(aChild); + } + } +} + void nsBindingManager::ContentRemoved(nsIDocument* aDocument, nsIContent* aContainer, nsIContent* aChild, PRInt32 aIndexInContainer) { - if (aIndexInContainer != -1 && + if (aContainer && aIndexInContainer != -1 && (mContentListTable.ops || mAnonymousNodesTable.ops)) { // It's not anonymous nsCOMPtr point = GetNestedInsertionPoint(aContainer, aChild); @@ -1443,17 +1462,26 @@ nsBindingManager::ContentRemoved(nsIDocument* aDocument, if (nodeList && isAnonymousContentList) { // Find a non-pseudo-insertion point and remove ourselves. - nsAnonymousContentList* contentList = static_cast(static_cast(nodeList)); - PRInt32 count = contentList->GetInsertionPointCount(); - for (PRInt32 i =0; i < count; i++) { - nsXBLInsertionPoint* point = contentList->GetInsertionPointAt(i); - if (point->GetInsertionIndex() != -1) { - point->RemoveChild(aChild); - } - } + RemoveChildFromInsertionPoint(static_cast + (static_cast + (nodeList)), + aChild, + PR_FALSE); SetInsertionParent(aChild, nsnull); } - } + } + + // Whether the child has a nested insertion point or not, aContainer might + // have insertion points under it. If that's the case, we need to remove + // aChild from the pseudo insertion point it's in. + if (mContentListTable.ops) { + nsAnonymousContentList* insertionPointList = + static_cast(LookupObject(mContentListTable, + aContainer)); + if (insertionPointList) { + RemoveChildFromInsertionPoint(insertionPointList, aChild, PR_TRUE); + } + } } } @@ -1534,7 +1562,7 @@ nsBindingManager::HandleChildInsertion(nsIContent* aContainer, { NS_PRECONDITION(aChild, "Must have child"); NS_PRECONDITION(!aContainer || - aContainer->IndexOf(aChild) == aIndexInContainer, + PRUint32(aContainer->IndexOf(aChild)) == aIndexInContainer, "Child not at the right index?"); nsIContent* ins = GetNestedInsertionPoint(aContainer, aChild); @@ -1546,8 +1574,10 @@ nsBindingManager::HandleChildInsertion(nsIContent* aContainer, &isAnonymousContentList); if (nodeList && isAnonymousContentList) { - // Find a non-pseudo-insertion point and just jam ourselves in. - // This is not 100% correct. Hack city, baby. + // Find a non-pseudo-insertion point and just jam ourselves in. This is + // not 100% correct, since there might be multiple insertion points under + // this insertion parent, and we should really be using the one that + // matches our content... Hack city, baby. nsAnonymousContentList* contentList = static_cast(nodeList.get()); diff --git a/content/xbl/src/nsBindingManager.h b/content/xbl/src/nsBindingManager.h index 689ba3de10f5..8651f0e42f37 100755 --- a/content/xbl/src/nsBindingManager.h +++ b/content/xbl/src/nsBindingManager.h @@ -241,7 +241,9 @@ protected: // A mapping from nsIContent* to an nsIDOMNodeList* // (nsAnonymousContentList*). This list contains an accurate // reflection of our *explicit* children (once intermingled with - // insertion points) in the altered DOM. + // insertion points) in the altered DOM. There is an entry for a + // content node in this table only if that content node has some + // kids. PLDHashTable mContentListTable; // A mapping from nsIContent* to an nsIDOMNodeList* @@ -250,7 +252,10 @@ protected: // intermingled with insertion points) in the altered DOM. This // table is not used if no insertion points were defined directly // underneath a tag in a binding. The NodeList from the - // is used instead as a performance optimization. + // is used instead as a performance optimization. There + // is an entry for a content node in this table only if that content + // node has a binding with a attached and this + // contains elements directly. PLDHashTable mAnonymousNodesTable; // A mapping from nsIContent* to nsIContent*. The insertion parent