Summary: | [GTK] Duplicate WebKitWebView::show-option-menu confuses introspection, should use --warn-error when building gir | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||
Component: | WebKitGTK | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | annulen, aperez, berto, bugs-noreply, cgarcia, ews-watchlist, gustavo, gyuyoung.kim, mcatanzaro, ryuan.choi, sergio | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=222803 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 178901, 226662 | ||||||||
Attachments: |
|
Description
Michael Catanzaro
2021-03-09 10:00:34 PST
Looks like the WPE version of our introspection is winning: <glib:signal name="show-option-menu" when="last" version="2.28"> <doc xml:space="preserve" filename="../Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp" line="2207">This signal is emitted when a select element in @web_view needs to display a dropdown menu. This signal can be used to show a custom menu, using @menu to get the details of all items that should be displayed. The area of the element in the #WebKitWebView is given as @rectangle parameter, it can be used to position the menu. To handle this signal asynchronously you should keep a ref of the @menu.</doc> <return-value transfer-ownership="none"> <doc xml:space="preserve" filename="../Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp" line="2220">%TRUE to stop other handlers from being invoked for the event. %FALSE to propagate the event further.</doc> <type name="gboolean" c:type="gboolean"/> </return-value> <parameters> <parameter name="object" transfer-ownership="none"> <type name="OptionMenu"/> </parameter> <parameter name="p0" transfer-ownership="none"> <type name="Gdk.Event"/> </parameter> <parameter name="p1" transfer-ownership="none"> <type name="Gdk.Rectangle"/> </parameter> </parameters> </glib:signal> So... that's bad. I've tried various ways of passing -DPLATFORM_GTK to the introspection scanner, but I'm not having much luck. I got it to work by splitting the signal declaration out into the GTK and WPE files. Created attachment 430983 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API (In reply to Michael Catanzaro from comment #1) > </parameter> > <parameter name="p0" transfer-ownership="none"> > <type name="Gdk.Event"/> > </parameter> > <parameter name="p1" transfer-ownership="none"> > <type name="Gdk.Rectangle"/> > </parameter> Heh, it took the *documentation* from the WPE version of the signal, but it took the *parameter list* from the GTK version of the signal. So that could have been a lot worse. Let me revise the commit message. Created attachment 430984 [details]
Patch
With this patch, the generated signal looks much better: <glib:signal name="show-option-menu" when="last" version="2.18"> <doc xml:space="preserve" filename="../../Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp" line="471">This signal is emitted when a select element in @web_view needs to display a dropdown menu. This signal can be used to show a custom menu, using @menu to get the details of all items that should be displayed. The area of the element in the #WebKitWebView is given as @rectangle parameter, it can be used to position the menu. If this was triggered by a user interaction, like a mouse click, @event parameter provides the #GdkEvent. To handle this signal asynchronously you should keep a ref of the @menu. The default signal handler will pop up a #GtkMenu.</doc> <return-value transfer-ownership="none"> <doc xml:space="preserve" filename="../../Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp" line="488">%TRUE to stop other handlers from being invoked for the event. %FALSE to propagate the event further.</doc> <type name="gboolean" c:type="gboolean"/> </return-value> <parameters> <parameter name="menu" transfer-ownership="none"> <doc xml:space="preserve" filename="../../Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp" line="474">the #WebKitOptionMenu</doc> <type name="OptionMenu"/> </parameter> <parameter name="event" transfer-ownership="none"> <doc xml:space="preserve" filename="../../Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp" line="475">the #GdkEvent that triggered the menu, or %NULL</doc> <type name="Gdk.Event"/> </parameter> <parameter name="rectangle" transfer-ownership="none"> <doc xml:space="preserve" filename="../../Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp" line="476">the option element area</doc> <type name="Gdk.Rectangle"/> </parameter> </parameters> </glib:signal> It will break any code that relied on positional parameters with the broken names p0 and p1, but those are obviously-broken names that a reasonable developer should not rely on, and it's very unlikely to be a problem. Comment on attachment 430984 [details] Patch Nice one, thanks for the patch! View in context: https://bugs.webkit.org/attachment.cgi?id=430984&action=review > Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:2177 > + signals[SHOW_OPTION_MENU] = createShowOptionMenuSignal(webViewClass); This looks a bit awkward, but makes sense. I suppose that's only because it's the first and only place where this is being done ;-) Comment on attachment 430984 [details]
Patch
I think we'll need this all over the place for GTK 3 vs. GTK 4, unless we can come up with something better....
Found 1 new test failure: imported/w3c/web-platform-tests/navigation-timing/nav2_test_attributes_values.html Committed r278682 (238659@main): <https://commits.webkit.org/238659@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430984 [details]. *** Bug 206794 has been marked as a duplicate of this bug. *** |