Merge pull request Polymer#1845 from Polymer/fix-1804

Clear composedNodes when an element upgrades without an insertion point

src/lib/dom-api.html

@@ -59,7 +59,7 @@
       // 3. node is <content> (host of container needs distribution)
       appendChild: function(node) {
         var handled;
-        this._removeNodeFromHost(node);
+        this._removeNodeFromHost(node, true);
         if (this._nodeIsInLogicalTree(this.node)) {
           this._addLogicalInfo(node, this.node);
           this._addNodeToHost(node);
@@ -69,8 +69,8 @@
         if (!handled && !this._tryRemoveUndistributedNode(node)) {
           // if adding to a shadyRoot, add to host instead
           var container = this.node._isShadyRoot ? this.node.host : this.node;
-          nativeAppendChild.call(container, node);
           addToComposedParent(container, node);
+          nativeAppendChild.call(container, node);
         }
         return node;
       },
@@ -80,7 +80,7 @@
           return this.appendChild(node);
         }
         var handled;
-        this._removeNodeFromHost(node);
+        this._removeNodeFromHost(node, true);
         if (this._nodeIsInLogicalTree(this.node)) {
           saveLightChildrenIfNeeded(this.node);
           var children = this.childNodes;
@@ -100,8 +100,8 @@
             this._firstComposedNode(ref_node) : ref_node;
           // if adding to a shadyRoot, add to host instead
           var container = this.node._isShadyRoot ? this.node.host : this.node;
-          nativeInsertBefore.call(container, node, ref_node);
           addToComposedParent(container, node, ref_node);
+          nativeInsertBefore.call(container, node, ref_node);
         }
         return node;
       },
@@ -126,8 +126,8 @@
           // not guaranteed to physically be in container; e.g.
           // undistributed nodes.
           if (container === node.parentNode) {
-            nativeRemoveChild.call(container, node);
             removeFromComposedParent(container, node);
+            nativeRemoveChild.call(container, node);
           }
         }
         return node;
@@ -228,10 +228,13 @@
         return parent && parent.shadyRoot && hasInsertionPoint(parent.shadyRoot);
       },
 
-      _removeNodeFromHost: function(node) {
+      // NOTE: if `ensureComposedRemoval` is true then the node should be 
+      // removed from its composed parent. 
+      _removeNodeFromHost: function(node, ensureComposedRemoval) {
         var hostNeedsDist;
         var root;
-        if (node._lightParent) {
+        var parent = node._lightParent;
+        if (parent) {
           root = this._ownerShadyRootForNode(node);
           if (root) {
             root.host._elementRemove(node);
@@ -243,6 +246,8 @@
         if (root && hostNeedsDist) {
           this._updateInsertionPoints(root.host);
           this._lazyDistribute(root.host);
+        } else if (ensureComposedRemoval) {
+          removeFromComposedParent(parent || node.parentNode, node);
         }
       },
 
@@ -258,8 +263,8 @@
               var node = dc$[i];
               var parent = node.parentNode;
               if (parent) {
-                nativeRemoveChild.call(parent, node);
                 removeFromComposedParent(parent, node);
+                nativeRemoveChild.call(parent, node);
               }
             }
           }
@@ -724,6 +729,9 @@
 
     function getLightChildren(node) {
       var children = node._lightChildren;
+      // TODO(sorvell): it's more correct to use _composedChildren instead of 
+      // childNodes here but any trivial failure to use Polymer.dom 
+      // will result in an error so we avoid using _composedChildren
       return children ? children : node.childNodes;
     }
 
@@ -739,9 +747,10 @@
       var i = ref_node ? children.indexOf(ref_node) : -1;
       if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
         var fragChildren = getComposedChildren(node);
-        fragChildren.forEach(function(c) {
-          addNodeToComposedChildren(c, parent, children, i);
-        });
+        for (var j=0; j < fragChildren.length; j++) {
+          addNodeToComposedChildren(fragChildren[j], parent, children, i + j);
+        }
+        node._composedChildren = null;
       } else {
         addNodeToComposedChildren(node, parent, children, i);
       }
@@ -750,11 +759,7 @@
 
     function addNodeToComposedChildren(node, parent, children, i) {
       node._composedParent = parent;
-      if (i >= 0) {
-        children.splice(i, 0, node);
-      } else {
-        children.push(node);
-      }
+      children.splice(i >= 0 ? i : children.length, 0, node);
     }
 
     function removeFromComposedParent(parent, node) {

src/mini/shady.html

@@ -138,6 +138,10 @@
           } else {
             if (!this.shadyRoot._hasDistributed) {
               this.textContent = '';
+              // reset composed children here in case they may have already
+              // been set (this shouldn't happen but can if dependency ordering
+              // is incorrect and as a result upgrade order is unexpected)
+              this._composedChildren = null;
               this.appendChild(this.shadyRoot);
             } else {
               // simplified non-tree walk composition

test/unit/dom-repeat.html

@@ -452,13 +452,13 @@ <h4>inDocumentRepeater</h4>
 
       test('move to different container', function() {
         var repeater = inDocumentRepeater;
-        repeater.parentElement.removeChild(repeater);
+        Polymer.dom(repeater.parentElement).removeChild(repeater);
         // TODO(kschaaf): detached/attached not called if takeRecords isn't
         // called between remove & insert on polyfill... See wcjs #311.
         CustomElements.takeRecords();
         CustomElements.takeRecords();
         CustomElements.takeRecords();
-        inDocumentContainer.appendChild(repeater);
+        Polymer.dom(inDocumentContainer).appendChild(repeater);
         CustomElements.takeRecords();
         CustomElements.takeRecords();
         CustomElements.takeRecords();
@@ -508,8 +508,8 @@ <h4>inDocumentRepeater</h4>
 
       test('basic rendering, downward item binding', function() {
         var r = inDocumentRepeater;
-        r.parentElement.removeChild(r);
-        inDocumentContainer.appendChild(r);
+        Polymer.dom(r.parentElement).removeChild(r);
+        Polymer.dom(inDocumentContainer).appendChild(r);
 
         var stamped = Polymer.dom(inDocumentContainer).querySelectorAll('*:not(template)');
         assert.equal(stamped.length, 3 + 3*3 + 3*3*3, 'total stamped count incorrect');

test/unit/polymer-dom-content.html

@@ -119,6 +119,28 @@
   </script>
 </dom-module>
 
+<dom-module id="x-lazy-no-dist">
+  <template>
+    Lazy no dist!
+  </template>
+</dom-module>
+
+<dom-module id="x-compose-lazy-no-dist">
+  <template>
+    <x-lazy-no-dist id="lazy">
+      <content></content>
+    </x-lazy-no-dist>
+  </template>
+  <script>
+  HTMLImports.whenReady(function() {
+    Polymer({is: 'x-compose-lazy-no-dist'});
+  });
+  </script>
+</dom-module>
+
+<x-compose-lazy-no-dist><span>Child</span></x-compose-lazy-no-dist>
+
+
 <script>
 
   suite('appendChild & removeChild of <content>', function() {
@@ -672,6 +694,97 @@
       document.body.removeChild(compose);
     });
 
+    test('append content to initially un-upgraded element', function() {
+      var lazyContainer = document.querySelector('x-compose-lazy-no-dist');
+      var child = Polymer.dom(lazyContainer).firstElementChild;
+      Polymer({is: 'x-lazy-no-dist'});
+      var content = document.createElement('content');
+      Polymer.dom(lazyContainer.$.lazy.root).appendChild(content);
+      Polymer.dom.flush();
+      assert.equal(Polymer.dom(content).getDistributedNodes()[1], child);
+      if (lazyContainer.shadyRoot) {
+        assert.equal(lazyContainer.$.lazy.lastElementChild, child);
+      }
+    });
+
+    test('composed children distributed', function() {
+      var host = document.createElement('x-dist');
+      document.body.appendChild(host);
+      var s0 = document.createElement('span');
+      var frag = document.createDocumentFragment();
+      var s1 = document.createElement('span');
+      var s2 = document.createElement('span');
+      var s3 = document.createElement('span');
+      frag.appendChild(s1);
+      frag.appendChild(s2);
+      frag.appendChild(s3);
+      Polymer.dom(host).appendChild(s0);
+      Polymer.dom(host).insertBefore(frag, s0);
+      Polymer.dom.flush();
+      if (host.shadyRoot) {
+        assert.equal(host.$.distWrapper.children.length, 4);
+        assert.equal(host.$.distWrapper.children[0], s1);
+        assert.equal(host.$.distWrapper.children[1], s2);
+        assert.equal(host.$.distWrapper.children[2], s3);
+        assert.equal(host.$.distWrapper.children[3], s0);
+        var composedChildren = host.$.distWrapper._composedChildren;
+        assert.equal(composedChildren.length, 4);
+        assert.equal(composedChildren[0], s1);
+        assert.equal(composedChildren[1], s2);
+        assert.equal(composedChildren[2], s3);
+        assert.equal(composedChildren[3], s0);
+      }
+      Polymer.dom(host).removeChild(s1);
+      Polymer.dom(host).removeChild(s2);
+      Polymer.dom(host).removeChild(s3);
+      Polymer.dom.flush();
+      if (host.shadyRoot) {
+        assert.equal(host.$.distWrapper.children.length, 1);
+        var composedChildren = host.$.distWrapper._composedChildren;
+        assert.equal(composedChildren.length, 1);
+        assert.equal(composedChildren[0], s0);
+      }
+    });
+
+    test('composed children added but not distributed', function() {
+      var host = document.createElement('x-dist');
+      document.body.appendChild(host);
+      var s0 = document.createElement('span');
+      var frag = document.createDocumentFragment();
+      var s1 = document.createElement('span');
+      var s2 = document.createElement('span');
+      var s3 = document.createElement('span');
+      frag.appendChild(s1);
+      frag.appendChild(s2);
+      frag.appendChild(s3);
+      Polymer.dom(host.$.distWrapper).appendChild(s0);
+      Polymer.dom(host.$.distWrapper).insertBefore(frag, s0);
+      Polymer.dom.flush();
+      if (host.shadyRoot) {
+        assert.equal(host.$.distWrapper.children.length, 4);
+        assert.equal(host.$.distWrapper.children[0], s1);
+        assert.equal(host.$.distWrapper.children[1], s2);
+        assert.equal(host.$.distWrapper.children[2], s3);
+        assert.equal(host.$.distWrapper.children[3], s0);
+        var composedChildren = host.$.distWrapper._composedChildren;
+        assert.equal(composedChildren.length, 4);
+        assert.equal(composedChildren[0], s1);
+        assert.equal(composedChildren[1], s2);
+        assert.equal(composedChildren[2], s3);
+        assert.equal(composedChildren[3], s0);
+      }
+      Polymer.dom(host.$.distWrapper).removeChild(s1);
+      Polymer.dom(host.$.distWrapper).removeChild(s2);
+      Polymer.dom(host.$.distWrapper).removeChild(s3);
+      Polymer.dom.flush();
+      if (host.shadyRoot) {
+        assert.equal(host.$.distWrapper.children.length, 1);
+        var composedChildren = host.$.distWrapper._composedChildren;
+        assert.equal(composedChildren.length, 1);
+        assert.equal(composedChildren[0], s0);
+      }
+    });
+
   });
 
   suite('multi-content mutations', function() {

test/unit/polymer-dom-elements.html

@@ -176,4 +176,4 @@
     </x-select-attr>
   </template>
   <script>Polymer({is: 'x-compose-select-attr'});</script>
-</dom-module>
+</dom-module>

test/unit/polymer-dom.js

@@ -743,5 +743,4 @@ suite('Polymer.dom non-distributed elements', function() {
     Polymer.dom(c1).appendChild(test);
     assert.notOk(Polymer.dom(test).getOwnerRoot(), 'getOwnerRoot incorrect for child moved from a root to no root');
   });
-
 });