From 1f88e6819abf21807dd7f541a60bcccd80480e3e Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 19 Aug 2025 13:35:13 -0400 Subject: [PATCH] LibWeb: Ensure hit testing is grapheme aware Previously, clicking in the middle of a multi-code point grapheme would place the cursor at a code unit index somewhere in the middle of the grapheme. This was not only visually misleading, but the user could then start typing and insert characters in the middle of the cluster. This also made text select pretty wonky. The main issue was that we were treating the glyph index in a glyph run as a code unit index. We must instead map that glyph index back to a code unit index with help from LibGfx (via harfbuzz). The distance computation used here was also a bit off, especially for the last glyph in a glyph run. We essentially want the cursor to end up on whichever edge of the clicked glyph it is closest to. The result of the distance computation limited us to the left edge of the last glyph. Instead, we can use the same edge tracking we use for form- associated elements to handle this for us. --- .../LibWeb/Painting/PaintableFragment.cpp | 15 +++++------- .../Text/expected/hit_testing/grapheme.txt | 6 +++++ .../Text/input/hit_testing/grapheme.html | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/hit_testing/grapheme.txt create mode 100644 Tests/LibWeb/Text/input/hit_testing/grapheme.html diff --git a/Libraries/LibWeb/Painting/PaintableFragment.cpp b/Libraries/LibWeb/Painting/PaintableFragment.cpp index c35d24bcce4..b988c2fcd63 100644 --- a/Libraries/LibWeb/Painting/PaintableFragment.cpp +++ b/Libraries/LibWeb/Painting/PaintableFragment.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -52,18 +53,14 @@ size_t PaintableFragment::index_in_node_for_point(CSSPixelPoint position) const if (relative_inline_offset < 0) return 0; - auto const& glyphs = m_glyph_run->glyphs(); - auto smallest_distance = AK::NumericLimits::max(); - for (size_t i = 0; i < glyphs.size(); ++i) { - auto distance_to_position = AK::abs(glyphs[i].position.x() - relative_inline_offset); + GraphemeEdgeTracker tracker { relative_inline_offset }; - // The last distance was smaller than this new distance, so we've found the closest glyph. - if (distance_to_position > smallest_distance) - return m_start_offset + i - 1; - smallest_distance = distance_to_position; + for (auto const& glyph : m_glyph_run->glyphs()) { + if (tracker.update(glyph.length_in_code_units, glyph.glyph_width) == IterationDecision::Break) + break; } - return m_start_offset + m_length_in_code_units - 1; + return m_start_offset + tracker.resolve(); } CSSPixelRect PaintableFragment::range_rect(Paintable::SelectionState selection_state, size_t start_offset_in_code_units, size_t end_offset_in_code_units) const diff --git a/Tests/LibWeb/Text/expected/hit_testing/grapheme.txt b/Tests/LibWeb/Text/expected/hit_testing/grapheme.txt new file mode 100644 index 00000000000..b424a723c48 --- /dev/null +++ b/Tests/LibWeb/Text/expected/hit_testing/grapheme.txt @@ -0,0 +1,6 @@ +Click [13, 20]: position=0 text="hello 👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻" +Click [16, 20]: position=1 text="ello 👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻" +Click [52, 20]: position=6 text="👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻" +Click [57, 20]: position=18 text="👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻" +Click [85, 20]: position=30 text="👩🏼‍❤️‍👨🏻👩🏼‍❤️‍👨🏻" +Click [90, 20]: position=42 text="👩🏼‍❤️‍👨🏻" diff --git a/Tests/LibWeb/Text/input/hit_testing/grapheme.html b/Tests/LibWeb/Text/input/hit_testing/grapheme.html new file mode 100644 index 00000000000..06d117308e3 --- /dev/null +++ b/Tests/LibWeb/Text/input/hit_testing/grapheme.html @@ -0,0 +1,24 @@ + + + +