Bug 233785 – [GTK4] WebKitWebView::context-menu errors out on GTK4

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alice Mikhaylenko]

(epiphany:2): GLib-GObject-WARNING **: 04:08:46.838: value_to_ffi_type: Unsupported fundamental type: GdkEvent

I suppose we could use a custom marshaller, but - how important is the event parameter anyway? Epiphany never uses it, for instance. In the past it had taken this event and carried it around, but it never actually did anything with it either.

[Michael Catanzaro]

(In reply to Alexander Mikhaylenko from comment #0)
> how important is the event parameter anyway? Epiphany never uses it, for instance.

I think you would need this if you were going to present a window, so that you could use the timestamp from the event?

[Alice Mikhaylenko]

You don't need a timestamp for showing a popover though. But even then,  if you want to present a toplevel when trying to open a context menu - `GDK_CURRENT_TIME` exists.

However, when writing this I didn't notice the signal doesn't pass coordinates to show the menu at otherwise (in case you want to make a completely custom menu), so I guess `GdkEvent` makes sense for that... Though at worst it could be x, y, timestamp.

[Michael Catanzaro]

(Keep in mind there is a web process version of this signal as well, which doesn't have the GdkEvent parameter.)

[Michael Catanzaro]

*** Bug 233997 has been marked as a duplicate of this bug. ***

[Alice Mikhaylenko]

This one is tricky - since it differs between GTK3 and GTK4, we have to split the signal and the vfunc between gtk3 and gtk4 somehow... This likely involves reorganizing WebKitWebView.c/h completely.

[Michael Catanzaro]

(In reply to Alexander Mikhaylenko from comment #5)
> This one is tricky - since it differs between GTK3 and GTK4, we have to
> split the signal and the vfunc between gtk3 and gtk4 somehow... This likely
> involves reorganizing WebKitWebView.c/h completely.

It shouldn't require a huge reorg. Remember that we are first waiting for gi-docgen support to land. Then we can move the doc comments that need to differ between GTK3/GTK4/WPE out into separate files, while leaving the code in one place. With this plan, #if won't work for the doc comments, but it will work for the code. You can just leave a non-doc comment where the doc comment would normally be, pointing to the file that holds the real doc comments.

...right?

[Adrian Perez]

(In reply to Michael Catanzaro from comment #6)
> (In reply to Alexander Mikhaylenko from comment #5)
> > This one is tricky - since it differs between GTK3 and GTK4, we have to
> > split the signal and the vfunc between gtk3 and gtk4 somehow... This likely
> > involves reorganizing WebKitWebView.c/h completely.
> 
> It shouldn't require a huge reorg. Remember that we are first waiting for
> gi-docgen support to land. Then we can move the doc comments that need to
> differ between GTK3/GTK4/WPE out into separate files, while leaving the code
> in one place. With this plan, #if won't work for the doc comments, but it
> will work for the code. You can just leave a non-doc comment where the doc
> comment would normally be, pointing to the file that holds the real doc
> comments.
> 
> ...right?

Support for gi-docgen landed in #226662 a while ago, but we still need
to go over the documentation and make sure all works fine for GTK4 builds.
Now we should be closer to having this fixed.

[Alice Mikhaylenko]

Emmanuele mentioned we can keep GdkEvent if we specify a custom marshaller.

I still think we should replace it though.

[Patrick Griffis]

Epiphany uses button and state from the event to pass along to WebExtensions.

[Alice Mikhaylenko]

Ok yeah, it _just_ landed and I didn't have it in my local checkout.

Fair enough, let's keep it then and have a custom marshaller.

[Alice Mikhaylenko]

Pull request: https://github.com/WebKit/WebKit/pull/2686

[EWS]

Committed 252908@main (7f7932695192): <https://commits.webkit.org/252908@main>

Reviewed commits have been landed. Closing PR #2686 and removing active labels.