NEW 117644
Mouse move events are not flagged as user gestures
https://bugs.webkit.org/show_bug.cgi?id=117644
Summary Mouse move events are not flagged as user gestures
Max Feil
Reported 2013-06-14 09:07:59 PDT
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.
Attachments
Patch (2.01 KB, patch)
2013-06-14 10:44 PDT, Max Feil
no flags
Archive of layout-test-results from APPLE-EWS-2 for win-future (793.41 KB, application/zip)
2013-06-14 23:27 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (603.51 KB, application/zip)
2013-06-15 00:01 PDT, Build Bot
no flags
Patch (2.12 KB, patch)
2013-06-15 20:01 PDT, Max Feil
no flags
Archive of layout-test-results from APPLE-EWS-3 for win-future (787.56 KB, application/zip)
2013-06-16 05:47 PDT, Build Bot
no flags
Patch (4.38 KB, patch)
2013-06-17 03:18 PDT, Max Feil
no flags
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (536.52 KB, application/zip)
2013-06-17 04:27 PDT, Build Bot
no flags
Patch (4.57 KB, patch)
2013-06-17 14:36 PDT, Max Feil
bfulgham: review-
Max Feil
Comment 1 2013-06-14 10:44:48 PDT
Build Bot
Comment 2 2013-06-14 23:27:07 PDT
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
Comment 3 2013-06-14 23:27:08 PDT
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 4 2013-06-15 00:01:46 PDT
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
Comment 5 2013-06-15 00:01:49 PDT
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
Comment 6 2013-06-15 18:53:33 PDT
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
Comment 7 2013-06-15 20:01:42 PDT
Max Feil
Comment 8 2013-06-15 20:03:44 PDT
(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 9 2013-06-16 05:47:25 PDT
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
Comment 10 2013-06-16 05:47:27 PDT
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
Comment 11 2013-06-16 14:06:18 PDT
(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
Comment 12 2013-06-16 14:17:54 PDT
(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
Comment 13 2013-06-16 14:18:54 PDT
What is m_lastMouseDownUserGestureToken? I suspect that the "else" part may not be needed or accurate.
Max Feil
Comment 14 2013-06-16 14:29:00 PDT
(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
Comment 15 2013-06-16 17:44:43 PDT
This needs tests.
Max Feil
Comment 16 2013-06-17 03:18:32 PDT
Max Feil
Comment 17 2013-06-17 03:19:52 PDT
(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 18 2013-06-17 04:27:06 PDT
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
Comment 19 2013-06-17 04:27:08 PDT
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
Comment 20 2013-06-17 14:36:50 PDT
Max Feil
Comment 21 2013-06-17 14:40:05 PDT
(In reply to comment #20) > Created an attachment (id=204855) [details] > Patch Latest patch makes corrections to the layout test.
Brent Fulgham
Comment 22 2016-03-14 10:46:35 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.