Explorar el Código

LibWeb: Don't crash on live range offset update during node insertion

When inserting a node into a parent, any live DOM ranges that reference
the parent may need to be updated. The spec does this by increasing or
decreasing the start/end offsets of each live range *before* actually
performing the insertion.

This caused us to crash with a verification failure, since it was
possible to set the range offset to an invalid value (that would go on
to immediately become valid after the insertion was finished).

This patch fixes the issue by adding special badged helpers on Range for
Node to reach into it and increase/decrease the offsets during node
insertion. This skips the offset validity check and actually makes our
code read slightly more like the spec.

Found by Domato :^)
Andreas Kling hace 1 año
padre
commit
bbb96d65b1

+ 5 - 0
Tests/LibWeb/Text/expected/DOM/update-live-ranges-on-node-insertion.txt

@@ -0,0 +1,5 @@
+ 1
+<H2 >
+2
+<H2 >
+2

+ 14 - 0
Tests/LibWeb/Text/input/DOM/update-live-ranges-on-node-insertion.html

@@ -0,0 +1,14 @@
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        const h2 = document.querySelector("h2");
+        document.getSelection().collapse(h2, 1);
+        var h3 = document.createElement("h3");
+        h2.insertBefore(h3, h2.firstChild)
+        println(document.getSelection().rangeCount);
+        printElement(document.getSelection().getRangeAt(0).startContainer);
+        println(document.getSelection().getRangeAt(0).startOffset);
+        printElement(document.getSelection().getRangeAt(0).endContainer);
+        println(document.getSelection().getRangeAt(0).endOffset);
+    });
+</script><h2>

+ 4 - 4
Userland/Libraries/LibWeb/DOM/Node.cpp

@@ -427,13 +427,13 @@ void Node::insert_before(JS::NonnullGCPtr<Node> node, JS::GCPtr<Node> child, boo
         // 1. For each live range whose start node is parent and start offset is greater than child’s index, increase its start offset by count.
         for (auto& range : Range::live_ranges()) {
             if (range->start_container() == this && range->start_offset() > child->index())
-                MUST(range->set_start(*range->start_container(), range->start_offset() + count));
+                range->increase_start_offset({}, count);
         }
 
         // 2. For each live range whose end node is parent and end offset is greater than child’s index, increase its end offset by count.
         for (auto& range : Range::live_ranges()) {
             if (range->end_container() == this && range->end_offset() > child->index())
-                MUST(range->set_end(*range->end_container(), range->end_offset() + count));
+                range->increase_end_offset({}, count);
         }
     }
 
@@ -601,13 +601,13 @@ void Node::remove(bool suppress_observers)
     // 6. For each live range whose start node is parent and start offset is greater than index, decrease its start offset by 1.
     for (auto& range : Range::live_ranges()) {
         if (range->start_container() == parent && range->start_offset() > index)
-            MUST(range->set_start(*range->start_container(), range->start_offset() - 1));
+            range->decrease_start_offset({}, 1);
     }
 
     // 7. For each live range whose end node is parent and end offset is greater than index, decrease its end offset by 1.
     for (auto& range : Range::live_ranges()) {
         if (range->end_container() == parent && range->end_offset() > index)
-            MUST(range->set_end(*range->end_container(), range->end_offset() - 1));
+            range->decrease_end_offset({}, 1);
     }
 
     // 8. For each NodeIterator object iterator whose root’s node document is node’s node document, run the NodeIterator pre-removing steps given node and iterator.

+ 20 - 0
Userland/Libraries/LibWeb/DOM/Range.cpp

@@ -1230,4 +1230,24 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<DocumentFragment>> Range::create_contextual
     return fragment_node;
 }
 
+void Range::increase_start_offset(Badge<Node>, WebIDL::UnsignedLong count)
+{
+    m_start_offset += count;
+}
+
+void Range::increase_end_offset(Badge<Node>, WebIDL::UnsignedLong count)
+{
+    m_end_offset += count;
+}
+
+void Range::decrease_start_offset(Badge<Node>, WebIDL::UnsignedLong count)
+{
+    m_start_offset -= count;
+}
+
+void Range::decrease_end_offset(Badge<Node>, WebIDL::UnsignedLong count)
+{
+    m_end_offset -= count;
+}
+
 }

+ 5 - 0
Userland/Libraries/LibWeb/DOM/Range.h

@@ -47,6 +47,11 @@ public:
     void collapse(bool to_start);
     WebIDL::ExceptionOr<void> select_node_contents(Node&);
 
+    void increase_start_offset(Badge<Node>, WebIDL::UnsignedLong);
+    void increase_end_offset(Badge<Node>, WebIDL::UnsignedLong);
+    void decrease_start_offset(Badge<Node>, WebIDL::UnsignedLong);
+    void decrease_end_offset(Badge<Node>, WebIDL::UnsignedLong);
+
     // https://dom.spec.whatwg.org/#dom-range-start_to_start
     enum HowToCompareBoundaryPoints : WebIDL::UnsignedShort {
         START_TO_START = 0,