mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-07-28 11:49:44 +00:00
LibWeb: Skip update_associated_selection()
when there's no selection
This change fixes at least two issues: - `update_associated_selection()` is responsible for selectionchange dispatch, and we were incorrectly dispatching this event on ranges that were not associated with a selection. - `Range::get_client_rects()` was using `update_associated_selection()` to refresh the selection state in the paintable tree for the current range, but since a range might not be associated with a selection, this could make the painted selection reflect the state of an arbitrary range instead of the actual selection range. Fixes a bug on Discord where any text typed into the message input would get selected.
This commit is contained in:
parent
69074a3841
commit
5874b7a76f
Notes:
github-actions[bot]
2025-07-03 20:17:46 +00:00
Author: https://github.com/kalenikaliaksandr
Commit: 5874b7a76f
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5292
Reviewed-by: https://github.com/gmta ✅
5 changed files with 73 additions and 12 deletions
|
@ -3,6 +3,7 @@
|
|||
* Copyright (c) 2022, Luke Wilde <lukew@serenityos.org>
|
||||
* Copyright (c) 2022-2023, Andreas Kling <andreas@ladybird.org>
|
||||
* Copyright (c) 2024-2025, Jelle Raaijmakers <jelle@ladybird.org>
|
||||
* Copyright (c) 2024-2025, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
||||
*
|
||||
* SPDX-License-Identifier: BSD-2-Clause
|
||||
*/
|
||||
|
@ -99,6 +100,9 @@ void Range::set_associated_selection(Badge<Selection::Selection>, GC::Ptr<Select
|
|||
|
||||
void Range::update_associated_selection()
|
||||
{
|
||||
if (!m_associated_selection)
|
||||
return;
|
||||
|
||||
auto& document = m_start_container->document();
|
||||
if (auto* viewport = document.paintable()) {
|
||||
viewport->recompute_selection_states(*this);
|
||||
|
@ -1143,7 +1147,6 @@ GC::Ref<Geometry::DOMRectList> Range::get_client_rects()
|
|||
return Geometry::DOMRectList::create(realm(), {});
|
||||
|
||||
start_container()->document().update_layout(DOM::UpdateLayoutReason::RangeGetClientRects);
|
||||
update_associated_selection();
|
||||
Vector<GC::Root<Geometry::DOMRect>> rects;
|
||||
// FIXME: take Range collapsed into consideration
|
||||
// 2. Iterate the node included in Range
|
||||
|
@ -1159,6 +1162,15 @@ GC::Ref<Geometry::DOMRectList> Range::get_client_rects()
|
|||
end_node = *end_node->child_at_index(m_end_offset - 1);
|
||||
}
|
||||
for (GC::Ptr<Node> node = start_node; node && node != end_node->next_in_pre_order(); node = node->next_in_pre_order()) {
|
||||
auto selection_state = Painting::Paintable::SelectionState::Full;
|
||||
if (node == start_node && node == end_node) {
|
||||
selection_state = Painting::Paintable::SelectionState::StartAndEnd;
|
||||
} else if (node == start_node) {
|
||||
selection_state = Painting::Paintable::SelectionState::Start;
|
||||
} else if (node == end_node) {
|
||||
selection_state = Painting::Paintable::SelectionState::End;
|
||||
}
|
||||
|
||||
auto node_type = static_cast<NodeType>(node->node_type());
|
||||
if (node_type == NodeType::ELEMENT_NODE) {
|
||||
// 1. For each element selected by the range, whose parent is not selected by the range, include the border
|
||||
|
@ -1181,7 +1193,7 @@ GC::Ref<Geometry::DOMRectList> Range::get_client_rects()
|
|||
auto const& paintable_lines = static_cast<Painting::PaintableWithLines const&>(*containing_block);
|
||||
auto fragments = paintable_lines.fragments();
|
||||
for (auto frag = fragments.begin(); frag != fragments.end(); frag++) {
|
||||
auto rect = frag->range_rect(start_offset(), end_offset());
|
||||
auto rect = frag->range_rect(selection_state, start_offset(), end_offset());
|
||||
if (rect.is_empty())
|
||||
continue;
|
||||
rects.append(Geometry::DOMRect::create(realm(),
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
||||
* Copyright (c) 2024-2025, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
|
||||
*
|
||||
* SPDX-License-Identifier: BSD-2-Clause
|
||||
*/
|
||||
|
@ -81,12 +81,12 @@ size_t PaintableFragment::index_in_node_for_point(CSSPixelPoint position) const
|
|||
return utf16_view().code_unit_offset_of(code_point_offset - 1);
|
||||
}
|
||||
|
||||
CSSPixelRect PaintableFragment::range_rect(size_t start_offset_in_code_units, size_t end_offset_in_code_units) const
|
||||
CSSPixelRect PaintableFragment::range_rect(Paintable::SelectionState selection_state, size_t start_offset_in_code_units, size_t end_offset_in_code_units) const
|
||||
{
|
||||
if (paintable().selection_state() == Paintable::SelectionState::None)
|
||||
if (selection_state == Paintable::SelectionState::None)
|
||||
return {};
|
||||
|
||||
if (paintable().selection_state() == Paintable::SelectionState::Full)
|
||||
if (selection_state == Paintable::SelectionState::Full)
|
||||
return absolute_rect();
|
||||
|
||||
auto const& font = glyph_run() ? glyph_run()->font() : layout_node().first_available_font();
|
||||
|
@ -110,7 +110,7 @@ CSSPixelRect PaintableFragment::range_rect(size_t start_offset_in_code_units, si
|
|||
// We operate on the UTF-8 string that is part of this fragment.
|
||||
auto text = utf8_view().substring_view(m_start_byte_offset, m_length_in_bytes);
|
||||
|
||||
if (paintable().selection_state() == Paintable::SelectionState::StartAndEnd) {
|
||||
if (selection_state == Paintable::SelectionState::StartAndEnd) {
|
||||
auto selection_start_in_this_fragment = code_unit_to_byte_offset(start_offset_in_code_units);
|
||||
auto selection_end_in_this_fragment = code_unit_to_byte_offset(end_offset_in_code_units);
|
||||
|
||||
|
@ -141,7 +141,7 @@ CSSPixelRect PaintableFragment::range_rect(size_t start_offset_in_code_units, si
|
|||
|
||||
return rect;
|
||||
}
|
||||
if (paintable().selection_state() == Paintable::SelectionState::Start) {
|
||||
if (selection_state == Paintable::SelectionState::Start) {
|
||||
auto selection_start_in_this_fragment = code_unit_to_byte_offset(start_offset_in_code_units);
|
||||
auto selection_end_in_this_fragment = m_length_in_bytes;
|
||||
|
||||
|
@ -168,7 +168,7 @@ CSSPixelRect PaintableFragment::range_rect(size_t start_offset_in_code_units, si
|
|||
|
||||
return rect;
|
||||
}
|
||||
if (paintable().selection_state() == Paintable::SelectionState::End) {
|
||||
if (selection_state == Paintable::SelectionState::End) {
|
||||
auto selection_start_in_this_fragment = 0u;
|
||||
auto selection_end_in_this_fragment = code_unit_to_byte_offset(end_offset_in_code_units);
|
||||
|
||||
|
@ -226,7 +226,7 @@ CSSPixelRect PaintableFragment::selection_rect() const
|
|||
}
|
||||
auto selection_start = text_control_element->selection_start();
|
||||
auto selection_end = text_control_element->selection_end();
|
||||
return range_rect(selection_start, selection_end);
|
||||
return range_rect(paintable().selection_state(), selection_start, selection_end);
|
||||
}
|
||||
|
||||
auto selection = paintable().document().get_selection();
|
||||
|
@ -236,7 +236,7 @@ CSSPixelRect PaintableFragment::selection_rect() const
|
|||
if (!range)
|
||||
return {};
|
||||
|
||||
return range_rect(range->start_offset(), range->end_offset());
|
||||
return range_rect(paintable().selection_state(), range->start_offset(), range->end_offset());
|
||||
}
|
||||
|
||||
Utf8View PaintableFragment::utf8_view() const
|
||||
|
|
|
@ -39,7 +39,7 @@ public:
|
|||
Gfx::Orientation orientation() const;
|
||||
|
||||
CSSPixelRect selection_rect() const;
|
||||
CSSPixelRect range_rect(size_t start_offset_in_code_units, size_t end_offset_in_code_units) const;
|
||||
CSSPixelRect range_rect(Paintable::SelectionState selection_state, size_t start_offset_in_code_units, size_t end_offset_in_code_units) const;
|
||||
|
||||
CSSPixels width() const { return m_size.width(); }
|
||||
CSSPixels height() const { return m_size.height(); }
|
||||
|
|
|
@ -0,0 +1,17 @@
|
|||
<!DOCTYPE html>
|
||||
<html>
|
||||
<body>
|
||||
<p id="text">This is a simple sentence used to test range and selection rectangles.</p>
|
||||
<script>
|
||||
const textNode = document.getElementById("text").firstChild;
|
||||
|
||||
const sel = window.getSelection();
|
||||
sel.removeAllRanges();
|
||||
|
||||
const selRange = document.createRange();
|
||||
selRange.setStart(textNode, 5);
|
||||
selRange.setEnd(textNode, 16);
|
||||
sel.addRange(selRange);
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
|
@ -0,0 +1,32 @@
|
|||
<!DOCTYPE html>
|
||||
<html class="reftest-wait">
|
||||
<link
|
||||
rel="match"
|
||||
href="../expected/range-not-assiciated-with-selection-should-not-affect-it-ref.html"
|
||||
/>
|
||||
<body>
|
||||
<p id="text">This is a simple sentence used to test range and selection rectangles.</p>
|
||||
<script>
|
||||
const textNode = document.getElementById("text").firstChild;
|
||||
|
||||
const sel = window.getSelection();
|
||||
sel.removeAllRanges();
|
||||
|
||||
const selRange = document.createRange();
|
||||
selRange.setStart(textNode, 5);
|
||||
selRange.setEnd(textNode, 16);
|
||||
sel.addRange(selRange);
|
||||
|
||||
requestAnimationFrame(() => {
|
||||
const otherRange = document.createRange();
|
||||
otherRange.setStart(textNode, 0);
|
||||
otherRange.setEnd(textNode, 0);
|
||||
|
||||
// Requesting client rects on a range not associated with selection should not affect selection
|
||||
otherRange.getClientRects();
|
||||
|
||||
document.documentElement.classList.remove("reftest-wait");
|
||||
});
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
Loading…
Add table
Add a link
Reference in a new issue