LibWeb: Update cursor and tooltip UI state regardless of event handlers

If a script on the page cancels a mousemove event, we would return early
and neglect to update the cursor. This is seen regularly on "Diablo Web"
where they set the cursor to "none" on the main canvas, and also cancel
mousemove events.

Author: trflynn89

Libraries/LibWeb/Internals/Internals.cpp

@@ -6,6 +6,7 @@
  */
 
 #include <AK/JsonObject.h>
+#include <LibGfx/Cursor.h>
 #include <LibJS/Runtime/Date.h>
 #include <LibJS/Runtime/VM.h>
 #include <LibUnicode/TimeZone.h>
@@ -236,6 +237,20 @@ void Internals::pinch(double x, double y, double scale_delta)
     page.handle_pinch_event(position, scale_delta);
 }
 
+String Internals::current_cursor()
+{
+    auto& page = this->page();
+
+    return page.current_cursor().visit(
+        [](Gfx::StandardCursor cursor) {
+            auto cursor_string = Gfx::standard_cursor_to_string(cursor);
+            return String::from_utf8_without_validation(cursor_string.bytes());
+        },
+        [](Gfx::ImageCursor const&) {
+            return "Image"_string;
+        });
+}
+
 WebIDL::ExceptionOr<bool> Internals::dispatch_user_activated_event(DOM::EventTarget& target, DOM::Event& event)
 {
     event.set_is_trusted(true);

Libraries/LibWeb/Internals/Internals.h

@@ -43,6 +43,8 @@ class WEB_API Internals final : public InternalsBase {
     void wheel(double x, double y, double delta_x, double delta_y);
     void pinch(double x, double y, double scale_delta);
 
+    String current_cursor();
+
     WebIDL::ExceptionOr<bool> dispatch_user_activated_event(DOM::EventTarget&, DOM::Event& event);
 
     void spoof_current_url(String const& url);

Libraries/LibWeb/Internals/Internals.idl

@@ -35,6 +35,8 @@ interface Internals {
     undefined wheel(double x, double y, double deltaX, double deltaY);
     undefined pinch(double x, double y, double scaleDelta);
 
+    DOMString currentCursor();
+
     boolean dispatchUserActivatedEvent(EventTarget target, Event event);
     undefined spoofCurrentURL(USVString url);
 

Libraries/LibWeb/Page/EventHandler.cpp

@@ -748,6 +748,8 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint visual_viewport_positio
         return EventResult::Dropped;
 
     auto& document = *m_navigable->active_document();
+    auto& page = m_navigable->page();
+
     auto viewport_position = document.visual_viewport()->map_to_layout_viewport(visual_viewport_position);
 
     m_navigable->active_document()->update_layout(DOM::UpdateLayoutReason::EventHandlerHandleMouseMove);
@@ -756,8 +758,8 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint visual_viewport_positio
         return EventResult::Dropped;
 
     bool hovered_node_changed = false;
-    bool is_hovering_link = false;
     Gfx::Cursor hovered_node_cursor = Gfx::StandardCursor::None;
+    GC::Ptr<HTML::HTMLAnchorElement const> hovered_link_element;
 
     GC::Ptr<Painting::Paintable> paintable;
     Optional<int> start_index;
@@ -769,11 +771,42 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint visual_viewport_positio
 
     GC::Ptr<DOM::Node> node;
 
-    ScopeGuard update_hovered_node_guard = [&node, &document] {
+    ScopeGuard update_hovered_node_and_ui_state_guard = [&] {
         document.set_hovered_node(node);
+
+        // FIXME: This check is only approximate. ImageCursors from the same CursorStyleValue share bitmaps, but may
+        //        repaint them. So comparing them does not tell you if they are the same image. Also, the image may
+        //        change even if the hovered node does not.
+        if (page.current_cursor() != hovered_node_cursor || hovered_node_changed) {
+            page.client().page_did_request_cursor_change(hovered_node_cursor);
+            page.set_current_cursor(hovered_node_cursor);
+        }
+
+        if (hovered_node_changed) {
+            GC::Ptr<HTML::HTMLElement const> hovered_html_element = node
+                ? node->enclosing_html_element_with_attribute(HTML::AttributeNames::title)
+                : nullptr;
+
+            if (hovered_html_element && hovered_html_element->title().has_value()) {
+                page.client().page_did_enter_tooltip_area(hovered_html_element->title()->to_byte_string());
+                page.set_is_in_tooltip_area(true);
+            } else if (page.is_in_tooltip_area()) {
+                page.client().page_did_leave_tooltip_area();
+                page.set_is_in_tooltip_area(false);
+            }
+
+            if (hovered_link_element) {
+                if (auto link_url = document.encoding_parse_url(hovered_link_element->href()); link_url.has_value()) {
+                    page.client().page_did_hover_link(*link_url);
+                    page.set_is_hovering_link(true);
+                }
+            } else if (page.is_hovering_link()) {
+                page.client().page_did_unhover_link();
+                page.set_is_hovering_link(false);
+            }
+        }
     };
 
-    GC::Ptr<HTML::HTMLAnchorElement const> hovered_link_element;
     if (paintable) {
         if (paintable->wants_mouse_events()) {
             if (paintable->handle_mousemove({}, viewport_position, buttons, modifiers) == Painting::Paintable::DispatchEventOfSameName::No) {
@@ -782,7 +815,7 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint visual_viewport_positio
             }
 
             // 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);
+            page.client().page_did_request_cursor_change(Gfx::StandardCursor::None);
         }
 
         node = dom_node_for_event_dispatch(*paintable);
@@ -804,10 +837,9 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint visual_viewport_positio
         GC::Ptr<Layout::Node> layout_node;
         bool found_parent_element = parent_element_for_event_dispatch(*paintable, node, layout_node);
         hovered_node_changed = node.ptr() != document.hovered_node();
+
         if (found_parent_element) {
             hovered_link_element = node->enclosing_link_element();
-            if (hovered_link_element)
-                is_hovering_link = true;
 
             if (paintable->layout_node().is_text_node()) {
                 hovered_node_cursor = resolve_cursor(*paintable->layout_node().parent(), cursor_data, Gfx::StandardCursor::IBeam);
@@ -856,38 +888,6 @@ EventResult EventHandler::handle_mousemove(CSSPixelPoint visual_viewport_positio
         }
     }
 
-    auto& page = m_navigable->page();
-
-    // FIXME: This check is only approximate. ImageCursors from the same CursorStyleValue share bitmaps, but may repaint them.
-    //        So comparing them does not tell you if they are the same image. Also, the image may change even if the hovered
-    //        node does not.
-    if (page.current_cursor() != hovered_node_cursor || hovered_node_changed) {
-        page.set_current_cursor(hovered_node_cursor);
-        page.client().page_did_request_cursor_change(hovered_node_cursor);
-    }
-
-    if (hovered_node_changed) {
-        GC::Ptr<HTML::HTMLElement const> hovered_html_element = node ? node->enclosing_html_element_with_attribute(HTML::AttributeNames::title) : nullptr;
-
-        if (hovered_html_element && hovered_html_element->title().has_value()) {
-            page.set_is_in_tooltip_area(true);
-            page.client().page_did_enter_tooltip_area(hovered_html_element->title()->to_byte_string());
-        } else if (page.is_in_tooltip_area()) {
-            page.set_is_in_tooltip_area(false);
-            page.client().page_did_leave_tooltip_area();
-        }
-
-        if (is_hovering_link) {
-            if (auto link_url = document.encoding_parse_url(hovered_link_element->href()); link_url.has_value()) {
-                page.set_is_hovering_link(true);
-                page.client().page_did_hover_link(*link_url);
-            }
-        } else if (page.is_hovering_link()) {
-            page.set_is_hovering_link(false);
-            page.client().page_did_unhover_link();
-        }
-    }
-
     return EventResult::Handled;
 }
 

Tests/LibWeb/Text/expected/css/cursor-applied-when-cancelled.txt

@@ -0,0 +1,4 @@
+Arrow
+OpenHand
+Arrow
+OpenHand

Tests/LibWeb/Text/input/css/cursor-applied-when-cancelled.html

@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<style>
+    #foo {
+        width: 100px;
+        height: 100px;
+
+        position: absolute;
+        top: 20px;
+        left: 20px;
+
+        cursor: grab;
+        background-color: red;
+    }
+</style>
+<div id="foo"></div>
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        internals.movePointerTo(0, 0);
+        println(internals.currentCursor());
+
+        internals.movePointerTo(100, 100);
+        println(internals.currentCursor());
+
+        foo.addEventListener("mousemove", e => e.preventDefault());
+
+        internals.movePointerTo(0, 0);
+        println(internals.currentCursor());
+
+        internals.movePointerTo(100, 100);
+        println(internals.currentCursor());
+    });
+</script>