From d56f7d2928297d5abb6969a861320c3b8076f135 Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Sun, 26 Jan 2025 00:35:59 +0100 Subject: [PATCH] LibWeb: Handle continuation chain during hit-testing instead of after My previous attempt at resolving the continuation chain tried to resolve `pointer-events: none` by repeatedly falling back to the parent paintable until one was found that _would_ want to handle pointer events. But since we were no longer performing hit-tests on those paintables, false positives could pop up. This could happen for out-of-flow block elements that did not overlap with their parent rects, for example. This approach works much better since it only handles the continuation case that's relevant (the "middle" anonymous box) and it does so during hit-testing instead of after, allowing all the other relevant logic to come into play. --- Libraries/LibWeb/Painting/PaintableBox.cpp | 49 ++++++++++--------- Libraries/LibWeb/Painting/PaintableBox.h | 1 + .../pointer-events-no-parent-extension.txt | 2 + .../pointer-events-no-parent-extension.html | 15 ++++++ 4 files changed, 45 insertions(+), 22 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/hit_testing/pointer-events-no-parent-extension.txt create mode 100644 Tests/LibWeb/Text/input/hit_testing/pointer-events-no-parent-extension.html diff --git a/Libraries/LibWeb/Painting/PaintableBox.cpp b/Libraries/LibWeb/Painting/PaintableBox.cpp index 63ba7ffe015f1..aa943dd4887a0 100644 --- a/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -965,12 +965,36 @@ TraversalDecision PaintableBox::hit_test(CSSPixelPoint position, HitTestType typ return TraversalDecision::Break; } + if (!visible_for_hit_testing()) + return TraversalDecision::Continue; + if (!absolute_border_box_rect().contains(position_adjusted_by_scroll_offset)) return TraversalDecision::Continue; + if (hit_test_continuation(callback) == TraversalDecision::Break) + return TraversalDecision::Break; + return callback(HitTestResult { const_cast(*this) }); } +TraversalDecision PaintableBox::hit_test_continuation(Function const& callback) const +{ + // If we're hit testing the "middle" part of a continuation chain, we are dealing with an anonymous box that is + // linked to a parent inline node. Since our block element children did not match the hit test, but we did, we + // should walk the continuation chain up to the inline parent and return a hit on that instead. + auto continuation_node = layout_node_with_style_and_box_metrics().continuation_of_node(); + if (!continuation_node || !layout_node().is_anonymous()) + return TraversalDecision::Continue; + + while (continuation_node->continuation_of_node()) + continuation_node = continuation_node->continuation_of_node(); + auto& paintable = *continuation_node->first_paintable(); + if (!paintable.visible_for_hit_testing()) + return TraversalDecision::Continue; + + return callback(HitTestResult { paintable }); +} + Optional PaintableBox::hit_test(CSSPixelPoint position, HitTestType type) const { Optional result; @@ -985,28 +1009,6 @@ Optional PaintableBox::hit_test(CSSPixelPoint position, HitTestTy return TraversalDecision::Break; return TraversalDecision::Continue; }); - - // If our hit-testing has resulted in a hit on a paintable, we know that it is the most specific hit. If that - // paintable turns out to be invisible for hit-testing, we need to traverse up the paintable tree to find the next - // paintable that is visible for hit-testing. This implements the behavior expected for pointer-events. - while (result.has_value() && !result->paintable->visible_for_hit_testing()) { - result->index_in_node = result->paintable->dom_node() ? result->paintable->dom_node()->index() : 0; - result->paintable = result->paintable->parent(); - - // If the new parent is an anonymous box part of a continuation, we need to follow the chain to the inline node - // that spawned the anonymous "middle" part of the continuation, since that inline node is the actual parent. - if (is(*result->paintable)) { - auto const& box_layout_node = static_cast(*result->paintable).layout_node_with_style_and_box_metrics(); - if (box_layout_node.is_anonymous() && box_layout_node.continuation_of_node()) { - auto const* original_inline_node = &box_layout_node; - while (original_inline_node->continuation_of_node()) - original_inline_node = original_inline_node->continuation_of_node(); - - result->paintable = const_cast(original_inline_node->first_paintable()); - } - } - } - return result; } @@ -1048,6 +1050,9 @@ TraversalDecision PaintableWithLines::hit_test(CSSPixelPoint position, HitTestTy return TraversalDecision::Break; } + if (!visible_for_hit_testing()) + return TraversalDecision::Continue; + for (auto const& fragment : fragments()) { if (fragment.paintable().has_stacking_context()) continue; diff --git a/Libraries/LibWeb/Painting/PaintableBox.h b/Libraries/LibWeb/Painting/PaintableBox.h index 6f57db8530a04..e67282fe4601b 100644 --- a/Libraries/LibWeb/Painting/PaintableBox.h +++ b/Libraries/LibWeb/Painting/PaintableBox.h @@ -144,6 +144,7 @@ class PaintableBox : public Paintable [[nodiscard]] virtual TraversalDecision hit_test(CSSPixelPoint position, HitTestType type, Function const& callback) const override; Optional hit_test(CSSPixelPoint, HitTestType) const; + [[nodiscard]] TraversalDecision hit_test_continuation(Function const& callback) const; virtual bool handle_mousewheel(Badge, CSSPixelPoint, unsigned buttons, unsigned modifiers, int wheel_delta_x, int wheel_delta_y) override; diff --git a/Tests/LibWeb/Text/expected/hit_testing/pointer-events-no-parent-extension.txt b/Tests/LibWeb/Text/expected/hit_testing/pointer-events-no-parent-extension.txt new file mode 100644 index 0000000000000..a64aa53366cf5 --- /dev/null +++ b/Tests/LibWeb/Text/expected/hit_testing/pointer-events-no-parent-extension.txt @@ -0,0 +1,2 @@ + +<#document> diff --git a/Tests/LibWeb/Text/input/hit_testing/pointer-events-no-parent-extension.html b/Tests/LibWeb/Text/input/hit_testing/pointer-events-no-parent-extension.html new file mode 100644 index 0000000000000..93c0f4bb15afb --- /dev/null +++ b/Tests/LibWeb/Text/input/hit_testing/pointer-events-no-parent-extension.html @@ -0,0 +1,15 @@ + + +
+ +
+ foobar +
+ +