Skip to content

Commit bbb96d6

Browse files
committed
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 :^)
1 parent 43d2c92 commit bbb96d6

File tree

5 files changed

+48
-4
lines changed

5 files changed

+48
-4
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
1
2+
<H2 >
3+
2
4+
<H2 >
5+
2
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<script src="../include.js"></script>
2+
<script>
3+
test(() => {
4+
const h2 = document.querySelector("h2");
5+
document.getSelection().collapse(h2, 1);
6+
var h3 = document.createElement("h3");
7+
h2.insertBefore(h3, h2.firstChild)
8+
println(document.getSelection().rangeCount);
9+
printElement(document.getSelection().getRangeAt(0).startContainer);
10+
println(document.getSelection().getRangeAt(0).startOffset);
11+
printElement(document.getSelection().getRangeAt(0).endContainer);
12+
println(document.getSelection().getRangeAt(0).endOffset);
13+
});
14+
</script><h2>

Userland/Libraries/LibWeb/DOM/Node.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -427,13 +427,13 @@ void Node::insert_before(JS::NonnullGCPtr<Node> node, JS::GCPtr<Node> child, boo
427427
// 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.
428428
for (auto& range : Range::live_ranges()) {
429429
if (range->start_container() == this && range->start_offset() > child->index())
430-
MUST(range->set_start(*range->start_container(), range->start_offset() + count));
430+
range->increase_start_offset({}, count);
431431
}
432432

433433
// 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.
434434
for (auto& range : Range::live_ranges()) {
435435
if (range->end_container() == this && range->end_offset() > child->index())
436-
MUST(range->set_end(*range->end_container(), range->end_offset() + count));
436+
range->increase_end_offset({}, count);
437437
}
438438
}
439439

@@ -601,13 +601,13 @@ void Node::remove(bool suppress_observers)
601601
// 6. For each live range whose start node is parent and start offset is greater than index, decrease its start offset by 1.
602602
for (auto& range : Range::live_ranges()) {
603603
if (range->start_container() == parent && range->start_offset() > index)
604-
MUST(range->set_start(*range->start_container(), range->start_offset() - 1));
604+
range->decrease_start_offset({}, 1);
605605
}
606606

607607
// 7. For each live range whose end node is parent and end offset is greater than index, decrease its end offset by 1.
608608
for (auto& range : Range::live_ranges()) {
609609
if (range->end_container() == parent && range->end_offset() > index)
610-
MUST(range->set_end(*range->end_container(), range->end_offset() - 1));
610+
range->decrease_end_offset({}, 1);
611611
}
612612

613613
// 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.

Userland/Libraries/LibWeb/DOM/Range.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,4 +1230,24 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<DocumentFragment>> Range::create_contextual
12301230
return fragment_node;
12311231
}
12321232

1233+
void Range::increase_start_offset(Badge<Node>, WebIDL::UnsignedLong count)
1234+
{
1235+
m_start_offset += count;
1236+
}
1237+
1238+
void Range::increase_end_offset(Badge<Node>, WebIDL::UnsignedLong count)
1239+
{
1240+
m_end_offset += count;
1241+
}
1242+
1243+
void Range::decrease_start_offset(Badge<Node>, WebIDL::UnsignedLong count)
1244+
{
1245+
m_start_offset -= count;
1246+
}
1247+
1248+
void Range::decrease_end_offset(Badge<Node>, WebIDL::UnsignedLong count)
1249+
{
1250+
m_end_offset -= count;
1251+
}
1252+
12331253
}

Userland/Libraries/LibWeb/DOM/Range.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ class Range final : public AbstractRange {
4747
void collapse(bool to_start);
4848
WebIDL::ExceptionOr<void> select_node_contents(Node&);
4949

50+
void increase_start_offset(Badge<Node>, WebIDL::UnsignedLong);
51+
void increase_end_offset(Badge<Node>, WebIDL::UnsignedLong);
52+
void decrease_start_offset(Badge<Node>, WebIDL::UnsignedLong);
53+
void decrease_end_offset(Badge<Node>, WebIDL::UnsignedLong);
54+
5055
// https://dom.spec.whatwg.org/#dom-range-start_to_start
5156
enum HowToCompareBoundaryPoints : WebIDL::UnsignedShort {
5257
START_TO_START = 0,

0 commit comments

Comments
 (0)