Bug 117644 – Mouse move events are not flagged as user gestures

• Component: UI Events
• Status: NEW

[Max Feil]

There is an inconsistency in the handling of mouse input events. Mouse pressed and mouse released events are flagged as user gestures, while mouse moved events are not. This may adversely affect code which later processes mouse events.

For example, the following problem came up in the BlackBerry port: To ensure that the user is intentionally adjusting the volume slider of an HTML5 audio element we reject volume changes which are not the result of a user gesture. All touch events and mouse press/release events passed this check. However dragging the slider using a mouse caused all volume changes to be rejected.

I will be proposing a patch to EventHandler::handleMouseMoveEvent() to address this inconsistency. I also want to make sure this is not being done intentionally.

[Max Feil]

Created attachment 204720 [details]
Patch

[Build Bot]

Comment on attachment 204720 [details]
Patch

Attachment 204720 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/899122

New failing tests:
fast/events/popup-blocked-from-mousemove.html

[Build Bot]

Created attachment 204761 [details]
Archive of layout-test-results from APPLE-EWS-2 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-2  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[Build Bot]

Comment on attachment 204720 [details]
Patch

Attachment 204720 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/861157

New failing tests:
fast/events/popup-blocked-from-mousemove.html

[Build Bot]

Created attachment 204762 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Sam Weinig]

This behavior is intentional (we even test it and you patch causes that test to fail, fast/events/popup-blocked-from-mousemove.html).  The idea is that moving the mouse over the page is unlikely to indicate user intent (is probably just the user moving the mouse over the screen).

If you are running into issues with drag, there may be a more limiting fix.

[Max Feil]

Created attachment 204779 [details]
Patch

[Max Feil]

(In reply to comment #6)
> The idea is that moving the mouse over the page is unlikely to indicate user intent (is probably just the user moving the mouse over the screen).

Thanks for pointing this out. I've modified my patch to treat mouse move as a user gesture only while a mouse button is pressed.

[Build Bot]

Comment on attachment 204779 [details]
Patch

Attachment 204779 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/922302

New failing tests:
fast/media/mq-transform-03.html
fast/media/mq-transform-02.html
platform/win/accessibility/multiple-select-element-role.html

[Build Bot]

Created attachment 204788 [details]
Archive of layout-test-results from APPLE-EWS-3 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-3  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[Max Feil]

(In reply to comment #9)
> New failing tests:
> fast/media/mq-transform-03.html
> fast/media/mq-transform-02.html
> platform/win/accessibility/multiple-select-element-role.html

I don't see how these failures could be from my patch. Also, these tests passed for my previous patch which was more invasive.

[Max Feil]

(In reply to comment #11)
> I don't see how these failures could be from my patch. Also, these tests passed for my previous patch which was more invasive.

Actually, these failures are happening for other people's patches as well, for example https://webkit-queues.appspot.com/patch/204773. So my patch should be considered as not introducing any new test failures.

[Alexey Proskuryakov]

What is m_lastMouseDownUserGestureToken? I suspect that the "else" part may not be needed or accurate.

[Max Feil]

(In reply to comment #13)
> What is m_lastMouseDownUserGestureToken? I suspect that the "else" part may not be needed or accurate.

m_lastMouseDownUserGestureToken is the token saved by EventHandler::handleMousePressEvent(). My patch follows the pattern of existing code in EventHandler::handleMouseReleaseEvent() which looks like:

    OwnPtr<UserGestureIndicator> gestureIndicator;

    if (m_lastMouseDownUserGestureToken)
        gestureIndicator = adoptPtr(new UserGestureIndicator(m_lastMouseDownUserGestureToken.release()));
    else
        gestureIndicator = adoptPtr(new UserGestureIndicator(DefinitelyProcessingUserGesture));


I assume the else is needed to handle the possibility of spurious or out-of-order events, for example those generated when a window gets focus with the mouse button already pressed. In that case you can get a release without a matching preceding press. Although I agree for my patch the "if (m_mousePressed)" check all but eliminates the need for the else.

[Sam Weinig]

This needs tests.

[Max Feil]

Created attachment 204805 [details]
Patch

[Max Feil]

(In reply to comment #16)
> Created an attachment (id=204805) [details]
> Patch

My latest patch addresses all comments thus far and includes a test.

[Build Bot]

Comment on attachment 204805 [details]
Patch

Attachment 204805 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/916197

New failing tests:
fast/events/popup-allowed-from-mouse-drag.html
fast/repaint/table-cell-collapsed-border-scroll.html

[Build Bot]

Created attachment 204808 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Max Feil]

Created attachment 204855 [details]
Patch

[Max Feil]

(In reply to comment #20)
> Created an attachment (id=204855) [details]
> Patch

Latest patch makes corrections to the layout test.

[Brent Fulgham]

Marking current patch r- due to its age (will not apply against current tree). If you rebaseline the patch against current ToT we can complete the review process.