From 6e16619a4a0c87676d156c552608f48bdcf12eb4 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 14 Jan 2016 15:12:34 -0800 Subject: [PATCH 1/3] Fixes #3295. Only cache a false-y result for an element's owner shady root iff the element is currently in the document. --- src/lib/dom-api-shady.html | 15 +++++++--- test/smoke/owner-root.html | 59 ++++++++++++++++++++++++++++++++++++++ test/unit/polymer-dom.js | 18 ++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 test/smoke/owner-root.html diff --git a/src/lib/dom-api-shady.html b/src/lib/dom-api-shady.html index e1f62ffe..3e4a216e 100644 --- a/src/lib/dom-api-shady.html +++ b/src/lib/dom-api-shady.html @@ -170,8 +170,8 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN if (!node) { return; } - if (node._ownerShadyRoot === undefined) { - var root; + var root = node._ownerShadyRoot; + if (root === undefined) { if (node._isShadyRoot) { root = node; } else { @@ -183,9 +183,16 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN root = null; } } - node._ownerShadyRoot = root; + // memo-ize result for performance but only memo-ize a false-y + // result if node is in the document. This avoids a problem where a root + // can be cached while an element is inside a fragment. + // If this happens and we cache the result, the value can become stale + // because for perf we avoid processing the subtree of added fragments. + if (root || document.contains(node)) { + node._ownerShadyRoot = root; + } } - return node._ownerShadyRoot; + return root; }, _maybeDistribute: function(node) { diff --git a/test/smoke/owner-root.html b/test/smoke/owner-root.html new file mode 100644 index 00000000..f4ef1120 --- /dev/null +++ b/test/smoke/owner-root.html @@ -0,0 +1,59 @@ + + + + + early owner-root + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/unit/polymer-dom.js b/test/unit/polymer-dom.js index ab9f735d..80a33077 100644 --- a/test/unit/polymer-dom.js +++ b/test/unit/polymer-dom.js @@ -1207,6 +1207,24 @@ suite('Polymer.dom non-distributed elements', function() { assert.equal(Polymer.dom(test).getOwnerRoot(), c1.root, 'getOwnerRoot incorrect for child added to element in root'); }); + test('getOwnerRoot when out of tree and adding subtree', function() { + var container = document.createDocumentFragment(); + var test = document.createElement('div'); + container.appendChild(test); + assert.notOk(Polymer.dom(test).getOwnerRoot(), 'getOwnerRoot incorrect when not in root'); + var c1 = document.createElement('x-compose'); + var project = c1.$.project; + Polymer.dom(project).appendChild(container); + Polymer.dom.flush(); + assert.equal(Polymer.dom(test).getOwnerRoot(), c1.root, 'getOwnerRoot incorrect for child added to element in root'); + Polymer.dom(project).removeChild(test); + Polymer.dom.flush(); + assert.notOk(Polymer.dom(test).getOwnerRoot(), 'getOwnerRoot incorrect for child moved from a root to no root'); + Polymer.dom(project).appendChild(test); + Polymer.dom.flush(); + assert.equal(Polymer.dom(test).getOwnerRoot(), c1.root, 'getOwnerRoot incorrect for child added to element in root'); + }); + test('getOwnerRoot, subtree', function() { var test = document.createElement('div'); var testChild = document.createElement('div'); From b9e5cce27fcd4734c61cf60f493241e1a78ccb68 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 14 Jan 2016 15:39:00 -0800 Subject: [PATCH 2/3] Ensure querySelector always returns `null` when a node is not found. Also optimize querySelector such that the matcher halts on the first result. --- src/lib/dom-api-shady.html | 8 +++++++- src/lib/dom-api.html | 22 +++++++++++++++------- test/unit/polymer-dom.js | 2 +- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/lib/dom-api-shady.html b/src/lib/dom-api-shady.html index 3e4a216e..46d445ce 100644 --- a/src/lib/dom-api-shady.html +++ b/src/lib/dom-api-shady.html @@ -352,7 +352,13 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN // TODO(sorvell): consider doing native QSA and filtering results. querySelector: function(selector) { - return this.querySelectorAll(selector)[0]; + // match selector and halt on first result. + var result = this._query(function(n) { + return DomApi.matchesSelector.call(n, selector); + }, this.node, function(n) { + return Boolean(n); + })[0]; + return result || null; }, querySelectorAll: function(selector) { diff --git a/src/lib/dom-api.html b/src/lib/dom-api.html index caa0ca2b..8cd9e9e9 100644 --- a/src/lib/dom-api.html +++ b/src/lib/dom-api.html @@ -139,26 +139,34 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN // NOTE: `_query` is used primarily for ShadyDOM's querySelector impl, // but it's also generally useful to recurse through the element tree // and is used by Polymer's styling system. - _query: function(matcher, node) { + _query: function(matcher, node, halter) { node = node || this.node; var list = []; - this._queryElements(TreeApi.Logical.getChildNodes(node), matcher, list); + this._queryElements(TreeApi.Logical.getChildNodes(node), matcher, + halter, list); return list; }, - _queryElements: function(elements, matcher, list) { + _queryElements: function(elements, matcher, halter, list) { for (var i=0, l=elements.length, c; (i Date: Thu, 14 Jan 2016 15:59:01 -0800 Subject: [PATCH 3/3] Correct use of document.contains to document.documentElement.contains on IE. --- src/lib/dom-api-shady.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dom-api-shady.html b/src/lib/dom-api-shady.html index 46d445ce..0e0b24a2 100644 --- a/src/lib/dom-api-shady.html +++ b/src/lib/dom-api-shady.html @@ -188,7 +188,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN // can be cached while an element is inside a fragment. // If this happens and we cache the result, the value can become stale // because for perf we avoid processing the subtree of added fragments. - if (root || document.contains(node)) { + if (root || document.documentElement.contains(node)) { node._ownerShadyRoot = root; } }