REGRESSION(r227544): [GTK] contextMenuEvent is NULL on CONTEXT_MENU call

[Jan-Michael Brummer]

After adding support for long press context menu epiphany crashes due to contextMenuEvent not beeing set (only called in button-press handler).

[Jan-Michael Brummer]

Created attachment 332492 [details]
Patch fixing contextMenuEvent

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Comment on attachment 332492 [details]
Patch fixing contextMenuEvent

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

We need to update the documentation of WebKitWebView::context-menu as well, to indicate that event may now be a GdkEventTouch of type GDK_TOUCH_END. (It would be GDK_TOUCH_END, right?)

Then I'll ask Carlos Garcia to do the final review, because this could in theory break applications depending on the assumptions they make about the type of the GdkEvent... I'd like to do this anyway, since I think it will be fine in practice.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:936
> +        priv->contextMenuEvent.reset(gdk_event_copy(touchEvent));

Ah, that was easier than I expected.

[Carlos Garcia Campos]

Comment on attachment 332492 [details]
Patch fixing contextMenuEvent

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

>> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:936
>> +        priv->contextMenuEvent.reset(gdk_event_copy(touchEvent));
> 
> Ah, that was easier than I expected.

I'm not sure this is the right fix, though. I was surprised we were missing the event, while we were simulating the click, but then I realized that GestureController is bypassing the WebKitWebViewBase. This means we are not doing the same in both cases. For example, a tap is not focusing the view, while a real button press is. In case of gestures we are not notifying the input method filter either. I think the event handling should be done by WebKitWebViewBase to ensure consistency. I would add a GestureControllerClient, implemented by a helper class in WebKitWebViewBase that has methods like handleTap(), handleDrag(), handleLongPress(), etc, that receive the touch event and any other ting required to handle it. Then the client decides how to handle that, doing the conversion and sending the events to the page. If I'm asking too much, I can do it if you prefer.

[Jan-Michael Brummer]

Please proceed, as my vacation is over and i would love to see this feature in a stable release soon :)

I can of course test the changes.

[Carlos Garcia Campos]

Created attachment 332639 [details]
Patch

[Michael Catanzaro]

Comment on attachment 332639 [details]
Patch

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

OK, thanks. Let's try it.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1001
> +    ViewGestureController(WebKitWebViewBase* webViewBase)

explicit

[Carlos Garcia Campos]

Committed r227893: <https://trac.webkit.org/changeset/227893>

[Michael Catanzaro]

Regression: bug #182350