Bug 233785 - [GTK4] WebKitWebView::context-menu errors out on GTK4
Summary: [GTK4] WebKitWebView::context-menu errors out on GTK4
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 233997 (view as bug list)
Depends on:
Blocks: GTK4
  Show dependency treegraph
 
Reported: 2021-12-02 15:19 PST by Alice Mikhaylenko
Modified: 2022-07-28 07:13 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alice Mikhaylenko 2021-12-02 15:19:40 PST
(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.
Comment 1 Michael Catanzaro 2021-12-02 17:50:50 PST
(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?
Comment 2 Alice Mikhaylenko 2021-12-02 23:17:52 PST
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.
Comment 3 Michael Catanzaro 2021-12-03 06:49:19 PST
(Keep in mind there is a web process version of this signal as well, which doesn't have the GdkEvent parameter.)
Comment 4 Michael Catanzaro 2021-12-15 06:47:34 PST
*** Bug 233997 has been marked as a duplicate of this bug. ***
Comment 5 Alice Mikhaylenko 2022-03-29 04:14:20 PDT
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.
Comment 6 Michael Catanzaro 2022-03-29 10:05:18 PDT
(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?
Comment 7 Adrian Perez 2022-06-12 14:08:20 PDT
(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.
Comment 8 Alice Mikhaylenko 2022-06-29 12:40:13 PDT
Emmanuele mentioned we can keep GdkEvent if we specify a custom marshaller.

I still think we should replace it though.
Comment 9 Patrick Griffis 2022-06-29 12:52:05 PDT
Epiphany uses button and state from the event to pass along to WebExtensions.
Comment 10 Alice Mikhaylenko 2022-06-29 12:55:51 PDT
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.
Comment 11 Alice Mikhaylenko 2022-07-22 21:40:22 PDT
Pull request: https://github.com/WebKit/WebKit/pull/2686
Comment 12 EWS 2022-07-28 07:12:55 PDT
Committed 252908@main (7f7932695192): <https://commits.webkit.org/252908@main>

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