After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 633191 - xsettings: Port GTK+ Modules loading to GSettings
xsettings: Port GTK+ Modules loading to GSettings
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: xsettings
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks: 633192 633194 633195
 
 
Reported: 2010-10-26 14:15 UTC by Bastien Nocera
Modified: 2010-10-26 14:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
xsettings: Port GTK+ Modules loading to GSettings (14.48 KB, patch)
2010-10-26 14:15 UTC, Bastien Nocera
none Details | Review
xsettings: Port GTK+ Modules loading to GSettings (30.85 KB, patch)
2010-10-26 14:16 UTC, Bastien Nocera
none Details | Review
xsettings: Port GTK+ Modules loading to GSettings (30.92 KB, patch)
2010-10-26 14:18 UTC, Bastien Nocera
reviewed Details | Review
xsettings: Port GTK+ Modules loading to GSettings (30.42 KB, patch)
2010-10-26 14:42 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2010-10-26 14:15:42 UTC
As discussed
Comment 1 Bastien Nocera 2010-10-26 14:15:44 UTC
Created attachment 173256 [details] [review]
xsettings: Port GTK+ Modules loading to GSettings

Adds the "disabled-gtk-modules" and "enabled-gtk-modules" GSettings
keys, for admins to lockdown loading (or not) particular GTK+ modules.

The modules that need it will install a .desktop file in
$(libdir)/gnome-settings-daemon-3.0/gtk-modules/
Comment 2 Bastien Nocera 2010-10-26 14:16:20 UTC
Created attachment 173257 [details] [review]
xsettings: Port GTK+ Modules loading to GSettings

Adds the "disabled-gtk-modules" and "enabled-gtk-modules" GSettings
keys, for admins to lockdown loading (or not) particular GTK+ modules.

The modules that need it will install a .desktop file in
$(libdir)/gnome-settings-daemon-3.0/gtk-modules/
Comment 3 Bastien Nocera 2010-10-26 14:18:00 UTC
Created attachment 173258 [details] [review]
xsettings: Port GTK+ Modules loading to GSettings

Adds the "disabled-gtk-modules" and "enabled-gtk-modules" GSettings
keys, for admins to lockdown loading (or not) particular GTK+ modules.

The modules that need it will install a .desktop file in
$(libdir)/gnome-settings-daemon-3.0/gtk-modules/
Comment 4 Bastien Nocera 2010-10-26 14:18:48 UTC
First patch was missing new files, and the second one was missing handling for non-conditional module loading.
Comment 5 Matthias Clasen 2010-10-26 14:35:41 UTC
Review of attachment 173258 [details] [review]:

::: plugins/xsettings/gsd-xsettings-gtk.c
@@ +211,3 @@
+                g_string_append (str, key);
+        else
+                g_string_append_printf (str, ":%s", (const char *) key);

Could just do 

if (str->len > 0) g_string_append_c (str, ':');

and avoid the printf here, but no big deal.

@@ +246,3 @@
+        str = g_string_new (NULL);
+        g_hash_table_foreach (ht, (GHFunc) stringify_gtk_modules, str);
+        g_hash_table_destroy (ht);

I'm concerned about ordering here. At least the a11y modules have some constraints about the order in which they are loaded.

@@ +282,3 @@
+                g_source_remove (gtk->priv->timeout);
+
+        gtk->priv->timeout = g_timeout_add (1000, (GSourceFunc) gtk_modules_dir_timeout, gtk);

GFileMonitor already has a timeout. You can just set that, instead of doing it manually.
Comment 6 Bastien Nocera 2010-10-26 14:42:32 UTC
Created attachment 173261 [details] [review]
xsettings: Port GTK+ Modules loading to GSettings

Adds the "disabled-gtk-modules" and "enabled-gtk-modules" GSettings
keys, for admins to lockdown loading (or not) particular GTK+ modules.

The modules that need it will install a .desktop file in
$(libdir)/gnome-settings-daemon-3.0/gtk-modules/
Comment 7 Bastien Nocera 2010-10-26 14:44:51 UTC
Comment on attachment 173258 [details] [review]
xsettings: Port GTK+ Modules loading to GSettings

Fixed the mispelling of "explicitly", removed the use of a timeout as GFileMonitor already does rate limiting.

The order problem is solved by loading both modules in the same .desktop file.
Comment 8 Bastien Nocera 2010-10-26 14:55:40 UTC
Attachment 173261 [details] pushed as 1f51464 - xsettings: Port GTK+ Modules loading to GSettings