From b60d465029609e1dc229df19e9d2688b9afece47 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 2 Aug 2025 14:56:57 +0200 Subject: [PATCH] LibWeb: Defer updating hovered node until end of mouse event handling Updating the hovered node may fire events, and so we can't assume the layout and paintable nodes we've found via hit testing will be valid after doing it. --- Libraries/LibWeb/Page/EventHandler.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Libraries/LibWeb/Page/EventHandler.cpp b/Libraries/LibWeb/Page/EventHandler.cpp index def8e555a65..f84912b438a 100644 --- a/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Libraries/LibWeb/Page/EventHandler.cpp @@ -608,6 +608,10 @@ EventResult EventHandler::handle_mousedown(CSSPixelPoint viewport_position, CSSP GC::Ref document = *m_navigable->active_document(); GC::Ptr node; + ScopeGuard update_hovered_node_guard = [&node, &document] { + document->set_hovered_node(node); + }; + { GC::Ptr paintable; if (auto result = target_for_mouse_position(viewport_position); result.has_value()) @@ -620,7 +624,6 @@ EventResult EventHandler::handle_mousedown(CSSPixelPoint viewport_position, CSSP VERIFY(pointer_events != CSS::PointerEvents::None); node = dom_node_for_event_dispatch(*paintable); - document->set_hovered_node(node); if (paintable->wants_mouse_events()) { if (paintable->handle_mousedown({}, viewport_position, button, modifiers) == Painting::Paintable::DispatchEventOfSameName::No) @@ -745,18 +748,25 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint viewport_position, CSSP start_index = result->index_in_node; } + GC::Ptr node; + + ScopeGuard update_hovered_node_guard = [&node, &document] { + document.set_hovered_node(node); + }; + GC::Ptr hovered_link_element; if (paintable) { if (paintable->wants_mouse_events()) { - document.set_hovered_node(paintable->dom_node()); - if (paintable->handle_mousemove({}, viewport_position, buttons, modifiers) == Painting::Paintable::DispatchEventOfSameName::No) + if (paintable->handle_mousemove({}, viewport_position, buttons, modifiers) == Painting::Paintable::DispatchEventOfSameName::No) { + node = paintable->dom_node(); return EventResult::Cancelled; + } // FIXME: It feels a bit aggressive to always update the cursor like this. m_navigable->page().client().page_did_request_cursor_change(Gfx::StandardCursor::None); } - auto node = dom_node_for_event_dispatch(*paintable); + node = dom_node_for_event_dispatch(*paintable); if (node && is(*node)) { if (auto content_navigable = static_cast(*node).content_navigable()) @@ -775,7 +785,6 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint viewport_position, CSSP GC::Ptr layout_node; bool found_parent_element = parent_element_for_event_dispatch(*paintable, node, layout_node); hovered_node_changed = node.ptr() != document.hovered_node(); - document.set_hovered_node(node); if (found_parent_element) { hovered_link_element = node->enclosing_link_element(); if (hovered_link_element)