- Revision
- 243233
- Author
- [email protected]
- Date
- 2019-03-20 13:26:18 -0700 (Wed, 20 Mar 2019)
Log Message
appendChild should throw when inserting an ancestor of a template into its content adopted to another document
https://bugs.webkit.org/show_bug.cgi?id=195984
Reviewed by Darin Adler.
LayoutTests/imported/w3c:
Rebaselined the test that is not fully passing.
* web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-content-hierarcy-expected.txt:
Source/WebCore:
The WPT test caught a bug that appendChild and other DOM insertion functions were incorrectly assuming that
any node that's in a HTML template element has the current document's template document as its owner.
The assumption is wrong when the template element's content DocumentFragment is adopted to another document.
Fixed the bug by always checking the ancestor host elements in checkAcceptChild. Also
Test: fast/dom/insert-template-parent-into-adopted-content.html
* dom/ContainerNode.cpp:
(WebCore::isInTemplateContent): Deleted. This code is simply wrong.
(WebCore::containsConsideringHostElements): Deleted. Call sites are updated to use containsIncludingHostElements.
(WebCore::containsIncludingHostElements): Moved from Node.cpp and optimized this code a bit. It's more efficient
to get the parent node and check for ShadowRoot and DocumentFragment only when the parent is null than to check
for those two node types before getting the parent node.
(WebCore::checkAcceptChild): Merged two code paths to call containsIncludingHostElements. The early return for
a pseudo element is there only to prevent tree corruption in release build even in the presence of a major bug
so it shouldn't be an spec compliance issue.
* dom/Node.cpp:
(WebCore::Node::containsIncludingHostElements const): Deleted.
* dom/Node.h:
LayoutTests:
Added a regression test.
* fast/dom/insert-template-parent-into-adopted-content-expected.txt: Added.
* fast/dom/insert-template-parent-into-adopted-content.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (243232 => 243233)
--- trunk/LayoutTests/ChangeLog 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/LayoutTests/ChangeLog 2019-03-20 20:26:18 UTC (rev 243233)
@@ -1,3 +1,15 @@
+2019-03-19 Ryosuke Niwa <[email protected]>
+
+ appendChild should throw when inserting an ancestor of a template into its content adopted to another document
+ https://bugs.webkit.org/show_bug.cgi?id=195984
+
+ Reviewed by Darin Adler.
+
+ Added a regression test.
+
+ * fast/dom/insert-template-parent-into-adopted-content-expected.txt: Added.
+ * fast/dom/insert-template-parent-into-adopted-content.html: Added.
+
2019-03-20 Simon Fraser <[email protected]>
Unreviewed test gardening. Fix the results for absolute-in-async-overflow-scroll.html.
Added: trunk/LayoutTests/fast/dom/insert-template-parent-into-adopted-content-expected.txt (0 => 243233)
--- trunk/LayoutTests/fast/dom/insert-template-parent-into-adopted-content-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/insert-template-parent-into-adopted-content-expected.txt 2019-03-20 20:26:18 UTC (rev 243233)
@@ -0,0 +1,13 @@
+This tests inserting the parent of a template element into its content document fragment
+after adopting the document fragment to another document. WebKit should throw HierarchyRequestError
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS template.content.appendChild(templateParent) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS template.content.insertBefore(templateParent, template.content.firstChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS template.content.replaceChild(templateParent, template.content.firstChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/insert-template-parent-into-adopted-content.html (0 => 243233)
--- trunk/LayoutTests/fast/dom/insert-template-parent-into-adopted-content.html (rev 0)
+++ trunk/LayoutTests/fast/dom/insert-template-parent-into-adopted-content.html 2019-03-20 20:26:18 UTC (rev 243233)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="templateParent"><template id="template"><span></span></template></div>
+<script src=""
+<script>
+
+description('This tests inserting the parent of a template element into its content document fragment<br>'
+ + 'after adopting the document fragment to another document. WebKit should throw HierarchyRequestError');
+
+const newDocument = document.implementation.createHTMLDocument();
+newDocument.adoptNode(template.content);
+shouldThrowErrorName('template.content.appendChild(templateParent)', 'HierarchyRequestError');
+shouldThrowErrorName('template.content.insertBefore(templateParent, template.content.firstChild)', 'HierarchyRequestError');
+shouldThrowErrorName('template.content.replaceChild(templateParent, template.content.firstChild)', 'HierarchyRequestError');
+
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (243232 => 243233)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2019-03-20 20:26:18 UTC (rev 243233)
@@ -1,3 +1,14 @@
+2019-03-19 Ryosuke Niwa <[email protected]>
+
+ appendChild should throw when inserting an ancestor of a template into its content adopted to another document
+ https://bugs.webkit.org/show_bug.cgi?id=195984
+
+ Reviewed by Darin Adler.
+
+ Rebaselined the test that is not fully passing.
+
+ * web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-content-hierarcy-expected.txt:
+
2019-03-20 Oriol Brufau <[email protected]>
[css-grid] Always consider baseline shim for the minimum contribution
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-content-hierarcy-expected.txt (243232 => 243233)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-content-hierarcy-expected.txt 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-content-hierarcy-expected.txt 2019-03-20 20:26:18 UTC (rev 243233)
@@ -1,6 +1,4 @@
PASS Template content should throw when its ancestor is being appended.
-FAIL Template content should throw exception when its ancestor in a different document but connected via host is being append. assert_throws: Template content should throw if any of ancestor is being appended. function "() => {
- tmpl.content.appendChild(parent);
- }" did not throw
+PASS Template content should throw exception when its ancestor in a different document but connected via host is being append.
Modified: trunk/Source/WebCore/ChangeLog (243232 => 243233)
--- trunk/Source/WebCore/ChangeLog 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/Source/WebCore/ChangeLog 2019-03-20 20:26:18 UTC (rev 243233)
@@ -1,3 +1,31 @@
+2019-03-19 Ryosuke Niwa <[email protected]>
+
+ appendChild should throw when inserting an ancestor of a template into its content adopted to another document
+ https://bugs.webkit.org/show_bug.cgi?id=195984
+
+ Reviewed by Darin Adler.
+
+ The WPT test caught a bug that appendChild and other DOM insertion functions were incorrectly assuming that
+ any node that's in a HTML template element has the current document's template document as its owner.
+ The assumption is wrong when the template element's content DocumentFragment is adopted to another document.
+
+ Fixed the bug by always checking the ancestor host elements in checkAcceptChild. Also
+
+ Test: fast/dom/insert-template-parent-into-adopted-content.html
+
+ * dom/ContainerNode.cpp:
+ (WebCore::isInTemplateContent): Deleted. This code is simply wrong.
+ (WebCore::containsConsideringHostElements): Deleted. Call sites are updated to use containsIncludingHostElements.
+ (WebCore::containsIncludingHostElements): Moved from Node.cpp and optimized this code a bit. It's more efficient
+ to get the parent node and check for ShadowRoot and DocumentFragment only when the parent is null than to check
+ for those two node types before getting the parent node.
+ (WebCore::checkAcceptChild): Merged two code paths to call containsIncludingHostElements. The early return for
+ a pseudo element is there only to prevent tree corruption in release build even in the presence of a major bug
+ so it shouldn't be an spec compliance issue.
+ * dom/Node.cpp:
+ (WebCore::Node::containsIncludingHostElements const): Deleted.
+ * dom/Node.h:
+
2019-03-20 Timothy Hatcher <[email protected]>
Unreviewed followup to r243169 to fix test failures.
Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (243232 => 243233)
--- trunk/Source/WebCore/dom/ContainerNode.cpp 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2019-03-20 20:26:18 UTC (rev 243233)
@@ -292,27 +292,34 @@
return true;
}
-static inline bool isInTemplateContent(const Node* node)
+static bool containsIncludingHostElements(const Node& possibleAncestor, const Node& node)
{
- Document& document = node->document();
- return &document == document.templateDocument();
-}
+ const Node* currentNode = &node;
+ do {
+ if (currentNode == &possibleAncestor)
+ return true;
+ const ContainerNode* parent = currentNode->parentNode();
+ if (!parent) {
+ if (is<ShadowRoot>(currentNode))
+ parent = downcast<ShadowRoot>(currentNode)->host();
+ else if (is<DocumentFragment>(*currentNode) && downcast<DocumentFragment>(*currentNode).isTemplateContent())
+ parent = static_cast<const TemplateContentDocumentFragment*>(currentNode)->host();
+ }
+ currentNode = parent;
+ } while (currentNode);
-static inline bool containsConsideringHostElements(const Node& newChild, const Node& newParent)
-{
- return (newParent.isInShadowTree() || isInTemplateContent(&newParent))
- ? newChild.containsIncludingHostElements(&newParent)
- : newChild.contains(&newParent);
+ return false;
}
static inline ExceptionOr<void> checkAcceptChild(ContainerNode& newParent, Node& newChild, const Node* refChild, Document::AcceptChildOperation operation)
{
+ if (containsIncludingHostElements(newChild, newParent))
+ return Exception { HierarchyRequestError };
+
// Use common case fast path if possible.
if ((newChild.isElementNode() || newChild.isTextNode()) && newParent.isElementNode()) {
ASSERT(!newParent.isDocumentTypeNode());
ASSERT(isChildTypeAllowed(newParent, newChild));
- if (containsConsideringHostElements(newChild, newParent))
- return Exception { HierarchyRequestError };
if (operation == Document::AcceptChildOperation::InsertOrAdd && refChild && refChild->parentNode() != &newParent)
return Exception { NotFoundError };
return { };
@@ -323,9 +330,6 @@
if (newChild.isPseudoElement())
return Exception { HierarchyRequestError };
- if (containsConsideringHostElements(newChild, newParent))
- return Exception { HierarchyRequestError };
-
if (operation == Document::AcceptChildOperation::InsertOrAdd && refChild && refChild->parentNode() != &newParent)
return Exception { NotFoundError };
@@ -342,7 +346,7 @@
{
ASSERT(!newParent.isDocumentTypeNode());
ASSERT(isChildTypeAllowed(newParent, newChild));
- if (containsConsideringHostElements(newChild, newParent))
+ if (containsIncludingHostElements(newChild, newParent))
return Exception { HierarchyRequestError };
return { };
}
Modified: trunk/Source/WebCore/dom/Node.cpp (243232 => 243233)
--- trunk/Source/WebCore/dom/Node.cpp 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/Source/WebCore/dom/Node.cpp 2019-03-20 20:26:18 UTC (rev 243233)
@@ -1029,19 +1029,6 @@
return false;
}
-bool Node::containsIncludingHostElements(const Node* node) const
-{
- while (node) {
- if (node == this)
- return true;
- if (is<DocumentFragment>(*node) && downcast<DocumentFragment>(*node).isTemplateContent())
- node = static_cast<const TemplateContentDocumentFragment*>(node)->host();
- else
- node = node->parentOrShadowHostNode();
- }
- return false;
-}
-
Node* Node::pseudoAwarePreviousSibling() const
{
Element* parentOrHost = is<PseudoElement>(*this) ? downcast<PseudoElement>(*this).hostElement() : parentElement();
Modified: trunk/Source/WebCore/dom/Node.h (243232 => 243233)
--- trunk/Source/WebCore/dom/Node.h 2019-03-20 20:24:36 UTC (rev 243232)
+++ trunk/Source/WebCore/dom/Node.h 2019-03-20 20:26:18 UTC (rev 243233)
@@ -392,7 +392,6 @@
bool isDescendantOrShadowDescendantOf(const Node*) const;
WEBCORE_EXPORT bool contains(const Node*) const;
bool containsIncludingShadowDOM(const Node*) const;
- bool containsIncludingHostElements(const Node*) const;
// Number of DOM 16-bit units contained in node. Note that rendered text length can be different - e.g. because of
// css-transform:capitalize breaking up precomposed characters and ligatures.
