From 4203b7823f2973a1d831b7faa657bd1565482f97 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 20 Nov 2024 13:00:56 +0100 Subject: [PATCH] LibWeb: Fix incorrect exception on replaceChild() with doctypes We were checking for presence of the wrong child in the parent. --- Libraries/LibWeb/DOM/Node.cpp | 2 +- .../dom/nodes/Node-replaceChild.txt | 39 ++ .../dom/nodes/Node-replaceChild.html | 349 ++++++++++++++++++ .../pre-insertion-validation-notfound.js | 108 ++++++ 4 files changed, 497 insertions(+), 1 deletion(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/dom/nodes/Node-replaceChild.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/dom/nodes/Node-replaceChild.html create mode 100644 Tests/LibWeb/Text/input/wpt-import/dom/nodes/pre-insertion-validation-notfound.js diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index a08979e5950..bc267ef7298 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -945,7 +945,7 @@ WebIDL::ExceptionOr> Node::replace_child(GC::Ref node, GC::R } else if (is(*node)) { // DocumentType // parent has a doctype child that is not child, or an element is preceding child. - if (first_child_of_type() != node || child->has_preceding_node_of_type_in_tree_order()) + if (first_child_of_type() != child || child->has_preceding_node_of_type_in_tree_order()) return WebIDL::HierarchyRequestError::create(realm(), "Invalid node type for insertion"_string); } } diff --git a/Tests/LibWeb/Text/expected/wpt-import/dom/nodes/Node-replaceChild.txt b/Tests/LibWeb/Text/expected/wpt-import/dom/nodes/Node-replaceChild.txt new file mode 100644 index 00000000000..135dd97a58b --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/dom/nodes/Node-replaceChild.txt @@ -0,0 +1,39 @@ +Summary + +Harness status: OK + +Rerun + +Found 29 tests + +29 Pass +Details +Result Test Name MessagePass Should check the 'parent' type before checking whether 'child' is a child of 'parent' +Pass Should check that 'node' is not an ancestor of 'parent' before checking whether 'child' is a child of 'parent' +Pass Should check whether 'child' is a child of 'parent' before checking whether 'node' is of a type that can have a parent. +Pass Should check whether 'child' is a child of 'parent' before checking whether 'node' is of a type that can have a parent of the type that 'parent' is. +Pass Should check whether 'child' is a child of 'parent' before checking whether 'node' can be inserted into the document given the kids the document has right now. +Pass Passing null to replaceChild should throw a TypeError. +Pass If child's parent is not the context node, a NotFoundError exception should be thrown +Pass If the context node is not a node that can contain children, a HierarchyRequestError exception should be thrown +Pass If node is an inclusive ancestor of the context node, a HierarchyRequestError should be thrown. +Pass If the context node is a document, inserting a document or text node should throw a HierarchyRequestError. +Pass If the context node is a document, inserting a DocumentFragment that contains a text node or too many elements should throw a HierarchyRequestError. +Pass If the context node is a document (without element children), inserting a DocumentFragment that contains multiple elements should throw a HierarchyRequestError. +Pass If the context node is a document, inserting a DocumentFragment with an element if there already is an element child should throw a HierarchyRequestError. +Pass If the context node is a document, inserting a DocumentFragment with an element before the doctype should throw a HierarchyRequestError. +Pass If the context node is a document, inserting an element if there already is an element child should throw a HierarchyRequestError. +Pass If the context node is a document, inserting an element before the doctype should throw a HierarchyRequestError. +Pass If the context node is a document, inserting a doctype if there already is a doctype child should throw a HierarchyRequestError. +Pass If the context node is a document, inserting a doctype after the document element should throw a HierarchyRequestError. +Pass If the context node is a DocumentFragment, inserting a document or a doctype should throw a HierarchyRequestError. +Pass If the context node is an element, inserting a document or a doctype should throw a HierarchyRequestError. +Pass Replacing a node with its next sibling should work (2 children) +Pass Replacing a node with its next sibling should work (4 children) +Pass Replacing a node with itself should not move the node +Pass If the context node is a document, inserting a new doctype should work. +Pass Replacing the document element with a DocumentFragment containing a single element should work. +Pass Replacing the document element with a DocumentFragment containing a single element and comments should work. +Pass Replacing the document element with a single element should work. +Pass replaceChild should work in the presence of mutation events. +Pass Replacing an element with a DocumentFragment should allow a child of the DocumentFragment to be found by Id. \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/dom/nodes/Node-replaceChild.html b/Tests/LibWeb/Text/input/wpt-import/dom/nodes/Node-replaceChild.html new file mode 100644 index 00000000000..7b2188dbbb9 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/dom/nodes/Node-replaceChild.html @@ -0,0 +1,349 @@ + + +Node.replaceChild + + + +
+ + + + diff --git a/Tests/LibWeb/Text/input/wpt-import/dom/nodes/pre-insertion-validation-notfound.js b/Tests/LibWeb/Text/input/wpt-import/dom/nodes/pre-insertion-validation-notfound.js new file mode 100644 index 00000000000..705283fa235 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/dom/nodes/pre-insertion-validation-notfound.js @@ -0,0 +1,108 @@ +function getNonParentNodes() { + return [ + document.implementation.createDocumentType("html", "", ""), + document.createTextNode("text"), + document.implementation.createDocument(null, "foo", null).createProcessingInstruction("foo", "bar"), + document.createComment("comment"), + document.implementation.createDocument(null, "foo", null).createCDATASection("data"), + ]; +} + +function getNonInsertableNodes() { + return [ + document.implementation.createHTMLDocument("title") + ]; +} + +function getNonDocumentParentNodes() { + return [ + document.createElement("div"), + document.createDocumentFragment(), + ]; +} + +// Test that the steps happen in the right order, to the extent that it's +// observable. The variable names "parent", "child", and "node" match the +// corresponding variables in the replaceChild algorithm in these tests. + +// Step 1 happens before step 3. +test(function() { + var illegalParents = getNonParentNodes(); + var child = document.createElement("div"); + var node = document.createElement("div"); + illegalParents.forEach(function (parent) { + assert_throws_dom("HierarchyRequestError", function() { + insertFunc.call(parent, node, child); + }); + }); +}, "Should check the 'parent' type before checking whether 'child' is a child of 'parent'"); + +// Step 2 happens before step 3. +test(function() { + var parent = document.createElement("div"); + var child = document.createElement("div"); + var node = document.createElement("div"); + + node.appendChild(parent); + assert_throws_dom("HierarchyRequestError", function() { + insertFunc.call(parent, node, child); + }); +}, "Should check that 'node' is not an ancestor of 'parent' before checking whether 'child' is a child of 'parent'"); + +// Step 3 happens before step 4. +test(function() { + var parent = document.createElement("div"); + var child = document.createElement("div"); + + var illegalChildren = getNonInsertableNodes(); + illegalChildren.forEach(function (node) { + assert_throws_dom("NotFoundError", function() { + insertFunc.call(parent, node, child); + }); + }); +}, "Should check whether 'child' is a child of 'parent' before checking whether 'node' is of a type that can have a parent."); + + +// Step 3 happens before step 5. +test(function() { + var child = document.createElement("div"); + + var node = document.createTextNode(""); + var parent = document.implementation.createDocument(null, "foo", null); + assert_throws_dom("NotFoundError", function() { + insertFunc.call(parent, node, child); + }); + + node = document.implementation.createDocumentType("html", "", ""); + getNonDocumentParentNodes().forEach(function (parent) { + assert_throws_dom("NotFoundError", function() { + insertFunc.call(parent, node, child); + }); + }); +}, "Should check whether 'child' is a child of 'parent' before checking whether 'node' is of a type that can have a parent of the type that 'parent' is."); + +// Step 3 happens before step 6. +test(function() { + var child = document.createElement("div"); + var parent = document.implementation.createDocument(null, null, null); + + var node = document.createDocumentFragment(); + node.appendChild(document.createElement("div")); + node.appendChild(document.createElement("div")); + assert_throws_dom("NotFoundError", function() { + insertFunc.call(parent, node, child); + }); + + node = document.createElement("div"); + parent.appendChild(document.createElement("div")); + assert_throws_dom("NotFoundError", function() { + insertFunc.call(parent, node, child); + }); + + parent.firstChild.remove(); + parent.appendChild(document.implementation.createDocumentType("html", "", "")); + node = document.implementation.createDocumentType("html", "", "") + assert_throws_dom("NotFoundError", function() { + insertFunc.call(parent, node, child); + }); +}, "Should check whether 'child' is a child of 'parent' before checking whether 'node' can be inserted into the document given the kids the document has right now.");