WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
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
Details
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
Details
Patch
(2.12 KB, patch)
2013-06-15 20:01 PDT
,
Max Feil
no flags
Details
Formatted Diff
Diff
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
Details
Patch
(4.38 KB, patch)
2013-06-17 03:18 PDT
,
Max Feil
no flags
Details
Formatted Diff
Diff
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
Details
Patch
(4.57 KB, patch)
2013-06-17 14:36 PDT
,
Max Feil
bfulgham
: review-
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Max Feil
Comment 1
2013-06-14 10:44:48 PDT
Created
attachment 204720
[details]
Patch
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
Created
attachment 204779
[details]
Patch
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
Created
attachment 204805
[details]
Patch
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
Created
attachment 204855
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug