Skip to content

Commit

Permalink
LibWeb: Fix infinite loop in ChildNode's before() and after()
Browse files Browse the repository at this point in the history
The loop that was supposed to check the chain of previous or next
siblings had a logic mistake where it would never traverse the chain,
so we would get stuck looking at the immediate sibling forever.
  • Loading branch information
awesomekling committed Mar 11, 2024
1 parent ad843b6 commit 35f359c
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 11 deletions.
@@ -0,0 +1,5 @@
<DIV id="one" >
<DIV id="two" >
<DIV id="two" >
<DIV id="one" >
PASS (didn't crash)
@@ -0,0 +1,5 @@
<DIV id="one" >
<DIV id="two" >
<DIV id="two" >
<DIV id="one" >
PASS (didn't crash)
13 changes: 13 additions & 0 deletions Tests/LibWeb/Text/input/DOM/ChildNode-after-next-sibling.html
@@ -0,0 +1,13 @@
<script src="../include.js"></script>
<div id="one"></div><div id="two"></div><script>
test(() => {
let one = document.getElementById("one");
let two = document.getElementById("two");
one.after(two);
printElement(one);
printElement(one.nextSibling);
printElement(two);
printElement(two.previousSibling);
println("PASS (didn't crash)");
});
</script>
13 changes: 13 additions & 0 deletions Tests/LibWeb/Text/input/DOM/ChildNode-before-previous-sibling.html
@@ -0,0 +1,13 @@
<script src="../include.js"></script>
<div id="one"></div><div id="two"></div><script>
test(() => {
let one = document.getElementById("one");
let two = document.getElementById("two");
two.before(one);
printElement(one);
printElement(one.nextSibling);
printElement(two);
printElement(two.previousSibling);
println("PASS (didn't crash)");
});
</script>
22 changes: 11 additions & 11 deletions Userland/Libraries/LibWeb/DOM/ChildNode.h
Expand Up @@ -58,7 +58,7 @@ class ChildNode {
return {};

// 3. Let viableNextSibling be this’s first following sibling not in nodes; otherwise null.
auto viable_next_sibling = viable_nest_sibling_for_insertion(nodes);
auto viable_next_sibling = viable_next_sibling_for_insertion(nodes);

// 4. Let node be the result of converting nodes into a node, given nodes and this’s node document.
auto node_to_insert = TRY(convert_nodes_to_single_node(nodes, node->document()));
Expand All @@ -82,7 +82,7 @@ class ChildNode {
return {};

// 3. Let viableNextSibling be this’s first following sibling not in nodes; otherwise null.
auto viable_next_sibling = viable_nest_sibling_for_insertion(nodes);
auto viable_next_sibling = viable_next_sibling_for_insertion(nodes);

// 4. Let node be the result of converting nodes into a node, given nodes and this’s node document.
auto node_to_insert = TRY(convert_nodes_to_single_node(nodes, node->document()));
Expand Down Expand Up @@ -121,47 +121,47 @@ class ChildNode {
{
auto* node = static_cast<NodeType*>(this);

while (auto* previous_sibling = node->previous_sibling()) {
for (auto* sibling = node->previous_sibling(); sibling; sibling = sibling->previous_sibling()) {
bool contained_in_nodes = false;

for (auto const& node_or_string : nodes) {
if (!node_or_string.template has<JS::Handle<Node>>())
continue;

auto node_in_vector = node_or_string.template get<JS::Handle<Node>>();
if (node_in_vector.cell() == previous_sibling) {
auto const& node_in_vector = node_or_string.template get<JS::Handle<Node>>();
if (node_in_vector.cell() == sibling) {
contained_in_nodes = true;
break;
}
}

if (!contained_in_nodes)
return previous_sibling;
return sibling;
}

return nullptr;
}

JS::GCPtr<Node> viable_nest_sibling_for_insertion(Vector<Variant<JS::Handle<Node>, String>> const& nodes)
JS::GCPtr<Node> viable_next_sibling_for_insertion(Vector<Variant<JS::Handle<Node>, String>> const& nodes)
{
auto* node = static_cast<NodeType*>(this);

while (auto* next_sibling = node->next_sibling()) {
for (auto* sibling = node->next_sibling(); sibling; sibling = sibling->next_sibling()) {
bool contained_in_nodes = false;

for (auto const& node_or_string : nodes) {
if (!node_or_string.template has<JS::Handle<Node>>())
continue;

auto& node_in_vector = node_or_string.template get<JS::Handle<Node>>();
if (node_in_vector.cell() == next_sibling) {
auto const& node_in_vector = node_or_string.template get<JS::Handle<Node>>();
if (node_in_vector.cell() == sibling) {
contained_in_nodes = true;
break;
}
}

if (!contained_in_nodes)
return next_sibling;
return sibling;
}

return nullptr;
Expand Down

0 comments on commit 35f359c

Please sign in to comment.