Fix #2587: When Polymer.dom(el).appendChild(node) is called, cleanup …

…work must be performed on the existing parent of node. This change fixes a missing case in this cleanup work: if the existing parent has a observer via `Polymer.dom(parent).observeNodes`, it needs to be notified that node is being removed even if the node does not have specific logical info. For example, if an observed node has no Shady DOM and has a child that is removed. A test for this case was added.
Polymer · Oct 17, 2015 · 0d4f418 · 0d4f418
1 parent 1c118e5
commit 0d4f418
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 1 deletion.
diff --git a/src/lib/dom-api.html b/src/lib/dom-api.html
@@ -274,7 +274,9 @@
       },
 
       _removeNodeFromParent: function(node) {
-        var parent = node._lightParent;
+        // note: we may need to notify and not have logical info so fallback
+        // to composed parentNode.
+        var parent = node._lightParent || node.parentNode;
         if (parent && hasDomApi(parent)) {
           factory(parent).notifyObserver();
         }

diff --git a/test/smoke/observeNodes-repeat.html b/test/smoke/observeNodes-repeat.html
@@ -0,0 +1,108 @@
+<!doctype html>
+<html>
+<head>
+
+  <title>observeNodes-repeat</title>
+
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+  <script src="../../../webcomponentsjs/webcomponents-lite.js"></script>
+  <link rel="import" href="../../polymer.html">
+
+<body>
+
+  <script>
+    Polymer({
+      is: 'x-observes-nodes',
+
+      properties: {
+        children: {
+          type: Array,
+          readOnly: true,
+          value: function() {
+            return [];
+          }
+        }
+      },
+
+      _updateChildren: function(info) {
+        console.log('Updating children!', info);
+        var children = Polymer.dom(this).queryDistributedElements('div');
+        this._setChildren(children);
+      },
+
+      attached: function() {
+        Polymer.dom(this).observeNodes(function(info) {
+          this._updateChildren(info);
+        }.bind(this));
+      }
+    });
+
+    function randomObject() {
+      return {
+        name: 'foo'
+      };
+    }
+
+    function randomArray(size) {
+      var array = [];
+      for (var i = 0; i < size; ++i) {
+        array.push(randomObject());
+      }
+      return array;
+    }
+
+    function numberOfDivs() {
+      return Polymer.dom(document).querySelectorAll('div').length;
+    }
+
+    function assertDivs(number) {
+      console.log('Do we have (' + number + ') divs?', number === numberOfDivs(), 'Actual: ' + numberOfDivs());
+    }
+
+    function assertChildren() {
+      var observesNodes = Polymer.dom(document).querySelector('x-observes-nodes');
+      console.log('Same number of children as divs?', observesNodes.children.length === numberOfDivs(), 'Actual: ' + observesNodes.children.length);
+    }
+
+
+
+    window.addEventListener('load', function() {
+      var dom = document.querySelector('#x-dom');
+
+      console.log('Starting tests..');
+
+      dom.set('items', randomArray(3));
+
+
+      Polymer.Base.async(function() {
+
+        assertDivs(3);
+        assertChildren();
+
+        dom.set('items', randomArray(6));
+
+        Polymer.Base.async(function() {
+          assertDivs(6);
+          assertChildren();
+
+          dom.set('items', randomArray(4));
+
+          Polymer.Base.async(function() {
+            assertDivs(4);
+            assertChildren();
+
+            console.log('Tests done!');
+          }, 10);
+        }, 10);
+      }, 10);
+    });
+  </script>
+
+  <template id="x-dom" is="dom-bind">
+    <x-observes-nodes><template is="dom-repeat" items="[[items]]"><div>{{item.name}}</div></template></x-observes-nodes>
+  </template>
+
+</body>
+</html>
diff --git a/test/unit/polymer-dom-observeNodes.html b/test/unit/polymer-dom-observeNodes.html
@@ -165,6 +165,8 @@
 
   <test-static><div>static A</div><div>static B</div></test-static>
 
+  <div id="staticDiv"></div>
+
 <script>
 
   suite('observeNodes', function() {
@@ -512,6 +514,38 @@
       document.body.removeChild(el);
     });
 
+    test('observe effective children changes in static content when adding to another host', function() {
+      var el = document.createElement('staticDiv');
+      document.body.appendChild(el);
+      var recorded;
+      var handle = Polymer.dom(el).observeNodes(function(info) {
+        recorded = info;
+      });
+      Polymer.dom.flush();
+      // add
+      var d = document.createElement('div');
+      var d1 = document.createElement('div');
+      Polymer.dom(el).appendChild(d);
+      Polymer.dom(el).appendChild(d1);
+      Polymer.dom.flush();
+      assert.equal(recorded.addedNodes.length, 2);
+      assert.equal(recorded.removedNodes.length, 0);
+      assert.equal(recorded.addedNodes[0], d);
+      assert.equal(recorded.addedNodes[1], d1);
+      // add somewhere else... we should see these as removes
+      Polymer.dom(document.body).appendChild(d);
+      Polymer.dom(document.body).appendChild(d1);
+      Polymer.dom.flush();
+      assert.equal(recorded.addedNodes.length, 0);
+      assert.equal(recorded.removedNodes.length, 2);
+      assert.equal(recorded.removedNodes[0], d);
+      assert.equal(recorded.removedNodes[1], d1);
+      // cleanup
+      Polymer.dom(document.body).removeChild(d);
+      Polymer.dom(document.body).removeChild(d1);
+      document.body.removeChild(el);
+    });
+
     test('observe effective children inside deep distributing element', function() {
       var el = document.createElement('test-content3');
       document.body.appendChild(el);