Bug 173765 - [GTK] Introspection: webkit_web_view_new_with_related_view needs to be marked as a constructor
Summary: [GTK] Introspection: webkit_web_view_new_with_related_view needs to be marked...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Minor
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-23 04:18 PDT by Matijs van Zuijlen
Modified: 2017-06-24 08:24 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.40 KB, patch)
2017-06-23 19:07 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (2.34 KB, patch)
2017-06-24 05:40 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matijs van Zuijlen 2017-06-23 04:18:07 PDT
The webkit_web_view_new_with_related_view function works as a constructor, but gobject-introspection marks it as a method, possibly due to its first parameter. Please consider annotating it as a constructor.

This function is currently defined here: https://trac.webkit.org/browser/webkit/trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGtk.cpp#L210

This report is a follow-up from this comment on a bug in Ruby-GNOME2: https://github.com/ruby-gnome2/ruby-gnome2/pull/1028#issuecomment-299650859
Comment 1 Michael Catanzaro 2017-06-23 15:39:39 PDT
Hm, maybe we should use this annotation for all constructors, just to be on the safe side.
Comment 2 Michael Catanzaro 2017-06-23 17:52:57 PDT
OK, I see the problem here is only because the first parameter is of the same type as the class, so this is probably the only problematic constructor in our API and it's not very likely we would accidentally add another in the future.

Unfortunately it's a significant introspection API break, and I've no doubt changing it will break some real applications. But the status quo is obviously broken, so I guess anyone who has used this constructor so far and not reported the breakage will just have to deal with it.
Comment 3 Michael Catanzaro 2017-06-23 19:07:09 PDT
Created attachment 313764 [details]
Patch
Comment 4 Michael Catanzaro 2017-06-23 19:08:27 PDT
I think it's going to need to be called out in the 2.18 release notes and not backported.
Comment 5 Build Bot 2017-06-23 19:09:29 PDT
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
Comment 6 Carlos Garcia Campos 2017-06-24 01:11:35 PDT
WPE doesn't support introspection yet.
Comment 7 Michael Catanzaro 2017-06-24 05:40:18 PDT
Created attachment 313785 [details]
Patch
Comment 8 WebKit Commit Bot 2017-06-24 08:14:44 PDT
Comment on attachment 313785 [details]
Patch

Clearing flags on attachment: 313785

Committed r218787: <http://trac.webkit.org/changeset/218787>
Comment 9 WebKit Commit Bot 2017-06-24 08:14:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Matijs van Zuijlen 2017-06-24 08:24:33 PDT
Awesome. Thanks for the quick response!