Bug 140747 – [GTK] Support using long-tap gesture to open context menu

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Adrian Perez]

Use GestureController/GtkGesture to bind long-tap to the context
menu (Note: initial support for gesture was already committed as
of bug #137812).

[Jan-Michael Brummer]

Created attachment 332256 [details]
Patch adding long press gesture

Adding a patch to open context menu through long press gesture.

It simulates a secondary mouse button press to open the menu.

[Adrian Perez]

Comment on attachment 332256 [details]
Patch adding long press gesture

Thanks a lot for the patch! I have taken a quick look, and it
looks correct to me. Note that I am reviewing informally, so we
will still need the actual r+ from an actual reviewer. Carlos
García (in CC) is a has touched the gestures code before (IIRC),
so probably he can rubber-stamp the patch for landing :-)

[Michael Catanzaro]

Comment on attachment 332256 [details]
Patch adding long press gesture

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

It looks good to me too, but seems good to give Carlos a chance to review it indeed.

> Source/WebKit/UIProcess/gtk/GestureController.h:125
> +        LongPressGesture(WebPageProxy&);

explicit LongPressGesture(WebPageProxy&);

[Carlos Garcia Campos]

Comment on attachment 332256 [details]
Patch adding long press gesture

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

> Source/WebKit/UIProcess/gtk/GestureController.cpp:302
> +    GUniquePtr<GdkEvent> pointerEvent(gdk_event_new(GDK_MOTION_NOTIFY));
> +    pointerEvent->motion.time = event->touch.time;
> +    pointerEvent->motion.x = event->touch.x;
> +    pointerEvent->motion.y = event->touch.y;
> +    pointerEvent->motion.x_root = event->touch.x_root;
> +    pointerEvent->motion.y_root = event->touch.y_root;
> +    pointerEvent->motion.state = event->touch.state;
> +    m_page.handleMouseEvent(NativeWebMouseEvent(pointerEvent.get(), 0));
> +
> +    pointerEvent.reset(gdk_event_new(GDK_BUTTON_PRESS));
> +    pointerEvent->button.button = GDK_BUTTON_SECONDARY;
> +    pointerEvent->button.time = event->touch.time;
> +    pointerEvent->button.x = event->touch.x;
> +    pointerEvent->button.y = event->touch.y;
> +    pointerEvent->button.x_root = event->touch.x_root;
> +    pointerEvent->button.y_root = event->touch.y_root;
> +    m_page.handleMouseEvent(NativeWebMouseEvent(pointerEvent.get(), 1));
> +
> +    pointerEvent->type = GDK_BUTTON_RELEASE;
> +    m_page.handleMouseEvent(NativeWebMouseEvent(pointerEvent.get(), 0));

This code is duplicated in tap gesture. We could move it to a helper function that simply receives the button (primary or secondary).

[Jan-Michael Brummer]

Created attachment 332370 [details]
Patch adding long press gesture - V2

Refactor mouse simulation into helper function.

[Michael Catanzaro]

Comment on attachment 332370 [details]
Patch adding long press gesture - V2

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

> Source/WebKit/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

You'll need to set the r? and cq? Bugzilla flags

[Carlos Garcia Campos]

Comment on attachment 332370 [details]
Patch adding long press gesture - V2

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

> Source/WebKit/UIProcess/gtk/GestureController.cpp:89
> +void GestureController::Gesture::simulateMousePress(const GdkEvent *event, int button_type)

I would call this Click instead of Press since this is Motion + Press + Release. 

const GdkEvent *event -> const GdkEvent* event
int button_type -> unsigned buttonType (or just button, but don't use underscore).

[Jan-Michael Brummer]

Created attachment 332372 [details]
Patch adding long press gesture - V3

Updated version based on latest review.

[WebKit Commit Bot]

Comment on attachment 332372 [details]
Patch adding long press gesture - V3

Clearing flags on attachment: 332372

Committed r227675: <https://trac.webkit.org/changeset/227675>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.