Prechádzať zdrojové kódy

LibWeb: Fix infinite loop in ChildNode's before() and after()

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.
Andreas Kling 1 rok pred
rodič
commit
35f359c51c

+ 5 - 0
Tests/LibWeb/Text/expected/DOM/ChildNode-after-next-sibling.txt

@@ -0,0 +1,5 @@
+ <DIV id="one" >
+<DIV id="two" >
+<DIV id="two" >
+<DIV id="one" >
+PASS (didn't crash)

+ 5 - 0
Tests/LibWeb/Text/expected/DOM/ChildNode-before-previous-sibling.txt

@@ -0,0 +1,5 @@
+ <DIV id="one" >
+<DIV id="two" >
+<DIV id="two" >
+<DIV id="one" >
+PASS (didn't crash)

+ 13 - 0
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 - 0
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>

+ 11 - 11
Userland/Libraries/LibWeb/DOM/ChildNode.h

@@ -58,7 +58,7 @@ public:
             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()));
@@ -82,7 +82,7 @@ public:
             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()));
@@ -121,47 +121,47 @@ private:
     {
         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;