LibWeb: Make hit testing traverse positioned descendants in right order

We were doing a forward traversal in hit testing which led to sometimes
incorrect results when multiple boxes were occupying the same X and Y
coordinate.

Userland/Libraries/LibWeb/Painting/Paintable.cpp

@@ -62,4 +62,20 @@ Paintable const* Paintable::next_sibling() const
     return layout_node ? layout_node->paintable() : nullptr;
 }
 
+Paintable const* Paintable::last_child() const
+{
+    auto* layout_child = m_layout_node.last_child();
+    for (; layout_child && !layout_child->paintable(); layout_child = layout_child->previous_sibling())
+        ;
+    return layout_child ? layout_child->paintable() : nullptr;
+}
+
+Paintable const* Paintable::previous_sibling() const
+{
+    auto* layout_node = m_layout_node.previous_sibling();
+    for (; layout_node && !layout_node->paintable(); layout_node = layout_node->previous_sibling())
+        ;
+    return layout_node ? layout_node->paintable() : nullptr;
+}
+
 }

Userland/Libraries/LibWeb/Painting/Paintable.h

@@ -56,13 +56,15 @@ public:
     virtual ~Paintable() = default;
 
     Paintable const* first_child() const;
+    Paintable const* last_child() const;
     Paintable const* next_sibling() const;
+    Paintable const* previous_sibling() const;
 
     template<typename U, typename Callback>
     TraversalDecision for_each_in_inclusive_subtree_of_type(Callback callback) const
     {
         if (is<U>(*this)) {
-            if (auto decision = callback(static_cast<const U&>(*this)); decision != TraversalDecision::Continue)
+            if (auto decision = callback(static_cast<U const&>(*this)); decision != TraversalDecision::Continue)
                 return decision;
         }
         for (auto* child = first_child(); child; child = child->next_sibling()) {

Userland/Libraries/LibWeb/Painting/StackingContext.cpp

@@ -407,6 +407,33 @@ Gfx::FloatPoint StackingContext::compute_transform_origin() const
     return { x, y };
 }
 
+template<typename U, typename Callback>
+static TraversalDecision for_each_in_inclusive_subtree_of_type_within_same_stacking_context_in_reverse(Paintable const& paintable, Callback callback)
+{
+    if (is<PaintableBox>(paintable) && static_cast<PaintableBox const&>(paintable).stacking_context())
+        return TraversalDecision::SkipChildrenAndContinue;
+
+    for (auto* child = paintable.last_child(); child; child = child->previous_sibling()) {
+        if (for_each_in_inclusive_subtree_of_type_within_same_stacking_context_in_reverse<U>(*child, callback) == TraversalDecision::Break)
+            return TraversalDecision::Break;
+    }
+    if (is<U>(paintable)) {
+        if (auto decision = callback(static_cast<U const&>(paintable)); decision != TraversalDecision::Continue)
+            return decision;
+    }
+    return TraversalDecision::Continue;
+}
+
+template<typename U, typename Callback>
+static TraversalDecision for_each_in_subtree_of_type_within_same_stacking_context_in_reverse(Paintable const& paintable, Callback callback)
+{
+    for (auto* child = paintable.last_child(); child; child = child->previous_sibling()) {
+        if (for_each_in_inclusive_subtree_of_type_within_same_stacking_context_in_reverse<U>(*child, callback) == TraversalDecision::Break)
+            return TraversalDecision::Break;
+    }
+    return TraversalDecision::Continue;
+}
+
 Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& position, HitTestType type) const
 {
     if (!m_box.is_visible())
@@ -428,7 +455,7 @@ Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& positio
     // NOTE: Hit testing follows reverse painting order, that's why the conditions here are reversed.
     for (ssize_t i = m_children.size() - 1; i >= 0; --i) {
         auto const& child = *m_children[i];
-        if (child.m_box.computed_values().z_index().value_or(0) < 0)
+        if (child.m_box.computed_values().z_index().value_or(0) <= 0)
             break;
         auto result = child.hit_test(transformed_position, type);
         if (result.has_value() && result->paintable->visible_for_hit_testing())
@@ -437,17 +464,31 @@ Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& positio
 
     Optional<HitTestResult> result;
     // 6. the child stacking contexts with stack level 0 and the positioned descendants with stack level 0.
-    paintable().for_each_in_subtree_of_type<PaintableBox>([&](auto& paint_box) {
+
+    for_each_in_subtree_of_type_within_same_stacking_context_in_reverse<PaintableBox>(paintable(), [&](PaintableBox const& paint_box) {
         // FIXME: Support more overflow variations.
         if (paint_box.computed_values().overflow_x() == CSS::Overflow::Hidden && paint_box.computed_values().overflow_y() == CSS::Overflow::Hidden) {
             if (!paint_box.absolute_border_box_rect().contains(transformed_position.x(), transformed_position.y()))
                 return TraversalDecision::SkipChildrenAndContinue;
         }
 
+        if (paint_box.stacking_context()) {
+            auto const& z_index = paint_box.computed_values().z_index();
+            if (!z_index.has_value() || z_index.value() == 0) {
+                auto candidate = paint_box.stacking_context()->hit_test(transformed_position, type);
+                if (candidate.has_value() && candidate->paintable->visible_for_hit_testing()) {
+                    result = move(candidate);
+                    return TraversalDecision::Break;
+                }
+            }
+        }
+
         auto& layout_box = paint_box.layout_box();
         if (layout_box.is_positioned() && !paint_box.stacking_context()) {
-            if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value())
+            if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) {
                 result = move(candidate);
+                return TraversalDecision::Break;
+            }
         }
         return TraversalDecision::Continue;
     });
@@ -462,7 +503,7 @@ Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& positio
     }
 
     // 4. the non-positioned floats.
-    paintable().for_each_in_subtree_of_type<PaintableBox>([&](auto const& paint_box) {
+    for_each_in_subtree_of_type_within_same_stacking_context_in_reverse<PaintableBox>(paintable(), [&](auto const& paint_box) {
         // FIXME: Support more overflow variations.
         if (paint_box.computed_values().overflow_x() == CSS::Overflow::Hidden && paint_box.computed_values().overflow_y() == CSS::Overflow::Hidden) {
             if (!paint_box.absolute_border_box_rect().contains(transformed_position.x(), transformed_position.y()))
@@ -471,8 +512,10 @@ Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& positio
 
         auto& layout_box = paint_box.layout_box();
         if (layout_box.is_floating()) {
-            if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value())
+            if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) {
                 result = move(candidate);
+                return TraversalDecision::Break;
+            }
         }
         return TraversalDecision::Continue;
     });
@@ -481,7 +524,7 @@ Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& positio
 
     // 3. the in-flow, non-inline-level, non-positioned descendants.
     if (!m_box.children_are_inline()) {
-        paintable().for_each_in_subtree_of_type<PaintableBox>([&](auto const& paint_box) {
+        for_each_in_subtree_of_type_within_same_stacking_context_in_reverse<PaintableBox>(paintable(), [&](auto const& paint_box) {
             // FIXME: Support more overflow variations.
             if (paint_box.computed_values().overflow_x() == CSS::Overflow::Hidden && paint_box.computed_values().overflow_y() == CSS::Overflow::Hidden) {
                 if (!paint_box.absolute_border_box_rect().contains(transformed_position.x(), transformed_position.y()))
@@ -490,8 +533,10 @@ Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& positio
 
             auto& layout_box = paint_box.layout_box();
             if (!layout_box.is_absolutely_positioned() && !layout_box.is_floating()) {
-                if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value())
+                if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) {
                     result = move(candidate);
+                    return TraversalDecision::Break;
+                }
             }
             return TraversalDecision::Continue;
         });