dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LibWeb: Invalidate layout tree of parent when replacing CharacterData

Browse source 
When replacing the data in `CharacterData`, we were only invalidating
text-dependent cache and layout of the layout node(s) corresponding to
that `CharacterData`. This is not sufficient: With `::first-letter`
styling, `TextSliceNode`s are being created for the first letter and the
remainder of the text. So, contrary to regular `TextNode`s, these store
offsets into the text data determined while building the layout tree.
Subsequently these become stale when only invalidating layout, but not
the layout tree. Using such stale offsets with updated text data leads
to crashes in `TextNode::compute_text_for_rendering()` if the text got
shorter and incorrect behavior if it got longer.

To fix this, invalidate the layout tree of the parent of the affected
`TextSliceNode`s, causing a rebuild that creates new slice nodes with
the updated data.

This fixes a crash when typing in the search box on
https://search.brave.com/ where a first-letter style is used to
capitalize the first letter of the search suggestions descriptions. This
crash was a regression from b67d73a661.
This commit is contained in:
InvalidUsernameException 2026-06-01 13:32:37 +02:00 committed by Tim Ledbetter
parent ece7b115f4
commit 4ae31dc777
4 changed files with 33 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
Libraries/LibWeb/DOM/CharacterData.cpp
View file

@ -140,17 +140,21 @@ WebIDL::ExceptionOr<void> CharacterData::replace_data(size_t offset, size_t coun
return {};
// NB: Called during DOM text mutation, layout is stale.
if (auto* text = as_if<Text>(*this)) {
Layout::TextOffsetMapping mapping { *text };
mapping.for_each_fragment([](Layout::TextNode& slice) {
if (is<Text>(*this)) {
if (is<Layout::TextSliceNode>(unsafe_layout_node())) {
// NB: Slice nodes cache data that is calculated at layout tree construction time.
// So for them, we need to invalidate the layout tree, not just layout.
if (parent())
parent()->set_needs_layout_tree_update(true, SetNeedsLayoutTreeUpdateReason::CharacterDataReplaceData);
} else if (auto* text_layout_node = as_if<Layout::TextNode>(unsafe_layout_node())) {
// NB: Since the text node's data has changed, we need to invalidate the text for rendering.
// This ensures that the new text is reflected in layout, even if we don't end up doing a full layout
// tree rebuild.
slice.invalidate_text_for_rendering();
text_layout_node->invalidate_text_for_rendering();
// We also need to relayout.
slice.set_needs_layout_update(SetNeedsLayoutReason::CharacterDataReplaceData);
});
text_layout_node->set_needs_layout_update(SetNeedsLayoutReason::CharacterDataReplaceData);
}
}
document().bump_character_data_version();

1
Libraries/LibWeb/DOM/Node.h
View file

@ -82,6 +82,7 @@ enum class SetNeedsLayoutReason {
[[nodiscard]] StringView to_string(SetNeedsLayoutReason);
#define ENUMERATE_SET_NEEDS_LAYOUT_TREE_UPDATE_REASONS(X) \
X(CharacterDataReplaceData) \
X(ElementSetInnerHTML) \
X(ElementSetShadowRoot) \
X(DetailsElementOpenedOrClosed) \

2
Tests/LibWeb/Ref/expected/modify-nodeValue-of-first-letter-styled-text-ref.html Normal file
View file

@ -0,0 +1,2 @@
<!DOCTYPE html>
<p>Short text</p>

20
Tests/LibWeb/Ref/input/modify-nodeValue-of-first-letter-styled-text.html Normal file
View file

@ -0,0 +1,20 @@
<!DOCTYPE html>
<html class="reftest-wait">
<link rel="match" href="../expected/modify-nodeValue-of-first-letter-styled-text-ref.html" />
<style>
#text:first-letter {
text-transform: capitalize;
}
</style>
<p id="text">this is some very long text. Well, actually not all that long, but long enough for our purposes.</p>
<script>
window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
setTimeout(() => {
document.getElementById("text").firstChild.nodeValue = "short text";
document.documentElement.classList.remove("reftest-wait");
}, 0);
});
});
</script>
</html>