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.
Created attachment 204720 [details] Patch
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
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
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
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
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.
Created attachment 204779 [details] Patch
(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.
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
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
(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.
(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.
What is m_lastMouseDownUserGestureToken? I suspect that the "else" part may not be needed or accurate.
(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.
This needs tests.
Created attachment 204805 [details] Patch
(In reply to comment #16) > Created an attachment (id=204805) [details] > Patch My latest patch addresses all comments thus far and includes a test.
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
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
Created attachment 204855 [details] Patch
(In reply to comment #20) > Created an attachment (id=204855) [details] > Patch Latest patch makes corrections to the layout test.
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.