GNOME Bugzilla – Bug 739739
Unable to use template from resource
Last modified: 2014-11-17 11:53:42 UTC
Hi, So I wanted to try the new template bindings in Gjs. So I tried it out in Polari. I kept getting messages like this: "Unable to load resource for composite template for type 'Gjs_UserListRow': The resource at '/org/gnome/Polari/resources/user-list-row.ui' does not exist" And the trouble seems to be two-fold. One is that Gjs sends in the complete string e.g: "resource:///org/gnome/Polari/user-list-row.ui" when Gtk.Widget.bind_template_child_full expects an absolute path like: "/org/gnome/Polari/user-list-row.ui". That did not help in the Polari case tho. The second issue is that the resource needs to be registered before any import that defines the a template class is used. So in Polari that meant moving the register call like so: diff --git a/src/application.js b/src/application.js index 06528c4..23e0aa7 100644 --- a/src/application.js +++ b/src/application.js @@ -9,10 +9,16 @@ const Gtk = imports.gi.Gtk; const Polari = imports.gi.Polari; const Tp = imports.gi.TelepathyGLib; +// We need to register our resources before we import anything that uses +// templates. Otherwise the resources will not be found in GtkWidgetClass init. +const Config = imports.config; +let resource = Gio.Resource.load(Config.RESOURCE_DIR + '/polari.gresource'); +resource._register(); + const AccountsMonitor = imports.accountsMonitor; const AppNotifications = imports.appNotifications; const ChatroomManager = imports.chatroomManager; -const Config = imports.config; + const Connections = imports.connections; const Format = imports.format; const Gettext = imports.gettext; @@ -45,9 +51,6 @@ const Application = new Lang.Class({ }, vfunc_startup: function() { - let resource = Gio.Resource.load(Config.RESOURCE_DIR + '/polari.gresource'); - resource._register(); - this.parent(); String.prototype.format = Format.format; This might be hard to realize for application developers? I will post a patch to fixup the resource:// path bug and to add a test. The test is pretty much guaranteed done wrong. I would like some assistance on how to add the resource properly to the test. Thanks!
Created attachment 290108 [details] [review] Gtk: Scrub resource:// prefix for template path Gtk.Widget.bind_template_child_full expects an absolute path to the resource. We need to scrub the resource:// part of the template string.
Created attachment 290109 [details] [review] Gtk: Add test case for template from resource
Created attachment 290110 [details] [review] Gtk: Add test case for template from resource
Review of attachment 290108 [details] [review]: ::: modules/overrides/Gtk.js @@ -52,3 @@ - if (typeof template == 'string' && - template.startsWith('resource:///')) - Gtk.Widget.set_template_from_resource.call(this, template); Or just use: Gtk.Widget.set_template_from_resource.call(this, template.slice(11));
Review of attachment 290110 [details] [review]: complex.ui is missing from the patch it seems.
Review of attachment 290110 [details] [review]: Oops, that's the correct status.
Created attachment 290113 [details] [review] Gtk: Add test case for template from resource
Created attachment 290114 [details] [review] Gtk: Scrub resource:// prefix for template path Gtk.Widget.bind_template_child_full expects an absolute path to the resource. We need to scrub the resource:// part of the template string.
Review of attachment 290113 [details] [review]: ::: installed-tests/js/testGtk.js @@ +10,3 @@ +try { + let resource = Gio.Resource.load('installed-tests/js/jsunit.gresource'); Anyone got an idea how to get this filename available for both local and installed tests? Generating a config.js? How to set the search_path correct in that case?
Review of attachment 290113 [details] [review]: ::: installed-tests/js/testGtk.js @@ +10,3 @@ +try { + let resource = Gio.Resource.load('installed-tests/js/jsunit.gresource'); Don't do this. Instead, generate C/H files and compile/link them togheter with gjs-unit.cpp in the jsunit runner. @@ +95,3 @@ + + assertEquals(new MyComplexGtkSubclass()); + assertEquals(new MyComplexGtkSubclassFromResource()); Please don't use the assertEquals() name, I thought this was a typo (missing JUnit.) before realizing you defined your own assertEquals()
*** Bug 739706 has been marked as a duplicate of this bug. ***
Btw, re the resource registration issue, it is true that resources need to be registered before the JS module is loaded, but it is also true that if one uses package.js or a C launcher binary this works fine already. Only apps using gnome-documents-style launching need special code - and those should be ported anyway because they don't pack the JS files into GResources.
Review of attachment 290113 [details] [review]: ::: installed-tests/js/testGtk.js @@ +10,3 @@ +try { + let resource = Gio.Resource.load('installed-tests/js/jsunit.gresource'); Well, I did this and could not get it to work. And it does not work in the gnome-maps case where bundle the resource like that. Did I mess up some where? Will upload how I did it. I also included a "standard" builder xml-file in the bundle. And loading that did work. But if I uncomment the template from resource code it says it cannot find the complex.ui resource.
Created attachment 290285 [details] [review] Gtk: Add test case for template from resource
(In reply to comment #13) > Review of attachment 290113 [details] [review]: > > ::: installed-tests/js/testGtk.js > @@ +10,3 @@ > > +try { > + let resource = Gio.Resource.load('installed-tests/js/jsunit.gresource'); > > Well, I did this and could not get it to work. And it does not work in the > gnome-maps case where bundle the resource like that. I tested it a day or two ago and it actually works without doing a manual register in Maps. The thing that made stuff fail for Maps was the "resource://"-prefix.
Created attachment 290306 [details] [review] Gtk: Add test case for template from resource
I ran this on a different system and it passed, so might be that I wasn't fully up-to-date, or that something wasn't clean in my build.
Created attachment 290322 [details] [review] Gtk: Add test case for template from resource
Review of attachment 290322 [details] [review]: Rest looks good. ::: Makefile-insttest.am @@ +31,3 @@ +jsunit_SOURCES = \ + installed-tests/gjs-unit.cpp \ + jsunit-resources.c js-unit-resources.h is missing from the sources list @@ +34,3 @@ + +jsunit_resources_files = $(shell glib-compile-resources --sourcedir=$(srcdir)/installed-tests/js --generate-dependencies $(srcdir)/test/jsunit.gresource.xml) +jsunit-resources.h: $(srcdir)/installed-tests/js/jsunit.gresources.xml $(jsunit_resource_files) You'll need to add the generated files to the CLEANFILES. ::: installed-tests/js/testGtk.js @@ +79,3 @@ win.add(content); + JSUnit.assertEquals("label is set to 'Complex!'", No need to change the indentation of those lines...
Review of attachment 290114 [details] [review]: ::: modules/overrides/Gtk.js @@ -51,3 @@ if (template) { - if (typeof template == 'string' && - template.startsWith('resource:///')) No need to change the indentation here either.
Created attachment 290622 [details] [review] Gtk: Scrub resource:// prefix for template path Gtk.Widget.bind_template_child_full expects an absolute path to the resource. We need to scrub the resource:// part of the template string.
Created attachment 290623 [details] [review] Gtk: Add test case for template from resource
Review of attachment 290622 [details] [review]: Looks good.
Review of attachment 290623 [details] [review]: Looks good as well.
Is anything blocking this or can we apply these patches. Giovanni what do you think?
I think that Bastien reviewed the patches already, so they're good to go. I don't know why he didn't mark them acn.
(In reply to comment #26) > I think that Bastien reviewed the patches already, so they're good to go. I > don't know why he didn't mark them acn. I didn't think I had the authority, and wanted to be sure somebody with more knowledge of the codebase looked at it. I guess that given that I had worked on shaping the patches for inclusion, I could have acn'ed them.