From b1805d65329ca8793129215399be50d67b15d161 Mon Sep 17 00:00:00 2001 From: Erik Arvidsson Date: Wed, 28 Aug 2013 16:38:31 -0400 Subject: [PATCH] Fix issues related to tree changes. - Changing content[select] to more restrictive needs to clean up non matching nodes (we clear the child nodes now) - Changes to fallback content needs to invalate - Adding a content or shadow needs to invalidate wip --- src/ShadowRenderer.js | 95 +++++++++++++++++++----------- src/wrappers/HTMLShadowElement.js | 4 -- src/wrappers/Node.js | 29 +++++++--- src/wrappers/events.js | 2 - test/js/HTMLContentElement.js | 96 +++++++++++++++++++++++++++++++ test/js/HTMLShadowElement.js | 77 +++++++++++++++++++++++++ test/js/test.js | 32 +++++++++-- 7 files changed, 283 insertions(+), 52 deletions(-) diff --git a/src/ShadowRenderer.js b/src/ShadowRenderer.js index c3cd916..a533848 100644 --- a/src/ShadowRenderer.js +++ b/src/ShadowRenderer.js @@ -8,7 +8,9 @@ var HTMLContentElement = scope.wrappers.HTMLContentElement; var HTMLShadowElement = scope.wrappers.HTMLShadowElement; var Node = scope.wrappers.Node; + var ShadowRoot = scope.wrappers.ShadowRoot; var assert = scope.assert; + var getHostForShadowRoot = scope.getHostForShadowRoot; var mixin = scope.mixin; var oneOf = scope.oneOf; var unwrap = scope.unwrap; @@ -53,7 +55,8 @@ function removeAllChildNodes(parentNodeWrapper) { var parentNode = unwrap(parentNodeWrapper); updateAllChildNodes(parentNodeWrapper); - parentNode.textContent = ''; + if (parentNode.firstChild) + parentNode.textContent = ''; } function appendChild(parentNodeWrapper, childWrapper) { @@ -264,6 +267,17 @@ return renderer; } + function getShadowRootAncestor(node) { + for (; node; node = node.parentNode) { + if (node instanceof ShadowRoot) + return node; + } + return null; + } + + function getRendererForShadowRoot(shadowRoot) { + return getRendererForHost(getHostForShadowRoot(shadowRoot)); + } ShadowRenderer.prototype = { @@ -277,8 +291,6 @@ var host = this.host; var shadowDOM = host.shadowRoot; - if (!shadowDOM) - return; this.removeAllChildNodes(this.host); @@ -315,14 +327,18 @@ } }, - renderAsAnyDomTree: function(visualParent, tree, child, isNested) { - this.appendChild(visualParent, child); + renderAsAnyDomTree: function(visualParent, tree, node, isNested) { + this.appendChild(visualParent, node); - if (isShadowHost(child)) { - render(child); + if (isShadowHost(node)) { + render(node); } else { - var parent = child; + var parent = node; var logicalChildNodes = getChildNodesSnapshot(parent); + // We associate the parent of a content/shadow with the renderer + // because we may need to remove stale childNodes. + if (shadowDOMRendererTable.get(parent)) + this.removeAllChildNodes(parent); logicalChildNodes.forEach(function(node) { this.renderNode(parent, tree, node, isNested); }, this); @@ -362,12 +378,13 @@ renderFallbackContent: function (visualParent, fallbackHost) { var logicalChildNodes = getChildNodesSnapshot(fallbackHost); + this.associateNode(fallbackHost); + this.remove(fallbackHost); logicalChildNodes.forEach(function(node) { this.appendChild(visualParent, node); }, this); }, - /** * Invalidates the attributes used to keep track of which attributes may * cause the renderer to be invalidated. @@ -481,24 +498,24 @@ } }, - // Visual DOM mutation. appendChild: function(parent, child) { + // this.associateNode(child); + this.associateNode(parent); appendChild(parent, child); - this.associateNode(child); }, remove: function(node) { + // this.associateNode(node); + this.associateNode(node.parentNode); remove(node); - this.associateNode(node); }, removeAllChildNodes: function(parent) { + this.associateNode(parent); removeAllChildNodes(parent); - // TODO(arv): Does this need to associate all the nodes with this renderer? }, associateNode: function(node) { - // TODO: Clear when moved out of shadow tree. shadowDOMRendererTable.set(node, this); } }; @@ -523,7 +540,7 @@ } function isShadowHost(shadowHost) { - return !!shadowHost.shadowRoot; + return shadowHost.shadowRoot; } function getShadowTrees(host) { @@ -544,34 +561,48 @@ new ShadowRenderer(host).render(); }; + // Need to rerender shadow host when: + // + // - a direct child to the ShadowRoot is added or removed + // - a direct child to the host is added or removed + // - a new shadow root is created + // - a direct child to a content/shadow element is added or removed + // - a sibling to a content/shadow element is added or removed + // - content[select] is changed + // - an attribute in a direct child to a host is modified + + /** + * This gets called when a node was added or removed to it. + */ Node.prototype.invalidateShadowRenderer = function(force) { - if (force || this.shadowRoot) { - var renderer = shadowDOMRendererTable.get(this); - if (renderer) { - renderer.invalidate(); - return true; - } + var renderer = shadowDOMRendererTable.get(this); + if (renderer) { + renderer.invalidate(); + return true; } return false; }; HTMLContentElement.prototype.getDistributedNodes = function() { - // TODO(arv): We should associate the element with the shadow root so we - // only have to rerender this ShadowRenderer. - renderAllPending(); + var renderer = shadowDOMRendererTable.get(this); + if (renderer) + renderer.render(); return getDistributedChildNodes(this); }; - HTMLShadowElement.prototype.invalidateShadowRenderer = - HTMLContentElement.prototype.invalidateShadowRenderer = function(force) { - var renderer = shadowDOMRendererTable.get(this); - if (renderer) { + HTMLShadowElement.prototype.nodeWasAdded_ = + HTMLContentElement.prototype.nodeWasAdded_ = function() { + // Invalidate old renderer if any. + this.invalidateShadowRenderer(); + + var shadowRoot = getShadowRootAncestor(this); + var renderer; + if (shadowRoot) + renderer = getRendererForShadowRoot(shadowRoot); + shadowDOMRendererTable.set(this, renderer); + if (renderer) renderer.invalidate(); - return true; - } - - return false; }; scope.eventParentsTable = eventParentsTable; diff --git a/src/wrappers/HTMLShadowElement.js b/src/wrappers/HTMLShadowElement.js index f8634fb..9f6b761 100644 --- a/src/wrappers/HTMLShadowElement.js +++ b/src/wrappers/HTMLShadowElement.js @@ -16,10 +16,6 @@ } HTMLShadowElement.prototype = Object.create(HTMLElement.prototype); mixin(HTMLShadowElement.prototype, { - invalidateShadowRenderer: function() { - return HTMLElement.prototype.invalidateShadowRenderer.call(this, true); - }, - // TODO: attribute boolean resetStyleInheritance; }); diff --git a/src/wrappers/Node.js b/src/wrappers/Node.js index 4b7857f..c74fec4 100644 --- a/src/wrappers/Node.js +++ b/src/wrappers/Node.js @@ -123,6 +123,11 @@ } } + function invalidateParent(node) { + var p = node.parentNode; + return p && p.invalidateShadowRenderer(); + } + var OriginalNode = window.Node; /** @@ -183,9 +188,7 @@ appendChild: function(childWrapper) { assertIsNodeWrapper(childWrapper); - // if (this.invalidateShadowRenderer() || - // childWrapper/invalidateShadowRenderer()) { - if (this.invalidateShadowRenderer()) { + if (this.invalidateShadowRenderer() || invalidateParent(childWrapper)) { var previousNode = this.lastChild; var nextNode = null; var nodes = collectNodes(childWrapper, this, previousNode, nextNode); @@ -200,6 +203,8 @@ originalAppendChild.call(this.impl, unwrap(childWrapper)); } + childWrapper.nodeWasAdded_(); + return childWrapper; }, @@ -212,9 +217,7 @@ assertIsNodeWrapper(refWrapper); assert(refWrapper.parentNode === this); - var childParent = childWrapper.parentNode; - if (this.invalidateShadowRenderer() || - childParent && childParent.invalidateShadowRenderer()) { + if (this.invalidateShadowRenderer() || invalidateParent(childWrapper)) { var previousNode = refWrapper.previousSibling; var nextNode = refWrapper; var nodes = collectNodes(childWrapper, this, previousNode, nextNode); @@ -240,6 +243,8 @@ unwrap(refWrapper)); } + childWrapper.nodeWasAdded_(); + return childWrapper; }, @@ -295,10 +300,9 @@ } var oldChildNode = unwrap(oldChildWrapper); - var newChildParent = newChildWrapper.parentNode; if (this.invalidateShadowRenderer() || - newChildParent && newChildParent.invalidateShadowRenderer()) { + invalidateParent(newChildWrapper)) { var previousNode = oldChildWrapper.previousSibling; var nextNode = oldChildWrapper.nextSibling; if (nextNode === newChildWrapper) @@ -327,9 +331,18 @@ oldChildNode); } + newChildWrapper.nodeWasAdded_(); + return oldChildWrapper; }, + /** + * Called after a node was added. Subclasses override this to invalidate + * the renderer as needed. + * @private + */ + nodeWasAdded_: function() {}, + hasChildNodes: function() { return this.firstChild === null; }, diff --git a/src/wrappers/events.js b/src/wrappers/events.js index 1d9fb75..5574e54 100644 --- a/src/wrappers/events.js +++ b/src/wrappers/events.js @@ -176,8 +176,6 @@ return true; if (a instanceof wrappers.ShadowRoot) { var host = scope.getHostForShadowRoot(a); - if (!host) - return false; return enclosedBy(rootOfNode(host), b); } return false; diff --git a/test/js/HTMLContentElement.js b/test/js/HTMLContentElement.js index dd7c8f8..758b802 100644 --- a/test/js/HTMLContentElement.js +++ b/test/js/HTMLContentElement.js @@ -6,6 +6,8 @@ suite('HTMLContentElement', function() { + var unwrap = ShadowDOMPolyfill.unwrap; + test('select', function() { var el = document.createElement('content'); assert.equal(el.select, null); @@ -40,4 +42,98 @@ suite('HTMLContentElement', function() { assertArrayEqual(content.getDistributedNodes(), [b]); }); + test('adding a new content element to a shadow tree', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + var b = host.lastChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var c = sr.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + var content = document.createElement('content'); + content.select = 'b'; + c.appendChild(content); + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + c.removeChild(content); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + }); + + test('restricting select further', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + var b = host.lastChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var c = sr.firstChild; + var content = c.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + content.select = 'b'; + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + }); + + test('Mutating content fallback', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var content = sr.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + content.textContent = 'fallback'; + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + var b = content.appendChild(document.createElement('b')); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + content.removeChild(b); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + }); + + test('Mutating content fallback 2', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var b = sr.firstChild; + var content = b.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + content.textContent = 'fallback'; + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + var c = content.appendChild(document.createElement('c')); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + content.removeChild(c); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + }); }); \ No newline at end of file diff --git a/test/js/HTMLShadowElement.js b/test/js/HTMLShadowElement.js index 1107e15..e9d8335 100644 --- a/test/js/HTMLShadowElement.js +++ b/test/js/HTMLShadowElement.js @@ -32,4 +32,81 @@ suite('HTMLShadowElement', function() { assert.equal(unwrap(host).innerHTML, 'dabcf'); }); + + test('adding a new shadow element to a shadow tree', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + + var sr2 = host.createShadowRoot(); + sr2.innerHTML = ''; + var b = sr2.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + var shadow = document.createElement('shadow'); + b.appendChild(shadow); + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + b.removeChild(shadow); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + }); + + test('Mutating shadow fallback', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var shadow = sr.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + shadow.textContent = 'fallback'; + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + var b = shadow.appendChild(document.createElement('b')); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + shadow.removeChild(b); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + }); + + test('Mutating shadow fallback 2', function() { + var host = document.createElement('div'); + host.innerHTML = ''; + var a = host.firstChild; + + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var b = sr.firstChild; + var shadow = b.firstChild; + + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, ''); + + shadow.textContent = 'fallback'; + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + var c = shadow.appendChild(document.createElement('c')); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + + shadow.removeChild(c); + host.offsetHeight; + assert.equal(unwrap(host).innerHTML, 'fallback'); + }); }); diff --git a/test/js/test.js b/test/js/test.js index 175bcbf..ec6165b 100644 --- a/test/js/test.js +++ b/test/js/test.js @@ -417,18 +417,38 @@ suite('Shadow DOM', function() { assert.equal(count, 0); assert.equal(getVisualInnerHtml(host), ''); - var e = sr.appendChild(document.createElement('e')); + b.appendChild(document.createElement('e')); + assert.equal(count, 0); + assert.equal(getVisualInnerHtml(host), ''); + + var f = sr.appendChild(document.createElement('f')); assert.equal(count, 1); - assert.equal(getVisualInnerHtml(host), ''); + assert.equal(getVisualInnerHtml(host), ''); - e.appendChild(document.createElement('f')); + f.appendChild(document.createElement('g')); assert.equal(count, 1); - assert.equal(getVisualInnerHtml(host), ''); + assert.equal(getVisualInnerHtml(host), + ''); - host.insertBefore(document.createElement('g'), text); + host.insertBefore(document.createElement('h'), text); assert.equal(count, 2); assert.equal(getVisualInnerHtml(host), - ''); + ''); + }); + + test('issue-235', function() { + var host = document.createElement('div'); + var sr = host.createShadowRoot(); + sr.innerHTML = ''; + var a = sr.firstChild; + var b = a.firstChild; + + assert.equal(getVisualInnerHtml(host), ''); + + var c = document.createElement('c'); + a.appendChild(c); + + assert.equal(a.childNodes.length, 2); }); }); \ No newline at end of file