Bug 514300 part 2. Make nsBindingManager::ContentRemoved remove the node from all the places it needs to be removed from. Also fixes bug 415301 and bug 495354. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 13 Jan 2010 11:30:11 -0500
changeset 37148 710cca46f64d71c4c9d8263e7889f32438548531
parent 37147 7963a18d337355815d4b6e2c1e8a40728621da3f
child 37149 a4c65dcecdf67c73129d25ca027504421900afad
push id11143
push userbzbarsky@mozilla.com
push dateWed, 13 Jan 2010 16:30:40 +0000
treeherdermozilla-central@a4c65dcecdf6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs514300, 415301, 495354
milestone1.9.3a1pre
Bug 514300 part 2. Make nsBindingManager::ContentRemoved remove the node from all the places it needs to be removed from. Also fixes bug 415301 and bug 495354. r=sicking
content/xbl/crashtests/415301-1.xul
content/xbl/crashtests/495354-1.xhtml
content/xbl/crashtests/crashtests.list
content/xbl/src/nsBindingManager.cpp
layout/reftests/bugs/495354-1-ref.xhtml
layout/reftests/bugs/495354-1a.xhtml
layout/reftests/bugs/495354-1b.xhtml
new file mode 100644
--- /dev/null
+++ b/content/xbl/crashtests/415301-1.xul
@@ -0,0 +1,34 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/global.css"?>
+
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        onload="boom();">
+
+
+<bindings xmlns="http://www.mozilla.org/xbl">
+
+<binding id="chil"><content><children/></content></binding>
+
+<binding id="ichil"><content>
+<xul:hbox style="-moz-binding: url(#chil)"><children/></xul:hbox>
+</content></binding>
+
+</bindings>
+
+
+<script type="text/javascript">
+
+function boom()
+{
+  document.getElementById("inner").removeChild(document.getElementById("lbb"));
+  document.getElementById("outer").style.MozBinding = "";
+}
+
+</script>
+
+
+<hbox id="outer" style="-moz-binding: url(#chil)"><hbox id="inner" style="-moz-binding: url(#ichil)"><listboxbody id="lbb" /></hbox></hbox>
+
+
+</window>
new file mode 100644
--- /dev/null
+++ b/content/xbl/crashtests/495354-1.xhtml
@@ -0,0 +1,8 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<bindings xmlns="http://www.mozilla.org/xbl"><binding id="x"><content><caption xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"><children xmlns="http://www.mozilla.org/xbl"/></caption></content></binding></bindings>
+</head>
+<body onload="document.getElementById('x').innerHTML = '9';">
+<span id="x" style="-moz-binding: url(#x)"><div></div></span>
+</body>
+</html>
--- a/content/xbl/crashtests/crashtests.list
+++ b/content/xbl/crashtests/crashtests.list
@@ -13,17 +13,19 @@ load 368641-1.xhtml
 load 378521-1.xhtml
 load 382376-1.xhtml
 load 382376-2.xhtml
 load 397596-1.xhtml
 load 404125-1.xhtml
 load 406900-1.xul
 load 406904-1.xhtml
 load 406904-2.xhtml
+load 415301-1.xul
 load 418133-1.xhtml
 load 415192-1.xul
 load 420233-1.xhtml
 load 421997-1.xhtml
 load 432813-1.xhtml
 load 460665-1.xhtml
 load 464863-1.xhtml
 load 472260-1.xhtml
 load 492978-1.xul
+load 495354-1.xhtml
--- a/content/xbl/src/nsBindingManager.cpp
+++ b/content/xbl/src/nsBindingManager.cpp
@@ -1531,16 +1531,21 @@ RemoveChildFromInsertionPoint(nsAnonymou
                               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.
+
+  // XXXbz nsXBLInsertionPoint::RemoveChild could return whether it
+  // removed something.  Wouldn't that let us short-circuit the walk?
+  // Or can a child be in multiple insertion points?  I wouldn't think
+  // so...
   PRInt32 count = aInsertionPointList->GetInsertionPointCount();
   for (PRInt32 i = 0; i < count; i++) {
     nsXBLInsertionPoint* point =
       aInsertionPointList->GetInsertionPointAt(i);
     if ((point->GetInsertionIndex() == -1) == aRemoveFromPseudoPoints) {
       point->RemoveChild(aChild);
     }
   }
@@ -1566,16 +1571,31 @@ nsBindingManager::ContentRemoved(nsIDocu
         // Find a non-pseudo-insertion point and remove ourselves.
         RemoveChildFromInsertionPoint(static_cast<nsAnonymousContentList*>
                                         (static_cast<nsIDOMNodeList*>
                                                     (nodeList)),
                                       aChild,
                                       PR_FALSE);
         SetInsertionParent(aChild, nsnull);
       }
+
+      // Also remove from the list in mContentListTable, if any.
+      if (mContentListTable.ops) {
+        nsCOMPtr<nsIDOMNodeList> otherNodeList =
+          static_cast<nsAnonymousContentList*>
+                     (LookupObject(mContentListTable, point));
+        if (otherNodeList && otherNodeList != nodeList) {
+          // otherNodeList is always anonymous
+          RemoveChildFromInsertionPoint(static_cast<nsAnonymousContentList*>
+                                        (static_cast<nsIDOMNodeList*>
+                                                    (otherNodeList)),
+                                        aChild,
+                                        PR_FALSE);
+        }
+      }
     }
 
     // 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<nsAnonymousContentList*>(LookupObject(mContentListTable,
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/495354-1-ref.xhtml
@@ -0,0 +1,5 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<body>
+<span><caption xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">9</caption></span>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/495354-1a.xhtml
@@ -0,0 +1,8 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<bindings xmlns="http://www.mozilla.org/xbl"><binding id="x"><content><caption xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"><children xmlns="http://www.mozilla.org/xbl"/></caption></content></binding></bindings>
+</head>
+<body onload="document.getElementById('x').innerHTML = '9';">
+<span id="x" style="-moz-binding: url(#x)"><div>123</div></span>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/495354-1b.xhtml
@@ -0,0 +1,8 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<bindings xmlns="http://www.mozilla.org/xbl"><binding id="x"><content><caption xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"><children xmlns="http://www.mozilla.org/xbl"/></caption></content></binding></bindings>
+</head>
+<body onload="document.getElementById('x').innerHTML = '9';">
+<span id="x" style="-moz-binding: url(#x)"><div style="border: 5px solid green">123</div></span>
+</body>
+</html>