Merge pull request #223 from arv/always-adopt-nodes

Make sure we adopt the child that we are inserting
googlearchive · Aug 22, 2013 · b11b32e · b11b32e
2 parents cfe0a09 + c6d5693
commit b11b32e
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 17 deletions.
diff --git a/src/wrappers/Node.js b/src/wrappers/Node.js
@@ -60,26 +60,31 @@
     return nodes;
   }
 
-  function adoptIfNeeded(node, doc) {
-    if (doc !== node.ownerDocument)
-      scope.adoptNodeNoRemove(node, doc);
+  function adoptNodesIfNeeded(owner, nodes) {
+    if (!nodes.length)
+      return;
+
+    var ownerDoc = owner.ownerDocument;
+
+    // All nodes have the same ownerDocument when we get here.
+    if (ownerDoc === nodes[0].ownerDocument)
+      return;
+
+    for (var i = 0; i < nodes.length; i++) {
+      scope.adoptNodeNoRemove(nodes[i], ownerDoc);
+    }
   }
 
   function unwrapNodesForInsertion(owner, nodes) {
+    adoptNodesIfNeeded(owner, nodes);
     var length = nodes.length;
 
-    if (length === 1) {
-      var node = nodes[0];
-      adoptIfNeeded(node, owner.ownerDocument);
-      return unwrap(node);
-    }
+    if (length === 1)
+      return unwrap(nodes[0]);
 
-    var ownerDoc = owner.ownerDocument;
-    var df = unwrap(ownerDoc.createDocumentFragment());
+    var df = unwrap(owner.ownerDocument.createDocumentFragment());
     for (var i = 0; i < length; i++) {
-      var node = nodes[i];
-      adoptIfNeeded(node, ownerDoc);
-      df.appendChild(unwrap(node));
+      df.appendChild(unwrap(nodes[i]));
     }
     return df;
   }
@@ -170,10 +175,6 @@
       if (!previousNode)
         this.firstChild_ = nodes[0];
 
-      // TODO(arv): It is unclear if we need to update the visual DOM here.
-      // A better aproach might be to make sure we only get here for nodes that
-      // are related to a shadow host and then invalidate that and re-render
-      // the host (on reflow?).
       originalAppendChild.call(this.impl, unwrapNodesForInsertion(this, nodes));
 
       return childWrapper;
@@ -201,11 +202,14 @@
       // insertBefore refWrapper no matter what the parent is?
       var refNode = unwrap(refWrapper);
       var parentNode = refNode.parentNode;
+
       if (parentNode) {
         originalInsertBefore.call(
             parentNode,
             unwrapNodesForInsertion(this, nodes),
             refNode);
+      } else {
+        adoptNodesIfNeeded(this, nodes);
       }
 
       return childWrapper;

diff --git a/test/js/paralleltrees.js b/test/js/paralleltrees.js
@@ -926,6 +926,60 @@ suite('Parallel Trees', function() {
       });
     });
 
+    test('insertBefore with different documents', function() {
+      var doc = document.implementation.createHTMLDocument('');
+      var div = doc.createElement('div');
+      div.innerHTML = '<a></a><b></b>';
+      var a = div.firstChild;
+      var b = div.lastChild;
+
+      visual.removeAllChildNodes(div);
+
+      expectStructure(div, {
+        firstChild: a,
+        lastChild: b
+      });
+
+      expectStructure(a, {
+        parentNode: div,
+        nextSibling: b
+      });
+
+      expectStructure(b, {
+        parentNode: div,
+        previousSibling: a
+      });
+
+      var c = document.createElement('c');
+      div.insertBefore(c, b);
+
+      expectStructure(div, {
+        firstChild: a,
+        lastChild: b
+      });
+
+      expectStructure(a, {
+        parentNode: div,
+        nextSibling: c
+      });
+
+      expectStructure(b, {
+        parentNode: div,
+        previousSibling: c
+      });
+
+      expectStructure(c, {
+        parentNode: div,
+        previousSibling: a,
+        nextSibling: b,
+      });
+
+      assert.equal(div.ownerDocument, doc);
+      assert.equal(a.ownerDocument, div.ownerDocument);
+      assert.equal(b.ownerDocument, div.ownerDocument);
+      assert.equal(c.ownerDocument, div.ownerDocument);
+    });
+
     suite('replaceChild', function() {
       test('simple', function() {
         var div = document.createElement('div');