Bug 96948 – [chromium] Enable shouldGesturesTriggerActive for Android

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Rick Byers]

In https://bugs.webkit.org/show_bug.cgi?id=96060 chromium got support for triggering the 'active' state via gesture events.  This is disabled on Android for now due to lack of GestureTapCancel support in the gesture recognizer there.  Once it's added and plumbed into WebKit, shouldGesturesTriggerActive in EventHandler.cpp should be updated to enable the support in Android as well.

[Rick Byers]

Note that this will fix the performance problem with touchstart triggering hover/active states (makes the touchstart hit-test a readonly test).

Created attachment 179331 [details]
Patch

[Rick Byers]

Have you already done the work elsewhere to send GestureTapCancel such that TapDown is always followed by either Tap or TapCancel?  It was the lack of any GestureTapCancel event that led me to have this be disabled on Android.

(In reply to comment #3)
> Have you already done the work elsewhere to send GestureTapCancel such that TapDown is always followed by either Tap or TapCancel?  It was the lack of any GestureTapCancel event that led me to have this be disabled on Android.

In gesture_event_filter, we have added gestureLongPress and gestureDoubleTap to the gestures that can follow a gestureTapDown. So right now TapDown is always folled by Tap, DoubleTap or DoubleTap for us. I think it is still OK even if we don't introduce TapCancel, right?

[Rick Byers]

No, I don't think that's enough.  EventHandler::handleGestureEvent assumes that TapDown is followed by either a Tap or TapCancel.  Otherwise you may find 'active' states getting stuck on (i.e. a missing Release hit test).

We thought about just looking for some set of specific events rather than introducing TapCancel, but we decided that would be too brittle - different platforms may have different events that follow and it's a pain to update them.  So figured it was better to require the platform to explicitly tell us when a Tap is no longer possible.

[James Robinson]

Comment on attachment 179331 [details]
Patch

R- for now based on rbyer's comments that this won't work yet

Comment on attachment 179331 [details]
Patch

We have landed all the changes necessary to handle this properly on Android. All TapDown are followed by a Tap or TapCancel right now.

[James Robinson]

Comment on attachment 179331 [details]
Patch

Any way to test this?

I think the original bug that introduced this change https://bugs.webkit.org/show_bug.cgi?id=96060 has the related tests. I will verify that those tests are running for us and then flip the cq flag.

Comment on attachment 179331 [details]
Patch

Verified that these tests are not disabled for us in TestExpectations chromium and chromium-android. After the change they should start running with the right behavior.

[WebKit Review Bot]

Comment on attachment 179331 [details]
Patch

Rejecting attachment 179331 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 179331, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13563 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
53>At revision 13563.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/16365947

Created attachment 186690 [details]
Patch

Sorry, it looks I had to fix the Changelog. Fixed it and added the test related info on the Changelog. Can you review again and push it to CQ if it looks good?

[WebKit Review Bot]

Comment on attachment 186690 [details]
Patch

Clearing flags on attachment: 186690

Committed r141929: <http://trac.webkit.org/changeset/141929>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.