Skip to content

Commit 0d4f418

Browse files
author
Steven Orvell
committed
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.
1 parent 1c118e5 commit 0d4f418

3 files changed

Lines changed: 145 additions & 1 deletion

File tree

‎src/lib/dom-api.html‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,9 @@
274274
},
275275

276276
_removeNodeFromParent: function(node) {
277-
var parent = node._lightParent;
277+
// note: we may need to notify and not have logical info so fallback
278+
// to composed parentNode.
279+
var parent = node._lightParent || node.parentNode;
278280
if (parent && hasDomApi(parent)) {
279281
factory(parent).notifyObserver();
280282
}

‎test/smoke/observeNodes-repeat.html‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
5+
<title>observeNodes-repeat</title>
6+
7+
<meta charset="utf-8">
8+
<meta name="viewport" content="width=device-width, initial-scale=1.0">
9+
10+
<script src="../../../webcomponentsjs/webcomponents-lite.js"></script>
11+
<link rel="import" href="../../polymer.html">
12+
13+
<body>
14+
15+
<script>
16+
Polymer({
17+
is: 'x-observes-nodes',
18+
19+
properties: {
20+
children: {
21+
type: Array,
22+
readOnly: true,
23+
value: function() {
24+
return [];
25+
}
26+
}
27+
},
28+
29+
_updateChildren: function(info) {
30+
console.log('Updating children!', info);
31+
var children = Polymer.dom(this).queryDistributedElements('div');
32+
this._setChildren(children);
33+
},
34+
35+
attached: function() {
36+
Polymer.dom(this).observeNodes(function(info) {
37+
this._updateChildren(info);
38+
}.bind(this));
39+
}
40+
});
41+
42+
function randomObject() {
43+
return {
44+
name: 'foo'
45+
};
46+
}
47+
48+
function randomArray(size) {
49+
var array = [];
50+
for (var i = 0; i < size; ++i) {
51+
array.push(randomObject());
52+
}
53+
return array;
54+
}
55+
56+
function numberOfDivs() {
57+
return Polymer.dom(document).querySelectorAll('div').length;
58+
}
59+
60+
function assertDivs(number) {
61+
console.log('Do we have (' + number + ') divs?', number === numberOfDivs(), 'Actual: ' + numberOfDivs());
62+
}
63+
64+
function assertChildren() {
65+
var observesNodes = Polymer.dom(document).querySelector('x-observes-nodes');
66+
console.log('Same number of children as divs?', observesNodes.children.length === numberOfDivs(), 'Actual: ' + observesNodes.children.length);
67+
}
68+
69+
70+
71+
window.addEventListener('load', function() {
72+
var dom = document.querySelector('#x-dom');
73+
74+
console.log('Starting tests..');
75+
76+
dom.set('items', randomArray(3));
77+
78+
79+
Polymer.Base.async(function() {
80+
81+
assertDivs(3);
82+
assertChildren();
83+
84+
dom.set('items', randomArray(6));
85+
86+
Polymer.Base.async(function() {
87+
assertDivs(6);
88+
assertChildren();
89+
90+
dom.set('items', randomArray(4));
91+
92+
Polymer.Base.async(function() {
93+
assertDivs(4);
94+
assertChildren();
95+
96+
console.log('Tests done!');
97+
}, 10);
98+
}, 10);
99+
}, 10);
100+
});
101+
</script>
102+
103+
<template id="x-dom" is="dom-bind">
104+
<x-observes-nodes><template is="dom-repeat" items="[[items]]"><div>{{item.name}}</div></template></x-observes-nodes>
105+
</template>
106+
107+
</body>
108+
</html>

‎test/unit/polymer-dom-observeNodes.html‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@
165165

166166
<test-static><div>static A</div><div>static B</div></test-static>
167167

168+
<div id="staticDiv"></div>
169+
168170
<script>
169171

170172
suite('observeNodes', function() {
@@ -512,6 +514,38 @@
512514
document.body.removeChild(el);
513515
});
514516

517+
test('observe effective children changes in static content when adding to another host', function() {
518+
var el = document.createElement('staticDiv');
519+
document.body.appendChild(el);
520+
var recorded;
521+
var handle = Polymer.dom(el).observeNodes(function(info) {
522+
recorded = info;
523+
});
524+
Polymer.dom.flush();
525+
// add
526+
var d = document.createElement('div');
527+
var d1 = document.createElement('div');
528+
Polymer.dom(el).appendChild(d);
529+
Polymer.dom(el).appendChild(d1);
530+
Polymer.dom.flush();
531+
assert.equal(recorded.addedNodes.length, 2);
532+
assert.equal(recorded.removedNodes.length, 0);
533+
assert.equal(recorded.addedNodes[0], d);
534+
assert.equal(recorded.addedNodes[1], d1);
535+
// add somewhere else... we should see these as removes
536+
Polymer.dom(document.body).appendChild(d);
537+
Polymer.dom(document.body).appendChild(d1);
538+
Polymer.dom.flush();
539+
assert.equal(recorded.addedNodes.length, 0);
540+
assert.equal(recorded.removedNodes.length, 2);
541+
assert.equal(recorded.removedNodes[0], d);
542+
assert.equal(recorded.removedNodes[1], d1);
543+
// cleanup
544+
Polymer.dom(document.body).removeChild(d);
545+
Polymer.dom(document.body).removeChild(d1);
546+
document.body.removeChild(el);
547+
});
548+
515549
test('observe effective children inside deep distributing element', function() {
516550
var el = document.createElement('test-content3');
517551
document.body.appendChild(el);

0 commit comments

Comments
 (0)