Skip to content

Commit

Permalink
In addition to fragments, also handle non-distributed elements more c…
Browse files Browse the repository at this point in the history
…ompletely.
  • Loading branch information
Steven Orvell committed Jan 23, 2016
1 parent 713377e commit fe2699e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 20 deletions.
41 changes: 22 additions & 19 deletions src/lib/dom-api-shady.html
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
14 changes: 13 additions & 1 deletion test/unit/polymer-dom-content.html
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand All @@ -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);
}
});

});


Expand Down

0 comments on commit fe2699e

Please sign in to comment.