In addition to fragments, also handle non-distributed elements more c…

…ompletely.

src/lib/dom-api-shady.html

@@ -99,14 +99,26 @@
         TreeApi.Logical.recordInsertBefore(node, this.node, ref_node);
       }
       // if not distributing and not adding to host, do a fast path addition
-      var handled = (this._maybeDistribute(node) ||
-        this._tryRemoveUndistributedNode(node));
-      // if distribution is handling this node and it's a fragment, 
-      // the actual dom may not be removed from the fragment if some nodes
+      var handled = this._maybeDistribute(node) ||
+        this.node.shadyRoot;
+      // if shady is handling this node, 
+      // the actual dom may not be removed if the node or fragment contents
       // remain undistributed so we ensure removal here.
-      if (handled && (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE)) {
-        while (node.firstChild) {
-          node.removeChild(node.firstChild);
+      // NOTE: we only remove from existing location iff shady dom is involved.
+      // This is because a node fragment is passed to the native add method
+      // which expects to see fragment children. Regular elements must also
+      // use this check because not doing so causes queuing of attached/detached
+      // and breaks, for example, dom-if's attached/detached checks.
+      if (handled) {
+        if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
+          while (node.firstChild) {
+            TreeApi.Composed.removeChild(node, node.firstChild);
+          }
+        } else {
+          var parent = TreeApi.Composed.getParentNode(node);
+          if (parent) {
+            TreeApi.Composed.removeChild(parent, node);
+          }
         }
       }
       return handled;
@@ -126,7 +138,8 @@
         var container = this.node._isShadyRoot ? this.node.host : this.node;
         // not guaranteed to physically be in container; e.g.
         // undistributed nodes.
-        if (container === node.parentNode) {
+        var parent = TreeApi.Composed.getParentNode(node);
+        if (container === parent) {
           TreeApi.Composed.removeChild(container, node);
         }
       }
@@ -266,16 +279,6 @@
       return added;
     },
 
-    _tryRemoveUndistributedNode: function(node) {
-      if (this.node.shadyRoot) {
-        var parent = TreeApi.Composed.getParentNode(node);
-        if (parent) {
-          TreeApi.Composed.removeChild(parent, node);
-        }
-        return true;
-      }
-    },
-
     _updateInsertionPoints: function(host) {
       var i$ = host.shadyRoot._insertionPoints =
         dom(host.shadyRoot).querySelectorAll(CONTENT);
@@ -315,7 +318,7 @@
           for (var j=0; j<dc$.length; j++) {
             hostNeedsDist = true;
             var node = dc$[j];
-            var parent = node.parentNode;
+            var parent = TreeApi.Composed.getParentNode(node);
             if (parent) {
               TreeApi.Composed.removeChild(parent, node);
             }

test/unit/polymer-dom-content.html

@@ -1443,7 +1443,7 @@
       });
     });
 
-    test('adding a document fragment always clears nodes in the fragment', function() {
+    test('adding a document fragment clears nodes in the fragment', function() {
       var x = document.createElement('x-dist-star');
       var frag = document.createDocumentFragment();
       frag.appendChild(document.createTextNode('hi'));
@@ -1453,6 +1453,18 @@
       assert.equal(frag.childNodes.length, 0);
     });
 
+    test('adding a non-distributing node removes the node from its current location', function() {
+      var x = document.createElement('x-dist-star');
+      var t = document.createTextNode('hi');
+      document.body.appendChild(t);
+      Polymer.dom(x).appendChild(t);
+      Polymer.dom.flush();
+      assert.equal(Polymer.dom(t).parentNode, x);
+      if (x.shadyRoot) {
+        assert.isNull(t.parentNode);
+      }
+    });
+
   });