Bug 93045 – Consider removing PlatformEvent::GestureDoubleTap

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

[Rick Byers]

In bug 93044, support is added for handling double tap gestures based on a 'tap count' supplied with the GestureTap.

This makes GestureDoubleTap somewhat redundant and not very useful.  We should probably remove it.

Created attachment 193178 [details]
Patch

[Rick Byers]

Comment on attachment 193178 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193178&action=review

Looks good, but I've got one suggestion for making this a little cleaner.

> Source/WebCore/ChangeLog:3
> +        Consider removing PlatformEvent::GestureDoubleTap

Update bug title to 'remove' now that we're sure it can be done

> Source/WebKit/chromium/src/WebInputEventConversion.cpp:190
> +        m_type = PlatformEvent::NoType;

This is a little odd.  Ideally we could just ensure that PlatformEvent::GestureDoubleTap never got here in the first place.  I know it's used in chromium WebKit for Android (where it probably doesn't make sense to use GestureTap with a tap count check since the semantics are different on Android - DoubleTap is not preceeded by a Tap event since Tap is delayed 300ms).  Perhaps we could cut it off there and never try to create a PlatformGestureEvent for it at all (relying on the ASSERT_NOT_REACHED below as a backstop)?

That would also require landing the Aura gesture-recognizer side change first (removing ET_GESTURE_DOUBLE_TAP) since we're still dispatching that (even though it's redundant).

Created attachment 193336 [details]
Patch

Comment on attachment 193178 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193178&action=review

>> Source/WebKit/chromium/src/WebInputEventConversion.cpp:190
>> +        m_type = PlatformEvent::NoType;
> 
> This is a little odd.  Ideally we could just ensure that PlatformEvent::GestureDoubleTap never got here in the first place.  I know it's used in chromium WebKit for Android (where it probably doesn't make sense to use GestureTap with a tap count check since the semantics are different on Android - DoubleTap is not preceeded by a Tap event since Tap is delayed 300ms).  Perhaps we could cut it off there and never try to create a PlatformGestureEvent for it at all (relying on the ASSERT_NOT_REACHED below as a backstop)?
> 
> That would also require landing the Aura gesture-recognizer side change first (removing ET_GESTURE_DOUBLE_TAP) since we're still dispatching that (even though it's redundant).

Right, in the new patch I drop the WebInputEvent::GestureDoubleTap in WebViewImpl so no GestureDoubleTap will reach WebCore.
Also a chromium patch is under review that will remove the path that can generate WebInputEvent::GestureDoubleTap so to make this
WebInpuEvent -> PlatformEvent conversion safe.

[Rick Byers]

Looks good to me.  You'll need a reviewer to take a look though.  Perhaps tonikitoo?

I don' think any other port was using GestureDoubleTap, but EWS will tell us for sure now that we'll get build errors if anyone tries to use PlatformEvent::GestureDoubleTap.

[WebKit Review Bot]

Comment on attachment 193336 [details]
Patch

Clearing flags on attachment: 193336

Committed r146378: <http://trac.webkit.org/changeset/146378>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.