From c145d4076e70c7b1555dc82006fc0fa51ae0b0fb Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Sat, 25 Jan 2025 22:41:21 +0100 Subject: [PATCH 1/2] LibWeb: Honor `pointer-events` for PaintableBox in own stacking context If a block element with its own stacking context has `pointer-events: none` set, it should be ignored as far as hit-testing goes. Fixes #3357. --- Libraries/LibWeb/Painting/StackingContext.cpp | 9 ++++----- .../LibWeb/Text/expected/hit_testing/pointer-events.txt | 3 +++ Tests/LibWeb/Text/input/hit_testing/pointer-events.html | 5 +++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Libraries/LibWeb/Painting/StackingContext.cpp b/Libraries/LibWeb/Painting/StackingContext.cpp index 3075bad75765..56dec20390bf 100644 --- a/Libraries/LibWeb/Painting/StackingContext.cpp +++ b/Libraries/LibWeb/Painting/StackingContext.cpp @@ -443,13 +443,12 @@ TraversalDecision StackingContext::hit_test(CSSPixelPoint position, HitTestType CSSPixelPoint enclosing_scroll_offset = paintable_box().cumulative_offset_of_enclosing_scroll_frame(); - auto position_adjusted_by_scroll_offset = transformed_position; - position_adjusted_by_scroll_offset.translate_by(-enclosing_scroll_offset); + auto position_adjusted_by_scroll_offset = transformed_position.translated(-enclosing_scroll_offset); // 1. the background and borders of the element forming the stacking context. - if (paintable_box().absolute_border_box_rect().contains(position_adjusted_by_scroll_offset.x(), position_adjusted_by_scroll_offset.y())) { - auto hit_test_result = HitTestResult { .paintable = const_cast(paintable_box()) }; - if (callback(hit_test_result) == TraversalDecision::Break) + if (paintable_box().visible_for_hit_testing() + && paintable_box().absolute_border_box_rect().contains(position_adjusted_by_scroll_offset)) { + if (callback({ const_cast(paintable_box()) }) == TraversalDecision::Break) return TraversalDecision::Break; } diff --git a/Tests/LibWeb/Text/expected/hit_testing/pointer-events.txt b/Tests/LibWeb/Text/expected/hit_testing/pointer-events.txt index 8da3817c29aa..519d4a62eda0 100644 --- a/Tests/LibWeb/Text/expected/hit_testing/pointer-events.txt +++ b/Tests/LibWeb/Text/expected/hit_testing/pointer-events.txt @@ -10,3 +10,6 @@ --- +
+ +--- \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/hit_testing/pointer-events.html b/Tests/LibWeb/Text/input/hit_testing/pointer-events.html index ee567380e475..47cb82a23108 100644 --- a/Tests/LibWeb/Text/input/hit_testing/pointer-events.html +++ b/Tests/LibWeb/Text/input/hit_testing/pointer-events.html @@ -19,6 +19,10 @@ foo
bar
bazlorem + + +
+
From c80253e4a414c8de05b7b4b3e7ee6b9cad77e68e Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Sun, 26 Jan 2025 00:35:59 +0100 Subject: [PATCH 2/2] LibWeb: Handle continuation chain during hit-testing instead of after it My previous attempt at resolving the continuation chain tried to deal with `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 63ba7ffe015f..aa943dd4887a 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 6f57db8530a0..e67282fe4601 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 000000000000..a64aa53366cf --- /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 000000000000..93c0f4bb15af --- /dev/null +++ b/Tests/LibWeb/Text/input/hit_testing/pointer-events-no-parent-extension.html @@ -0,0 +1,15 @@ + + +
+ +
+ foobar +
+ +