Don't fire a mousemove event when a modifier key is pressed

https://bugs.webkit.org/show_bug.cgi?id=16271
rdar://81287778

Reviewed by Wenson Hsieh.

Previously, we dispatched mousemove events whenever a modifier key was
pressed. This was used to call the UIClient delegate callback
mouseDidMoveOverElement, which downstream clients could use to modify
various behaviors based on whether or not modifier keys were pressed.

Unfortunately, the act of dispatching a mousemove event meant that
pressing modifier keys could cause unexpected mouse behavior. One notable
example is Slack. When using Slack through Minibrowser, it is possible
to experience spontaneous mouse selection when a cursor was originally
placed behind a completion list, since our hover state gets constantly
re-evaluated.

In this patch, we introduce a mechanism to avoid the need to fire a
mousemove event when modifier keys are pressed. This is achieved by
adding a new message from the UI process to the web process where we
request that a given mouse event's hit test be performed. With the hit
test result and user data associated with said mouse event, we can
simply call into the UIClient delegate callback mouseDidMoveOverElement,
avoiding the regular mouse event dispatch dance altogether.

* LayoutTests/fast/events/no-mousemove-on-modifier-key-press-expected.txt: Added.
* LayoutTests/fast/events/no-mousemove-on-modifier-key-press.html: Added.

Layout test that verifies pressing the modifier keys does not fire a
MouseMove event.

* Source/WebCore/page/Chrome.h:

Export (and make public) Chrome::getToolTip to access from the web
process.

* Source/WebCore/page/EventHandler.cpp:
(WebCore::EventHandler::getHitTypeForMouseMoveEvent):
(WebCore::EventHandler::getHitTestResultForMouseEvent):
(WebCore::EventHandler::handleMouseMoveEvent):

Introduce two functions getHitTypeForMouseMoveEvent and
getHitTestResultForMouseEvent. The latter strips out some common code
from handleMouseMoveEvent so that it can be called independently to
identify a hitType, whereas the former is a handy way to produce a
hitTestResult without bringing the MouseEventWithHitTestResults type
into web process land.

* Source/WebCore/page/EventHandler.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::dispatchMouseDidMoveOverElementAsynchronously):

Introduce a method that communicates the need for user data and a hit
test on a mouse event without actually dispatching said mouse event to
the web process. Once we round trip from the web process, it feeds the
returned data to the client delegate callback mouseDidMoveOverElement.

* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::viewDidMoveToWindow):
(WebKit::WebViewImpl::fireMouseDidMoveOverElement):
(WebKit::WebViewImpl::postFakeMouseMovedEventForFlagsChangedEvent): Deleted.

Rename postFakeMouseMovedEventForFlagsChangedEvent to
fireMouseDidMoveOverElement to better represent our signaling intent.

fireMouseDidMoveOverElement then communicates with the web page to
receive user data and hit test results before passing that over to the
client delegate callback.

* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::performHitTestAndGetUserData):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

Introduce a new message that receives a mouse event from the UI process
and calls into the page's event handler and chrome clients to perform a
hit test on the received mouse event. This allows us to circumvent the
usual dance with a dispatched mouse event while returning the user data
and hit test result associated with a mouse event.

Canonical link: https://commits.webkit.org/264455@main

LayoutTests/fast/events/no-mousemove-on-modifier-key-press-expected.txt

@@ -0,0 +1,15 @@
+This test verifies that MouseMove events are not fired when modifier keys are pressed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+To run this test by hand, press a modifier key.
+
+Dispatched KeyDown event with modifier metaKey
+Dispatched KeyDown event with modifier altKey
+Dispatched KeyDown event with modifier shiftKey
+Dispatched KeyDown event with modifier ctrlKey
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

LayoutTests/fast/events/no-mousemove-on-modifier-key-press.html

@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/ui-helper.js"></script>
+<style>
+.hidden {
+    display: none;
+}
+</style>
+</head>
+
+<body>
+<p id="description"></p>
+<p id="manual-instructions" class="hide">To run this test by hand, press a modifier key.</p>
+<div id="console"></div>
+<script>
+window.jsTestIsAsync = true;
+
+async function fireKeyDownEventsWithModifiers() {
+    for (const modifierKey of ['metaKey', 'altKey', 'shiftKey', 'ctrlKey']) {
+        await UIHelper.keyDown("k", [modifierKey]);
+        debug('Dispatched KeyDown event with modifier ' + modifierKey);
+    }
+}
+
+async function runTest()
+{
+    if (!window.testRunner)
+        document.getElementById("manual-instructions").classList.remove("hidden");
+
+    window.addEventListener('mousemove', () => testFailed("MouseMove event detected"));
+
+    if (window.testRunner)
+        await fireKeyDownEventsWithModifiers();
+    finishJSTest();
+}
+
+description("This test verifies that MouseMove events are not fired when modifier keys are pressed.");
+runTest();
+</script>
+</body>
+</html>

Source/WebCore/page/Chrome.cpp

@@ -340,6 +340,7 @@ void Chrome::setStatusbarText(LocalFrame& frame, const String& status)
     m_client->setStatusbarText(frame.displayStringModifiedByEncoding(status));
 }
 
+// FIXME: Adopt `OptionSet<PlatformEventModifier>` instead of raw integral modifierFlags. https://bugs.webkit.org/show_bug.cgi?id=257175
 void Chrome::mouseDidMoveOverElement(const HitTestResult& result, unsigned modifierFlags)
 {
     auto* localMainFrame = dynamicDowncast<LocalFrame>(m_page.mainFrame());

Source/WebCore/page/Chrome.h

@@ -230,11 +230,11 @@ class Chrome : public HostWindow {
     void registerPopupOpeningObserver(PopupOpeningObserver&);
     void unregisterPopupOpeningObserver(PopupOpeningObserver&);
 
+    WEBCORE_EXPORT void getToolTip(const HitTestResult&, String&, TextDirection&);
+
 private:
     void notifyPopupOpeningObservers() const;
 
-    void getToolTip(const HitTestResult&, String&, TextDirection&);
-
     Page& m_page;
     UniqueRef<ChromeClient> m_client;
     // FIXME: This should be WeakPtr<PopupOpeningObserver>.

Source/WebCore/page/EventHandler.cpp

@@ -1978,6 +1978,42 @@ bool EventHandler::passMouseMovedEventToScrollbars(const PlatformMouseEvent& eve
     return handleMouseMoveEvent(event, &hitTestResult, true);
 }
 
+OptionSet<HitTestRequest::Type> EventHandler::getHitTypeForMouseMoveEvent(const PlatformMouseEvent& platformMouseEvent, bool onlyUpdateScrollbars)
+{
+    OptionSet hitType { HitTestRequest::Type::Move, HitTestRequest::Type::DisallowUserAgentShadowContent, HitTestRequest::Type::AllowFrameScrollbars };
+    if (m_mousePressed)
+        hitType.add(HitTestRequest::Type::Active);
+    else if (onlyUpdateScrollbars) {
+        // Mouse events should be treated as "read-only" if we're updating only scrollbars. This
+        // means that :hover and :active freeze in the state they were in, rather than updating
+        // for nodes the mouse moves while the window is not key (which will be the case if
+        // onlyUpdateScrollbars is true).
+        hitType.add(HitTestRequest::Type::ReadOnly);
+    }
+
+#if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
+    // Treat any mouse move events as readonly if the user is currently touching the screen.
+    if (m_touchPressed) {
+        hitType.add(HitTestRequest::Type::Active);
+        hitType.add(HitTestRequest::Type::ReadOnly);
+    }
+#endif
+
+#if ENABLE(PENCIL_HOVER)
+    if (platformMouseEvent.pointerType() == WebCore::penPointerEventType())
+        hitType.add(WebCore::HitTestRequest::Type::PenEvent);
+#else
+    UNUSED_PARAM(platformMouseEvent);
+#endif
+    return hitType;
+}
+
+HitTestResult EventHandler::getHitTestResultForMouseEvent(const PlatformMouseEvent& platformMouseEvent)
+{
+    HitTestRequest request(getHitTypeForMouseMoveEvent(platformMouseEvent));
+    return prepareMouseEvent(request, platformMouseEvent).hitTestResult();
+}
+
 bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& platformMouseEvent, HitTestResult* hitTestResult, bool onlyUpdateScrollbars)
 {
 #if ENABLE(TOUCH_EVENTS)
@@ -2022,31 +2058,7 @@ bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& platformMouseE
         return m_lastScrollbarUnderMouse->mouseMoved(platformMouseEvent);
 #endif
 
-    OptionSet<HitTestRequest::Type> hitType { HitTestRequest::Type::Move, HitTestRequest::Type::DisallowUserAgentShadowContent, HitTestRequest::Type::AllowFrameScrollbars };
-    if (m_mousePressed)
-        hitType.add(HitTestRequest::Type::Active);
-    else if (onlyUpdateScrollbars) {
-        // Mouse events should be treated as "read-only" if we're updating only scrollbars. This  
-        // means that :hover and :active freeze in the state they were in, rather than updating  
-        // for nodes the mouse moves while the window is not key (which will be the case if 
-        // onlyUpdateScrollbars is true). 
-        hitType.add(HitTestRequest::Type::ReadOnly);
-    }
-
-#if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
-    // Treat any mouse move events as readonly if the user is currently touching the screen.
-    if (m_touchPressed) {
-        hitType.add(HitTestRequest::Type::Active);
-        hitType.add(HitTestRequest::Type::ReadOnly);
-    }
-#endif
-
-#if ENABLE(PENCIL_HOVER)
-    if (platformMouseEvent.pointerType() == WebCore::penPointerEventType())
-        hitType.add(WebCore::HitTestRequest::Type::PenEvent);
-#endif
-
-    HitTestRequest request(hitType);
+    HitTestRequest request(getHitTypeForMouseMoveEvent(platformMouseEvent, onlyUpdateScrollbars));
     MouseEventWithHitTestResults mouseEvent = prepareMouseEvent(request, platformMouseEvent);
     if (hitTestResult)
         *hitTestResult = mouseEvent.hitTestResult();

Source/WebCore/page/EventHandler.h

@@ -209,6 +209,8 @@ class EventHandler {
     WEBCORE_EXPORT void lostMouseCapture();
 
     WEBCORE_EXPORT bool handleMousePressEvent(const PlatformMouseEvent&);
+    WEBCORE_EXPORT OptionSet<HitTestRequest::Type> getHitTypeForMouseMoveEvent(const PlatformMouseEvent&, bool onlyUpdateScrollbars = false);
+    WEBCORE_EXPORT HitTestResult getHitTestResultForMouseEvent(const PlatformMouseEvent&);
     bool handleMouseMoveEvent(const PlatformMouseEvent&, HitTestResult* = nullptr, bool onlyUpdateScrollbars = false);
     WEBCORE_EXPORT bool handleMouseReleaseEvent(const PlatformMouseEvent&);
     bool handleMouseForceEvent(const PlatformMouseEvent&);

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -3186,6 +3186,13 @@ void WebPageProxy::handleMouseEvent(const NativeWebMouseEvent& event)
         processNextQueuedMouseEvent();
 }
 
+void WebPageProxy::dispatchMouseDidMoveOverElementAsynchronously(const NativeWebMouseEvent& event)
+{
+    sendWithAsyncReply(Messages::WebPage::PerformHitTestForMouseEvent { event }, [this, protectedThis = Ref { *this }](WebHitTestResultData&& hitTestResult, OptionSet<WebEventModifier> modifiers, UserData&& userData) {
+        this->mouseDidMoveOverElement(WTFMove(hitTestResult), modifiers.toRaw(), WTFMove(userData));
+    });
+}
+
 void WebPageProxy::processNextQueuedMouseEvent()
 {
     if (!hasRunningProcess())
@@ -6938,8 +6945,7 @@ void WebPageProxy::setStatusText(const String& text)
 void WebPageProxy::mouseDidMoveOverElement(WebHitTestResultData&& hitTestResultData, uint32_t opaqueModifiers, UserData&& userData)
 {
     m_lastMouseMoveHitTestResult = API::HitTestResult::create(hitTestResultData);
-    auto modifiers = OptionSet<WebEventModifier>::fromRaw(opaqueModifiers);
-    m_uiClient->mouseDidMoveOverElement(*this, hitTestResultData, modifiers, m_process->transformHandlesToObjects(userData.object()).get());
+    m_uiClient->mouseDidMoveOverElement(*this, hitTestResultData, OptionSet<WebEventModifier>::fromRaw(opaqueModifiers), m_process->transformHandlesToObjects(userData.object()).get());
     setToolTip(hitTestResultData.toolTipText);
 }
 

Source/WebKit/UIProcess/WebPageProxy.h

@@ -1096,6 +1096,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
     bool isProcessingMouseEvents() const;
     void processNextQueuedMouseEvent();
     void handleMouseEvent(const NativeWebMouseEvent&);
+    void dispatchMouseDidMoveOverElementAsynchronously(const NativeWebMouseEvent&);
 
     void doAfterProcessingAllPendingMouseEvents(Function<void()>&&);
     void didFinishProcessingAllPendingMouseEvents();

Source/WebKit/UIProcess/mac/WebViewImpl.h

@@ -735,8 +735,6 @@ ALLOW_DEPRECATED_DECLARATIONS_END
     void scheduleSetTopContentInsetDispatch();
     void dispatchSetTopContentInset();
 
-    void postFakeMouseMovedEventForFlagsChangedEvent(NSEvent *);
-
     void sendToolTipMouseExited();
     void sendToolTipMouseEntered();
 
@@ -751,6 +749,8 @@ ALLOW_DEPRECATED_DECLARATIONS_END
     void nativeMouseEventHandler(NSEvent *);
     void nativeMouseEventHandlerInternal(NSEvent *);
 
+    void scheduleMouseDidMoveOverElement(NSEvent *);
+
     void mouseMovedInternal(NSEvent *);
     void mouseDownInternal(NSEvent *);
     void mouseUpInternal(NSEvent *);

Source/WebKit/UIProcess/mac/WebViewImpl.mm

@@ -2181,7 +2181,7 @@ static bool isInRecoveryOS()
             WeakPtr weakThis { *this };
             m_flagsChangedEventMonitor = [NSEvent addLocalMonitorForEventsMatchingMask:NSEventMaskFlagsChanged handler:[weakThis] (NSEvent *flagsChangedEvent) {
                 if (weakThis)
-                    weakThis->postFakeMouseMovedEventForFlagsChangedEvent(flagsChangedEvent);
+                    weakThis->scheduleMouseDidMoveOverElement(flagsChangedEvent);
                 return flagsChangedEvent;
             }];
         }
@@ -2306,13 +2306,13 @@ static bool isInRecoveryOS()
     return hitView;
 }
 
-void WebViewImpl::postFakeMouseMovedEventForFlagsChangedEvent(NSEvent *flagsChangedEvent)
+void WebViewImpl::scheduleMouseDidMoveOverElement(NSEvent *flagsChangedEvent)
 {
     NSEvent *fakeEvent = [NSEvent mouseEventWithType:NSEventTypeMouseMoved location:flagsChangedEvent.window.mouseLocationOutsideOfEventStream
         modifierFlags:flagsChangedEvent.modifierFlags timestamp:flagsChangedEvent.timestamp windowNumber:flagsChangedEvent.windowNumber
         context:nullptr eventNumber:0 clickCount:0 pressure:0];
     NativeWebMouseEvent webEvent(fakeEvent, m_lastPressureEvent.get(), m_view.getAutoreleased());
-    m_page->handleMouseEvent(webEvent);
+    m_page->dispatchMouseDidMoveOverElementAsynchronously(webEvent);
 }
 
 WebCore::DestinationColorSpace WebViewImpl::colorSpace()

Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -104,6 +104,7 @@
 #include "WebFullScreenManagerMessages.h"
 #include "WebGamepadProvider.h"
 #include "WebGeolocationClient.h"
+#include "WebHitTestResultData.h"
 #include "WebImage.h"
 #include "WebInspector.h"
 #include "WebInspectorClient.h"
@@ -3395,6 +3396,26 @@ void WebPage::mouseEvent(const WebMouseEvent& mouseEvent, std::optional<Vector<S
     revokeSandboxExtensions(mouseEventSandboxExtensions);
 }
 
+void WebPage::performHitTestForMouseEvent(const WebMouseEvent& event, CompletionHandler<void(WebHitTestResultData&&, OptionSet<WebEventModifier>, UserData&&)>&& completionHandler)
+{
+    RefPtr localMainFrame = dynamicDowncast<WebCore::LocalFrame>(corePage()->mainFrame());
+    if (!localMainFrame || !localMainFrame->view())
+        return;
+
+    auto hitTestResult = localMainFrame->eventHandler().getHitTestResultForMouseEvent(platform(event));
+
+    String toolTip;
+    TextDirection toolTipDirection;
+    corePage()->chrome().getToolTip(hitTestResult, toolTip, toolTipDirection);
+
+    RefPtr<API::Object> userData;
+    WebHitTestResultData hitTestResultData { hitTestResult, toolTip };
+    auto modifiers = event.modifiers();
+    injectedBundleUIClient().mouseDidMoveOverElement(this, hitTestResult, modifiers, userData);
+
+    completionHandler(WTFMove(hitTestResultData), WTFMove(modifiers), UserData(WebProcess::singleton().transformObjectsToHandles(WTFMove(userData).get()).get()));
+}
+
 void WebPage::handleWheelEvent(const WebWheelEvent& event, const OptionSet<WheelEventProcessingSteps>& processingSteps, std::optional<bool> willStartSwipe, CompletionHandler<void(WebCore::ScrollingNodeID, std::optional<WebCore::WheelScrollGestureState>, bool)>&& completionHandler)
 {
 #if ENABLE(ASYNC_SCROLLING)

Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -2038,6 +2038,8 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
     void handleAcceptedCandidate(WebCore::TextCheckingResult);
 #endif
 
+    void performHitTestForMouseEvent(const WebMouseEvent&, CompletionHandler<void(WebHitTestResultData&&, OptionSet<WebEventModifier>, UserData&&)>&&);
+
 #if PLATFORM(COCOA)
     void requestActiveNowPlayingSessionInfo(CompletionHandler<void(bool, bool, const String&, double, double, uint64_t)>&&);
     RetainPtr<NSData> accessibilityRemoteTokenData() const;

Source/WebKit/WebProcess/WebPage/WebPage.messages.in

@@ -573,6 +573,8 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
     DidEndMagnificationGesture()
 #endif
 
+    PerformHitTestForMouseEvent(WebKit::WebMouseEvent event) -> (struct WebKit::WebHitTestResultData hitTestResult, OptionSet<WebKit::WebEventModifier> modifiers, WebKit::UserData messageBody)
+
     EffectiveAppearanceDidChange(bool useDarkAppearance, bool useElevatedUserInterfaceLevel)
 
 #if HAVE(APP_ACCENT_COLORS)