Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Fix issues where we did not update the renderer after DOM changes #233

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 63 additions & 32 deletions src/ShadowRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -53,7 +55,8 @@
function removeAllChildNodes(parentNodeWrapper) {
var parentNode = unwrap(parentNodeWrapper);
updateAllChildNodes(parentNodeWrapper);
parentNode.textContent = '';
if (parentNode.firstChild)
parentNode.textContent = '';
}

function appendChild(parentNodeWrapper, childWrapper) {
Expand Down Expand Up @@ -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 = {

Expand All @@ -277,8 +291,6 @@

var host = this.host;
var shadowDOM = host.shadowRoot;
if (!shadowDOM)
return;

this.removeAllChildNodes(this.host);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
};
Expand All @@ -523,7 +540,7 @@
}

function isShadowHost(shadowHost) {
return !!shadowHost.shadowRoot;
return shadowHost.shadowRoot;
}

function getShadowTrees(host) {
Expand All @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions src/wrappers/HTMLShadowElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down
29 changes: 21 additions & 8 deletions src/wrappers/Node.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@
}
}

function invalidateParent(node) {
var p = node.parentNode;
return p && p.invalidateShadowRenderer();
}

var OriginalNode = window.Node;

/**
Expand Down Expand Up @@ -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);
Expand All @@ -200,6 +203,8 @@
originalAppendChild.call(this.impl, unwrap(childWrapper));
}

childWrapper.nodeWasAdded_();

return childWrapper;
},

Expand All @@ -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);
Expand All @@ -240,6 +243,8 @@
unwrap(refWrapper));
}

childWrapper.nodeWasAdded_();

return childWrapper;
},

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
},
Expand Down
2 changes: 0 additions & 2 deletions src/wrappers/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading