LibWeb: Make hit testing functions return Optional<HitTestResult>

Using "HitTestResult with null paintable" as a way to signal misses was
unnecessarily confusing. Let's use Optional instead. :^)

Userland/Libraries/LibWeb/Page/EventHandler.cpp

@@ -139,7 +139,7 @@ bool EventHandler::handle_mousewheel(const Gfx::IntPoint& position, unsigned int
     // FIXME: Support wheel events in nested browsing contexts.
 
     auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact);
-    if (result.paintable && result.paintable->handle_mousewheel({}, position, buttons, modifiers, wheel_delta_x, wheel_delta_y))
+    if (result.has_value() && result->paintable->handle_mousewheel({}, position, buttons, modifiers, wheel_delta_x, wheel_delta_y))
         return true;
 
     if (auto* page = m_browsing_context.page()) {
@@ -164,8 +164,8 @@ bool EventHandler::handle_mouseup(const Gfx::IntPoint& position, unsigned button
     if (m_mouse_event_tracking_layout_node) {
         paintable = m_mouse_event_tracking_layout_node->paintable();
     } else {
-        auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact);
+        if (auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact); result.has_value())
-        paintable = result.paintable;
+            paintable = result->paintable;
     }
 
     if (paintable && paintable->wants_mouse_events()) {
@@ -175,8 +175,8 @@ bool EventHandler::handle_mouseup(const Gfx::IntPoint& position, unsigned button
         // Things may have changed as a consequence of Layout::Node::handle_mouseup(). Hit test again.
         if (!paint_root())
             return true;
-        auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact);
+        if (auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact); result.has_value())
-        paintable = result.paintable;
+            paintable = result->paintable;
     }
 
     if (paintable) {
@@ -223,9 +223,9 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt
             paintable = m_mouse_event_tracking_layout_node->paintable();
         } else {
             auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact);
-            if (!result.paintable)
+            if (!result.has_value())
                 return false;
-            paintable = result.paintable;
+            paintable = result->paintable;
         }
 
         auto pointer_events = paintable->computed_values().pointer_events();
@@ -301,7 +301,7 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt
     } else {
         if (button == GUI::MouseButton::Primary) {
             auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::TextCursor);
-            if (result.paintable && result.paintable->layout_node().dom_node()) {
+            if (result.has_value() && result->paintable->layout_node().dom_node()) {
 
                 // See if we want to focus something.
                 bool did_focus_something = false;
@@ -316,8 +316,8 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt
                 // If we didn't focus anything, place the document text cursor at the mouse position.
                 // FIXME: This is all rather strange. Find a better solution.
                 if (!did_focus_something) {
-                    m_browsing_context.set_cursor_position(DOM::Position(*result.paintable->layout_node().dom_node(), result.index_in_node));
+                    m_browsing_context.set_cursor_position(DOM::Position(*result->paintable->layout_node().dom_node(), result->index_in_node));
-                    layout_root()->set_selection({ { result.paintable->layout_node(), result.index_in_node }, {} });
+                    layout_root()->set_selection({ { result->paintable->layout_node(), result->index_in_node }, {} });
                     m_in_mouse_selection = true;
                 }
             }
@@ -348,9 +348,10 @@ bool EventHandler::handle_mousemove(const Gfx::IntPoint& position, unsigned butt
     if (m_mouse_event_tracking_layout_node) {
         paintable = m_mouse_event_tracking_layout_node->paintable();
     } else {
-        auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact);
+        if (auto result = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::Exact); result.has_value()) {
-        paintable = result.paintable;
+            paintable = result->paintable;
-        start_index = result.index_in_node;
+            start_index = result->index_in_node;
+        }
     }
 
     const HTML::HTMLAnchorElement* hovered_link_element = nullptr;
@@ -409,9 +410,9 @@ bool EventHandler::handle_mousemove(const Gfx::IntPoint& position, unsigned butt
         }
         if (m_in_mouse_selection) {
             auto hit = paint_root()->hit_test(position.to_type<float>(), Painting::HitTestType::TextCursor);
-            if (start_index.has_value() && hit.paintable && hit.paintable->layout_node().dom_node()) {
+            if (start_index.has_value() && hit.has_value() && hit->paintable->layout_node().dom_node()) {
-                m_browsing_context.set_cursor_position(DOM::Position(*hit.paintable->layout_node().dom_node(), *start_index));
+                m_browsing_context.set_cursor_position(DOM::Position(*hit->paintable->layout_node().dom_node(), *start_index));
-                layout_root()->set_selection_end({ hit.paintable->layout_node(), hit.index_in_node });
+                layout_root()->set_selection_end({ hit->paintable->layout_node(), hit->index_in_node });
             }
             if (auto* page = m_browsing_context.page())
                 page->client().page_did_change_selection();

Userland/Libraries/LibWeb/Painting/Paintable.cpp

@@ -41,7 +41,7 @@ bool Paintable::handle_mousewheel(Badge<EventHandler>, Gfx::IntPoint const&, uns
     return false;
 }
 
-HitTestResult Paintable::hit_test(Gfx::FloatPoint const&, HitTestType) const
+Optional<HitTestResult> Paintable::hit_test(Gfx::FloatPoint const&, HitTestType) const
 {
     return {};
 }

Userland/Libraries/LibWeb/Painting/Paintable.h

@@ -22,7 +22,7 @@ enum class PaintPhase {
 };
 
 struct HitTestResult {
-    RefPtr<Painting::Paintable> paintable;
+    NonnullRefPtr<Painting::Paintable> paintable;
     int index_in_node { 0 };
 
     enum InternalPosition {
@@ -50,7 +50,7 @@ public:
     virtual void before_children_paint(PaintContext&, PaintPhase) const { }
     virtual void after_children_paint(PaintContext&, PaintPhase) const { }
 
-    virtual HitTestResult hit_test(Gfx::FloatPoint const&, HitTestType) const;
+    virtual Optional<HitTestResult> hit_test(Gfx::FloatPoint const&, HitTestType) const;
 
     virtual bool wants_mouse_events() const { return false; }
 

Userland/Libraries/LibWeb/Painting/PaintableBox.cpp

@@ -528,30 +528,31 @@ void PaintableBox::for_each_child_in_paint_order(Callback callback) const
     });
 }
 
-HitTestResult PaintableBox::hit_test(Gfx::FloatPoint const& position, HitTestType type) const
+Optional<HitTestResult> PaintableBox::hit_test(Gfx::FloatPoint const& position, HitTestType type) const
 {
     if (layout_box().is_initial_containing_block_box()) {
         const_cast<Layout::InitialContainingBlock&>(static_cast<Layout::InitialContainingBlock const&>(layout_box())).build_stacking_context_tree_if_needed();
         return stacking_context()->hit_test(position, type);
     }
 
-    HitTestResult result { absolute_border_box_rect().contains(position.x(), position.y()) ? this : nullptr };
+    Optional<HitTestResult> result;
+    if (absolute_border_box_rect().contains(position.x(), position.y()))
+        result = HitTestResult { *this };
     for_each_child_in_paint_order([&](auto& child) {
         if (child.paintable()) {
-            auto child_result = child.paintable()->hit_test(position, type);
+            if (auto child_result = child.paintable()->hit_test(position, type); child_result.has_value())
-            if (child_result.paintable)
+                result = move(child_result);
-                result = child_result;
         }
     });
     return result;
 }
 
-HitTestResult PaintableWithLines::hit_test(const Gfx::FloatPoint& position, HitTestType type) const
+Optional<HitTestResult> PaintableWithLines::hit_test(const Gfx::FloatPoint& position, HitTestType type) const
 {
     if (!layout_box().children_are_inline())
         return PaintableBox::hit_test(position, type);
 
-    HitTestResult last_good_candidate;
+    Optional<HitTestResult> last_good_candidate;
     for (auto& line_box : m_line_boxes) {
         for (auto& fragment : line_box.fragments()) {
             if (is<Layout::Box>(fragment.layout_node()) && static_cast<Layout::Box const&>(fragment.layout_node()).paint_box()->stacking_context())
@@ -559,16 +560,18 @@ HitTestResult PaintableWithLines::hit_test(const Gfx::FloatPoint& position, HitT
             if (fragment.absolute_rect().contains(position)) {
                 if (is<Layout::BlockContainer>(fragment.layout_node()) && fragment.layout_node().paintable())
                     return fragment.layout_node().paintable()->hit_test(position, type);
-                return { fragment.layout_node().paintable(), fragment.text_index_at(position.x()) };
+                return HitTestResult { *fragment.layout_node().paintable(), fragment.text_index_at(position.x()) };
             }
             if (fragment.absolute_rect().top() <= position.y())
-                last_good_candidate = { fragment.layout_node().paintable(), fragment.text_index_at(position.x()) };
+                last_good_candidate = HitTestResult { *fragment.layout_node().paintable(), fragment.text_index_at(position.x()) };
         }
     }
 
-    if (type == HitTestType::TextCursor && last_good_candidate.paintable)
+    if (type == HitTestType::TextCursor && last_good_candidate.has_value())
         return last_good_candidate;
-    return { absolute_border_box_rect().contains(position.x(), position.y()) ? this : nullptr };
+    if (absolute_border_box_rect().contains(position.x(), position.y()))
+        return HitTestResult { *this };
+    return {};
 }
 
 }

Userland/Libraries/LibWeb/Painting/PaintableBox.h

@@ -115,7 +115,7 @@ public:
     virtual void before_children_paint(PaintContext&, PaintPhase) const override;
     virtual void after_children_paint(PaintContext&, PaintPhase) const override;
 
-    virtual HitTestResult hit_test(Gfx::FloatPoint const&, HitTestType) const override;
+    virtual Optional<HitTestResult> hit_test(Gfx::FloatPoint const&, HitTestType) const override;
 
     void invalidate_stacking_context();
 
@@ -166,7 +166,7 @@ public:
     virtual bool wants_mouse_events() const override { return false; }
     virtual bool handle_mousewheel(Badge<EventHandler>, const Gfx::IntPoint&, unsigned buttons, unsigned modifiers, int wheel_delta_x, int wheel_delta_y) override;
 
-    virtual HitTestResult hit_test(Gfx::FloatPoint const&, HitTestType) const override;
+    virtual Optional<HitTestResult> hit_test(Gfx::FloatPoint const&, HitTestType) const override;
 
 protected:
     PaintableWithLines(Layout::BlockContainer const&);

Userland/Libraries/LibWeb/Painting/StackingContext.cpp

@@ -273,7 +273,7 @@ void StackingContext::paint(PaintContext& context) const
     }
 }
 
-HitTestResult StackingContext::hit_test(Gfx::FloatPoint const& position, HitTestType type) const
+Optional<HitTestResult> StackingContext::hit_test(Gfx::FloatPoint const& position, HitTestType type) const
 {
     // FIXME: Use the transform origin specified in CSS or SVG
     auto transform_origin = m_box.paint_box()->absolute_position();
@@ -289,27 +289,27 @@ HitTestResult StackingContext::hit_test(Gfx::FloatPoint const& position, HitTest
         if (child.m_box.computed_values().z_index().value_or(0) < 0)
             break;
         auto result = child.hit_test(transformed_position, type);
-        if (result.paintable)
+        if (result.has_value())
             return result;
     }
 
-    HitTestResult result;
+    Optional<HitTestResult> result;
     // 6. the child stacking contexts with stack level 0 and the positioned descendants with stack level 0.
     m_box.for_each_in_subtree_of_type<Layout::Box>([&](Layout::Box const& box) {
         if (box.is_positioned() && !box.paint_box()->stacking_context()) {
             result = box.paint_box()->hit_test(transformed_position, type);
-            if (result.paintable)
+            if (result.has_value())
                 return IterationDecision::Break;
         }
         return IterationDecision::Continue;
     });
-    if (result.paintable)
+    if (result.has_value())
         return result;
 
     // 5. the in-flow, inline-level, non-positioned descendants, including inline tables and inline blocks.
     if (m_box.children_are_inline() && is<Layout::BlockContainer>(m_box)) {
         auto result = m_box.paint_box()->hit_test(transformed_position, type);
-        if (result.paintable)
+        if (result.has_value())
             return result;
     }
 
@@ -317,23 +317,25 @@ HitTestResult StackingContext::hit_test(Gfx::FloatPoint const& position, HitTest
     m_box.for_each_in_subtree_of_type<Layout::Box>([&](Layout::Box const& box) {
         if (box.is_floating()) {
             result = box.paint_box()->hit_test(transformed_position, type);
-            if (result.paintable)
+            if (result.has_value())
                 return IterationDecision::Break;
         }
         return IterationDecision::Continue;
     });
+    if (result.has_value())
+        return result;
 
     // 3. the in-flow, non-inline-level, non-positioned descendants.
     if (!m_box.children_are_inline()) {
         m_box.for_each_in_subtree_of_type<Layout::Box>([&](Layout::Box const& box) {
             if (!box.is_absolutely_positioned() && !box.is_floating()) {
                 result = box.paint_box()->hit_test(transformed_position, type);
-                if (result.paintable)
+                if (result.has_value())
                     return IterationDecision::Break;
             }
             return IterationDecision::Continue;
         });
-        if (result.paintable)
+        if (result.has_value())
             return result;
     }
 
@@ -343,14 +345,14 @@ HitTestResult StackingContext::hit_test(Gfx::FloatPoint const& position, HitTest
         if (child.m_box.computed_values().z_index().value_or(0) < 0)
             break;
         auto result = child.hit_test(transformed_position, type);
-        if (result.paintable)
+        if (result.has_value())
             return result;
     }
 
     // 1. the background and borders of the element forming the stacking context.
     if (m_box.paint_box()->absolute_border_box_rect().contains(transformed_position)) {
         return HitTestResult {
-            .paintable = m_box.paintable(),
+            .paintable = *m_box.paintable(),
         };
     }
 

Userland/Libraries/LibWeb/Painting/StackingContext.h

@@ -30,7 +30,7 @@ public:
 
     void paint_descendants(PaintContext&, Layout::Node&, StackingContextPaintPhase) const;
     void paint(PaintContext&) const;
-    HitTestResult hit_test(Gfx::FloatPoint const&, HitTestType) const;
+    Optional<HitTestResult> hit_test(Gfx::FloatPoint const&, HitTestType) const;
 
     void dump(int indent = 0) const;