Skip to content

Commit

Permalink
LibWeb: Don't crash on live range offset update during node insertion
Browse files Browse the repository at this point in the history
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 :^)
  • Loading branch information
awesomekling committed Mar 12, 2024
1 parent a98b229 commit 8e00e7f
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
1
<H2 >
2
<H2 >
2
Original file line number Diff line number Diff line change
@@ -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>
8 changes: 4 additions & 4 deletions Userland/Libraries/LibWeb/DOM/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions Userland/Libraries/LibWeb/DOM/Range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 5 additions & 0 deletions Userland/Libraries/LibWeb/DOM/Range.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ class Range final : public AbstractRange {
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,
Expand Down

0 comments on commit 8e00e7f

Please sign in to comment.