From bbb96d65b1ab108f9f1bab571f9ecf46b218aa18 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 12 Mar 2024 13:08:10 +0100 Subject: [PATCH] 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 :^) --- .../update-live-ranges-on-node-insertion.txt | 5 +++++ .../update-live-ranges-on-node-insertion.html | 14 +++++++++++++ Userland/Libraries/LibWeb/DOM/Node.cpp | 8 ++++---- Userland/Libraries/LibWeb/DOM/Range.cpp | 20 +++++++++++++++++++ Userland/Libraries/LibWeb/DOM/Range.h | 5 +++++ 5 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/DOM/update-live-ranges-on-node-insertion.txt create mode 100644 Tests/LibWeb/Text/input/DOM/update-live-ranges-on-node-insertion.html diff --git a/Tests/LibWeb/Text/expected/DOM/update-live-ranges-on-node-insertion.txt b/Tests/LibWeb/Text/expected/DOM/update-live-ranges-on-node-insertion.txt new file mode 100644 index 0000000000..0d1d20b1c4 --- /dev/null +++ b/Tests/LibWeb/Text/expected/DOM/update-live-ranges-on-node-insertion.txt @@ -0,0 +1,5 @@ + 1 +

+2 +

+2 diff --git a/Tests/LibWeb/Text/input/DOM/update-live-ranges-on-node-insertion.html b/Tests/LibWeb/Text/input/DOM/update-live-ranges-on-node-insertion.html new file mode 100644 index 0000000000..8153a93e2e --- /dev/null +++ b/Tests/LibWeb/Text/input/DOM/update-live-ranges-on-node-insertion.html @@ -0,0 +1,14 @@ + +

diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 9c6bfc6493..a421847411 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -427,13 +427,13 @@ void Node::insert_before(JS::NonnullGCPtr node, JS::GCPtr 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. diff --git a/Userland/Libraries/LibWeb/DOM/Range.cpp b/Userland/Libraries/LibWeb/DOM/Range.cpp index 75183ddaaf..ef13773e1f 100644 --- a/Userland/Libraries/LibWeb/DOM/Range.cpp +++ b/Userland/Libraries/LibWeb/DOM/Range.cpp @@ -1230,4 +1230,24 @@ WebIDL::ExceptionOr> Range::create_contextual return fragment_node; } +void Range::increase_start_offset(Badge, WebIDL::UnsignedLong count) +{ + m_start_offset += count; +} + +void Range::increase_end_offset(Badge, WebIDL::UnsignedLong count) +{ + m_end_offset += count; +} + +void Range::decrease_start_offset(Badge, WebIDL::UnsignedLong count) +{ + m_start_offset -= count; +} + +void Range::decrease_end_offset(Badge, WebIDL::UnsignedLong count) +{ + m_end_offset -= count; +} + } diff --git a/Userland/Libraries/LibWeb/DOM/Range.h b/Userland/Libraries/LibWeb/DOM/Range.h index 239bfc246d..0fc2461af2 100644 --- a/Userland/Libraries/LibWeb/DOM/Range.h +++ b/Userland/Libraries/LibWeb/DOM/Range.h @@ -47,6 +47,11 @@ public: void collapse(bool to_start); WebIDL::ExceptionOr select_node_contents(Node&); + void increase_start_offset(Badge, WebIDL::UnsignedLong); + void increase_end_offset(Badge, WebIDL::UnsignedLong); + void decrease_start_offset(Badge, WebIDL::UnsignedLong); + void decrease_end_offset(Badge, WebIDL::UnsignedLong); + // https://dom.spec.whatwg.org/#dom-range-start_to_start enum HowToCompareBoundaryPoints : WebIDL::UnsignedShort { START_TO_START = 0,