Make sure we call nodeWasAdded_ on all added nodes

googlearchive · Sep 9, 2013 · 36fe41a · 36fe41a
1 parent 941a863
commit 36fe41a
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 8 deletions.
diff --git a/src/wrappers/Node.js b/src/wrappers/Node.js
@@ -26,7 +26,7 @@
    * This updates the internal pointers for node, previousNode and nextNode.
    */
   function collectNodes(node, parentNode, previousNode, nextNode) {
-    if (node.nodeType !== Node.DOCUMENT_FRAGMENT_NODE) {
+    if (!(node instanceof DocumentFragment)) {
       if (node.parentNode)
         node.parentNode.removeChild(node);
       node.parentNode_ = parentNode;
@@ -60,6 +60,24 @@
     return nodes;
   }
 
+  function collectNodesNoNeedToUpdatePointers(node) {
+    if (node instanceof DocumentFragment) {
+      var nodes = [];
+      var i = 0;
+      for (var child = node.firstChild; child; child = child.nextSibling) {
+        nodes[i++] = child;
+      }
+      return nodes;
+    }
+    return [node];
+  }
+
+  function nodesWereAdded(nodes) {
+    for (var i = 0; i < nodes.length; i++) {
+      nodes[i].nodeWasAdded_();
+    }
+  }
+
   function ensureSameOwnerDocument(parent, child) {
     var ownerDoc = parent.nodeType === Node.DOCUMENT_NODE ?
         parent : parent.ownerDocument;
@@ -188,22 +206,25 @@
     appendChild: function(childWrapper) {
       assertIsNodeWrapper(childWrapper);
 
+      var nodes;
+
       if (this.invalidateShadowRenderer() || invalidateParent(childWrapper)) {
         var previousNode = this.lastChild;
         var nextNode = null;
-        var nodes = collectNodes(childWrapper, this, previousNode, nextNode);
+        nodes = collectNodes(childWrapper, this, previousNode, nextNode);
 
         this.lastChild_ = nodes[nodes.length - 1];
         if (!previousNode)
           this.firstChild_ = nodes[0];
 
         originalAppendChild.call(this.impl, unwrapNodesForInsertion(this, nodes));
       } else {
+        nodes = collectNodesNoNeedToUpdatePointers(childWrapper)
         ensureSameOwnerDocument(this, childWrapper);
         originalAppendChild.call(this.impl, unwrap(childWrapper));
       }
 
-      childWrapper.nodeWasAdded_();
+      nodesWereAdded(nodes);
 
       return childWrapper;
     },
@@ -217,10 +238,12 @@
       assertIsNodeWrapper(refWrapper);
       assert(refWrapper.parentNode === this);
 
+      var nodes;
+
       if (this.invalidateShadowRenderer() || invalidateParent(childWrapper)) {
         var previousNode = refWrapper.previousSibling;
         var nextNode = refWrapper;
-        var nodes = collectNodes(childWrapper, this, previousNode, nextNode);
+        nodes = collectNodes(childWrapper, this, previousNode, nextNode);
 
         if (this.firstChild === refWrapper)
           this.firstChild_ = nodes[0];
@@ -238,12 +261,13 @@
           adoptNodesIfNeeded(this, nodes);
         }
       } else {
+        nodes = collectNodesNoNeedToUpdatePointers(childWrapper);
         ensureSameOwnerDocument(this, childWrapper);
         originalInsertBefore.call(this.impl, unwrap(childWrapper),
                                   unwrap(refWrapper));
       }
 
-      childWrapper.nodeWasAdded_();
+      nodesWereAdded(nodes);
 
       return childWrapper;
     },
@@ -300,15 +324,15 @@
       }
 
       var oldChildNode = unwrap(oldChildWrapper);
+      var nodes;
 
       if (this.invalidateShadowRenderer() ||
           invalidateParent(newChildWrapper)) {
         var previousNode = oldChildWrapper.previousSibling;
         var nextNode = oldChildWrapper.nextSibling;
         if (nextNode === newChildWrapper)
           nextNode = newChildWrapper.nextSibling;
-        var nodes = collectNodes(newChildWrapper, this,
-                                 previousNode, nextNode);
+        nodes = collectNodes(newChildWrapper, this, previousNode, nextNode);
 
         if (this.firstChild === oldChildWrapper)
           this.firstChild_ = nodes[0];
@@ -326,12 +350,13 @@
               oldChildNode);
         }
       } else {
+        nodes = collectNodesNoNeedToUpdatePointers(newChildWrapper);
         ensureSameOwnerDocument(this, newChildWrapper);
         originalReplaceChild.call(this.impl, unwrap(newChildWrapper),
                                   oldChildNode);
       }
 
-      newChildWrapper.nodeWasAdded_();
+      nodesWereAdded(nodes);
 
       return oldChildWrapper;
     },

diff --git a/test/js/HTMLContentElement.js b/test/js/HTMLContentElement.js
@@ -42,6 +42,19 @@ suite('HTMLContentElement', function() {
     assertArrayEqual(content.getDistributedNodes(), [b]);
   });
 
+  test('getDistributedNodes add document fragment with content', function() {
+    var host = document.createElement('div');
+    host.innerHTML = ' <a></a> <a></a> <a></a> ';
+    var root = host.createShadowRoot();
+    var df = document.createDocumentFragment();
+    df.appendChild(document.createTextNode(' '));
+    var content = df.appendChild(document.createElement('content'));
+    df.appendChild(document.createTextNode(' '));
+    root.appendChild(df);
+
+    assertArrayEqual(content.getDistributedNodes().length, 3);
+  });
+
   test('adding a new content element to a shadow tree', function() {
     var host = document.createElement('div');
     host.innerHTML = '<a></a><b></b>';