Bug 175937

Summary: [GTK] gtk_builder_get_type_from_name doesn't work with WebKitWebView
Product: WebKit Reporter: Debarshi Ray <rishi.is>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: REOPENED ---    
Severity: Normal CC: bugs-noreply, cgarcia, christian, mcatanzaro
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Test case
none
Test case none

Description Debarshi Ray 2017-08-24 08:52:34 PDT
Created attachment 318990 [details]
Test case

For any child widget inside a GTK+ widget template, GtkBuilder uses gtk_builder_get_type_from_name to get a GType corresponding to the class name. If a WebKitWebView is used it will use:
  type = gtk_builder_get_type_from_name (builder, "WebKitWebView");

GtkBuilder has some heuristics to get the *_get_type method corresponding to the class name so that it can register the type, if needed. See _gtk_builder_resolve_type_lazily in gtk/gtkbuilder.c.

This isn't working with WebKitWebView. However, it works with VteTerminal, which also offers a GtkWidget with a public C API that's internally implemented using C++.
Comment 1 Debarshi Ray 2017-08-24 08:55:06 PDT
Created attachment 318991 [details]
Test case

Use %u, not %d, with guint.
Comment 2 Michael Catanzaro 2017-08-24 16:09:25 PDT
(In reply to Debarshi Ray from comment #0)
> GtkBuilder has some heuristics to get the *_get_type method corresponding to
> the class name so that it can register the type, if needed. See
> _gtk_builder_resolve_type_lazily in gtk/gtkbuilder.c.

The problem is it is looking for the symbol web_kit_web_view_get_type, when it needs to look for webkit_web_view_get_type. This is unfortunate. I'm not sure what to do about this.

Workaround: manually register the type before creating the GtkBuilder. Something like g_object_unref (g_object_ref_sink (webkit_web_view_new ())) should work.
Comment 3 Michael Catanzaro 2017-08-24 16:10:47 PDT
(In reply to Michael Catanzaro from comment #2) 
> The problem is it is looking for the symbol web_kit_web_view_get_type, when
> it needs to look for webkit_web_view_get_type. This is unfortunate. I'm not
> sure what to do about this.

To be clear, it would surely work automatically if the widget was named WebkitWebView rather than WebKitWebView. I think this problem came up somewhere else recently, but I don't remember where.
Comment 4 Michael Catanzaro 2017-08-24 16:12:57 PDT
And we already use C linkage, so C++ is not the issue.

(In reply to Michael Catanzaro from comment #2)
> Workaround: manually register the type before creating the GtkBuilder.
> Something like g_object_unref (g_object_ref_sink (webkit_web_view_new ()))
> should work.

I guess that's overkill. Just calling webkit_web_view_get_type() would be the usual workaround. I'm sure similar workarounds exist elsewhere in GNOME.
Comment 5 Christian Hergert 2017-08-24 17:19:49 PDT
With "Kit" being more and more common, maybe it makes sense to teach GtkBuilder that words followed by "Kit" should be joined?
Comment 6 Debarshi Ray 2017-08-25 01:07:31 PDT
(In reply to Michael Catanzaro from comment #2)
> The problem is it is looking for the symbol web_kit_web_view_get_type, when
> it needs to look for webkit_web_view_get_type.

Oh, yes, of course! :)

(In reply to Michael Catanzaro from comment #4)
> (In reply to Michael Catanzaro from comment #2)
>> Workaround: manually register the type before creating the GtkBuilder.
>> Something like g_object_unref (g_object_ref_sink (webkit_web_view_new ()))
>> should work.
> 
> I guess that's overkill. Just calling webkit_web_view_get_type() would be
> the usual workaround. I'm sure similar workarounds exist elsewhere in GNOME.

The "recommended workaround" would be:
  g_type_ensure (WEBKIT_TYPE_WEB_VIEW)

(In reply to Christian Hergert from comment #5)
> With "Kit" being more and more common, maybe it makes sense to teach
> GtkBuilder that words followed by "Kit" should be joined?

Or, I wonder if webkitgtk should have a compatibility shim called web_kit_web_view_get_type that just calls webkit_web_view_get_type?
Comment 7 Michael Catanzaro 2017-08-25 07:51:27 PDT
(In reply to Debarshi Ray from comment #6)
> Or, I wonder if webkitgtk should have a compatibility shim called
> web_kit_web_view_get_type that just calls webkit_web_view_get_type?

Good idea. Let's do that.
Comment 8 Michael Catanzaro 2017-08-25 07:55:29 PDT
(In reply to Michael Catanzaro from comment #7)
> Good idea. Let's do that.

Hm, is it only needed for WebKitWebView, since that's the only GtkBuildable thing in the API? Maybe there are other things in glib that will look for web_kit_*_get_type()?
Comment 9 Christian Hergert 2017-08-25 14:07:02 PDT
(In reply to Michael Catanzaro from comment #8)
> (In reply to Michael Catanzaro from comment #7)
> > Good idea. Let's do that.
> 
> Hm, is it only needed for WebKitWebView, since that's the only GtkBuildable
> thing in the API? Maybe there are other things in glib that will look for
> web_kit_*_get_type()?

GtkBuilder files can be used for plenty of things not-widgetry. So starting down this path is likely going to just add more things later on.

Another option is to just use static constructors to register the types you care about. Since GObject should be initialized first (as a dependency of your library) you can probably just g_type_ensure() from a single static constructor.
Comment 10 Michael Catanzaro 2017-08-25 17:06:25 PDT
(In reply to Christian Hergert from comment #9) 
> Another option is to just use static constructors to register the types you
> care about. Since GObject should be initialized first (as a dependency of
> your library) you can probably just g_type_ensure() from a single static
> constructor.

This seems like the path of least-resistance.
Comment 11 Debarshi Ray 2017-08-26 05:45:04 PDT
(In reply to Michael Catanzaro from comment #10)
> (In reply to Christian Hergert from comment #9) 
> > Another option is to just use static constructors to register the types you
> > care about. Since GObject should be initialized first (as a dependency of
> > your library) you can probably just g_type_ensure() from a single static
> > constructor.
> 
> This seems like the path of least-resistance.

By constructor, I'd expect something like this:
https://git.gnome.org/browse/gnome-online-accounts/commit/src/goabackend?id=bbab1cebf4d976b51680a2ab6c4730b9580f5098

And in that case, you'd still be fixing this bug. :)

I don't think we should be placing the onus on the application to register the type. That reflects very badly on our platform since it will trip up anybody using GTK+ widget templates - something that we expect people to use to build UIs. If we are not going to make the effort to make these things just work, then it is dishonest for us to advertise them. Especially when there are multiple ways to fix the problem in the platform.

Minutes before I myself hit this problem, I was helping a newcomer on #webkitgtk+ who was puzzled why his GTK+ widget templates were not working in PyGObject. I myself hit this problem while programming in Vala. Having to figure out how g_type_ensure translates into a language binding is a pretty sucky experience, even for someone who has been programming in C/GObject for years.
Comment 12 Michael Catanzaro 2017-08-26 07:37:22 PDT
So what types do we need to register in our library constructor? Everything?
Comment 13 Michael Catanzaro 2017-08-26 19:21:31 PDT
On IRC we agreed the best way to solve this is to just add a web_kit_web_view_get_type() that calls webkit_web_view_get_type(), like Rishi suggested originally. So that's easy. Only hard part is figuring out how to export the symbol. I'm not sure where in the build system our linker configuration is hiding!
Comment 14 Carlos Garcia Campos 2017-08-28 02:46:05 PDT
(In reply to Christian Hergert from comment #5)
> With "Kit" being more and more common, maybe it makes sense to teach
> GtkBuilder that words followed by "Kit" should be joined?

I like this approach.
Comment 15 Debarshi Ray 2017-08-28 06:04:02 PDT
(In reply to Christian Hergert from comment #5)
> With "Kit" being more and more common, maybe it makes sense to teach
> GtkBuilder that words followed by "Kit" should be joined?

Do you know of any other *Kits where this would be a problem? I looked at PackageKit (Pk, pk_), PolKit (Polkit, polkit) and P11-Kit (P11Kit, p11_kit). The last doesn't seem to use gtk+, so probably not relevant.
Comment 16 Debarshi Ray 2017-08-28 09:32:11 PDT
By the way, Vala doesn't know about GtkBuilder's "type-func" attribute, and GtkBuilder ignores the "type-func" attribute if "class" is also present. This means when valac looks at the template class' [GtkChild] variable and tries to find its XML counterpart, it will fail, because it will ignore objects without the "class" attribute.

So, if you want to waste your time (please don't :) debating the merits of g_type_ensure and type-func as possible workarounds, then the former seems like the better option.
Comment 17 Debarshi Ray 2017-08-28 11:00:18 PDT
(In reply to Debarshi Ray from comment #16)
> By the way, Vala doesn't know about GtkBuilder's "type-func" attribute, and
> GtkBuilder ignores the "type-func" attribute if "class" is also present.
> This means when valac looks at the template class' [GtkChild] variable and
> tries to find its XML counterpart, it will fail, because it will ignore
> objects without the "class" attribute.

Filed these against gtk+:
https://bugzilla.gnome.org/show_bug.cgi?id=786931
https://bugzilla.gnome.org/show_bug.cgi?id=786932