From 556679fedd98d1d3f7bf0450807ec8256ddc474f Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Mon, 29 Jan 2024 06:28:17 +0100 Subject: [PATCH] LibWeb: Account for scroll offset in hit-testing The hit-testing position is now shifted by the scroll offsets before performing any checks for containment. This is implemented by assigning each PaintableBox/InlinePaintable an offset corresponding to the scroll frame in which it is contained. The non-scroll-adjusted position is still passed down when recursing to children because the assigned offset accumulated for nested scroll frames. With this change, hit testing works in the Inspector. Fixes https://github.com/SerenityOS/serenity/issues/22068 --- .../expected/hit_testing/overflow-scroll.txt | 2 + .../input/hit_testing/overflow-scroll.html | 49 +++++++++++++++++++ .../LibWeb/Painting/InlinePaintable.cpp | 8 ++- .../LibWeb/Painting/InlinePaintable.h | 2 + .../LibWeb/Painting/PaintableBox.cpp | 24 ++++++--- .../Libraries/LibWeb/Painting/PaintableBox.h | 4 ++ .../LibWeb/Painting/ViewportPaintable.cpp | 2 + 7 files changed, 81 insertions(+), 10 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/hit_testing/overflow-scroll.txt create mode 100644 Tests/LibWeb/Text/input/hit_testing/overflow-scroll.html diff --git a/Tests/LibWeb/Text/expected/hit_testing/overflow-scroll.txt b/Tests/LibWeb/Text/expected/hit_testing/overflow-scroll.txt new file mode 100644 index 0000000000..9242d09e4e --- /dev/null +++ b/Tests/LibWeb/Text/expected/hit_testing/overflow-scroll.txt @@ -0,0 +1,2 @@ + Line 1 Line 2 Line 3 Line 4 Line 5 Line 6 Line 7 Line 8 Line 9 Line 10 Line 11 Line 12 Line 13 Line 14 Line 15 Line 16 Line 17 Line 18 Line 19 Line 20

+ diff --git a/Tests/LibWeb/Text/input/hit_testing/overflow-scroll.html b/Tests/LibWeb/Text/input/hit_testing/overflow-scroll.html new file mode 100644 index 0000000000..99f0183bbd --- /dev/null +++ b/Tests/LibWeb/Text/input/hit_testing/overflow-scroll.html @@ -0,0 +1,49 @@ + + + + +

+

Line 1

+

Line 2

+

Line 3

+

Line 4

+

Line 5

+

Line 6

+

Line 7

+ Line 8 +

Line 9

+

Line 10

+

Line 11

+

Line 12

+

Line 13

+

Line 14

+

Line 15

+

Line 16

+

Line 17

+

Line 18

+

Line 19

+

Line 20

+
+ + diff --git a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp index fbce8a88da..1f24507aae 100644 --- a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp @@ -182,15 +182,19 @@ void InlinePaintable::for_each_fragment(Callback callback) const Optional InlinePaintable::hit_test(CSSPixelPoint position, HitTestType type) const { + auto position_adjusted_by_scroll_offset = position; + if (m_enclosing_scroll_frame_offset.has_value()) + position_adjusted_by_scroll_offset.translate_by(-m_enclosing_scroll_frame_offset.value()); + for (auto& fragment : m_fragments) { if (fragment.paintable().stacking_context()) continue; auto fragment_absolute_rect = fragment.absolute_rect(); - if (fragment_absolute_rect.contains(position)) { + if (fragment_absolute_rect.contains(position_adjusted_by_scroll_offset)) { if (auto result = fragment.paintable().hit_test(position, type); result.has_value()) return result; return HitTestResult { const_cast(fragment.paintable()), - fragment.text_index_at(position.x()) }; + fragment.text_index_at(position_adjusted_by_scroll_offset.x()) }; } } diff --git a/Userland/Libraries/LibWeb/Painting/InlinePaintable.h b/Userland/Libraries/LibWeb/Painting/InlinePaintable.h index 378031957f..d80ed3efa8 100644 --- a/Userland/Libraries/LibWeb/Painting/InlinePaintable.h +++ b/Userland/Libraries/LibWeb/Painting/InlinePaintable.h @@ -38,6 +38,7 @@ public: Vector const& box_shadow_data() const { return m_box_shadow_data; } void set_scroll_frame_id(int id) { m_scroll_frame_id = id; } + void set_enclosing_scroll_frame_offset(CSSPixelPoint offset) { m_enclosing_scroll_frame_offset = offset; } void set_clip_rect(Optional rect) { m_clip_rect = rect; } private: @@ -47,6 +48,7 @@ private: void for_each_fragment(Callback) const; Optional m_scroll_frame_id; + Optional m_enclosing_scroll_frame_offset; Optional m_clip_rect; Vector m_box_shadow_data; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 04504fa47c..1f691acd5c 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -672,6 +672,10 @@ Layout::BlockContainer& PaintableWithLines::layout_box() Optional PaintableBox::hit_test(CSSPixelPoint position, HitTestType type) const { + auto position_adjusted_by_scroll_offset = position; + if (enclosing_scroll_frame_offset().has_value()) + position_adjusted_by_scroll_offset.translate_by(-enclosing_scroll_frame_offset().value()); + if (!is_visible()) return {}; @@ -680,7 +684,7 @@ Optional PaintableBox::hit_test(CSSPixelPoint position, HitTestTy return stacking_context()->hit_test(position, type); } - if (!absolute_border_box_rect().contains(position.x(), position.y())) + if (!absolute_border_box_rect().contains(position_adjusted_by_scroll_offset.x(), position_adjusted_by_scroll_offset.y())) return {}; for (auto* child = first_child(); child; child = child->next_sibling()) { @@ -700,6 +704,10 @@ Optional PaintableBox::hit_test(CSSPixelPoint position, HitTestTy Optional PaintableWithLines::hit_test(CSSPixelPoint position, HitTestType type) const { + auto position_adjusted_by_scroll_offset = position; + if (enclosing_scroll_frame_offset().has_value()) + position_adjusted_by_scroll_offset.translate_by(-enclosing_scroll_frame_offset().value()); + if (!layout_box().children_are_inline() || m_fragments.is_empty()) return PaintableBox::hit_test(position, type); @@ -717,10 +725,10 @@ Optional PaintableWithLines::hit_test(CSSPixelPoint position, Hit if (fragment.paintable().stacking_context()) continue; auto fragment_absolute_rect = fragment.absolute_rect(); - if (fragment_absolute_rect.contains(position)) { + if (fragment_absolute_rect.contains(position_adjusted_by_scroll_offset)) { if (auto result = fragment.paintable().hit_test(position, type); result.has_value()) return result; - return HitTestResult { const_cast(fragment.paintable()), fragment.text_index_at(position.x()) }; + return HitTestResult { const_cast(fragment.paintable()), fragment.text_index_at(position_adjusted_by_scroll_offset.x()) }; } // If we reached this point, the position is not within the fragment. However, the fragment start or end might be the place to place the cursor. @@ -728,11 +736,11 @@ Optional PaintableWithLines::hit_test(CSSPixelPoint position, Hit // The best candidate is either the end of the line above, the beginning of the line below, or the beginning or end of the current line. // We arbitrarily choose to consider the end of the line above and ignore the beginning of the line below. // If we knew the direction of selection, we could make a better choice. - if (fragment_absolute_rect.bottom() - 1 <= position.y()) { // fully below the fragment + if (fragment_absolute_rect.bottom() - 1 <= position_adjusted_by_scroll_offset.y()) { // fully below the fragment last_good_candidate = HitTestResult { const_cast(fragment.paintable()), fragment.start() + fragment.length() }; - } else if (fragment_absolute_rect.top() <= position.y()) { // vertically within the fragment - if (position.x() < fragment_absolute_rect.left()) { // left of the fragment - if (!last_good_candidate.has_value()) { // first fragment of the line + } else if (fragment_absolute_rect.top() <= position_adjusted_by_scroll_offset.y()) { // vertically within the fragment + if (position_adjusted_by_scroll_offset.x() < fragment_absolute_rect.left()) { // left of the fragment + if (!last_good_candidate.has_value()) { // first fragment of the line last_good_candidate = HitTestResult { const_cast(fragment.paintable()), fragment.start() }; } } else { // right of the fragment @@ -743,7 +751,7 @@ Optional PaintableWithLines::hit_test(CSSPixelPoint position, Hit if (type == HitTestType::TextCursor && last_good_candidate.has_value()) return last_good_candidate; - if (is_visible() && absolute_border_box_rect().contains(position.x(), position.y())) + if (is_visible() && absolute_border_box_rect().contains(position_adjusted_by_scroll_offset.x(), position_adjusted_by_scroll_offset.y())) return HitTestResult { const_cast(*this) }; return {}; } diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.h b/Userland/Libraries/LibWeb/Painting/PaintableBox.h index 3c668b3b9a..f84c939724 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.h @@ -195,6 +195,7 @@ public: void set_clip_rect(Optional rect) { m_clip_rect = rect; } void set_scroll_frame_id(int id) { m_scroll_frame_id = id; } + void set_enclosing_scroll_frame_offset(CSSPixelPoint offset) { m_enclosing_scroll_frame_offset = offset; } void set_corner_clip_radii(CornerRadii const& corner_radii) { m_corner_clip_radii = corner_radii; } protected: @@ -208,6 +209,8 @@ protected: virtual CSSPixelRect compute_absolute_rect() const; virtual CSSPixelRect compute_absolute_paint_rect() const; + Optional enclosing_scroll_frame_offset() const { return m_enclosing_scroll_frame_offset; } + private: [[nodiscard]] virtual bool is_paintable_box() const final { return true; } @@ -224,6 +227,7 @@ private: Optional m_clip_rect; Optional m_scroll_frame_id; + Optional m_enclosing_scroll_frame_offset; Optional m_corner_clip_radii; Optional m_override_borders_data; diff --git a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp index f1010c222f..5c234e79f1 100644 --- a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp @@ -82,9 +82,11 @@ void ViewportPaintable::assign_scroll_frame_ids(HashMap(paintable); const_cast(paintable_box).set_scroll_frame_id(scroll_frame_id->id); + const_cast(paintable_box).set_enclosing_scroll_frame_offset(scroll_frame_id->offset); } else if (paintable.is_inline_paintable()) { auto const& inline_paintable = static_cast(paintable); const_cast(inline_paintable).set_scroll_frame_id(scroll_frame_id->id); + const_cast(inline_paintable).set_enclosing_scroll_frame_offset(scroll_frame_id->offset); } break; }