Created attachment 137994 [details] [review]
Remove libglade from media-keys plugin

Created attachment 137995 [details] [review]
Remove libglade from a11y-keyboard plugin

Can't do the keyboard plugin right now due to xklavier-4.0 dependency

Created attachment 137996 [details] [review]
Remove libglade from a11y-keyboard plugin

Sorry, attached the same patch twice.

Created attachment 137998 [details] [review]
Remove libglade from a11y-keyboard plugin

*sigh* another try, this time the version after whitespace correction

Sorry, for the spam.

 gsd_a11y_preferences_dialog_init (GsdA11yPreferencesDialog *dialog)
 {
         GtkWidget *widget;
-        GladeXML  *xml;
+	static const gchar *ui_file_path = GTKBUILDERDIR "/" GTKBUILDER_UI_FILE;

Wrong indentation here.

+        g_assert (gtk_builder_add_objects_from_file (builder, ui_file_path,
+                                                     objects, NULL) != 0);

Please don't do that. The assert in the old code shouldn't have been there, either. Not finding the UI description is not a programmer error.

(In reply to comment #6)
>  gsd_a11y_preferences_dialog_init (GsdA11yPreferencesDialog *dialog)
>  {
>          GtkWidget *widget;
> -        GladeXML  *xml;
> +       static const gchar *ui_file_path = GTKBUILDERDIR "/"
> GTKBUILDER_UI_FILE;
> 
> Wrong indentation here.

Slipped through again. :-)

> 
> +        g_assert (gtk_builder_add_objects_from_file (builder, ui_file_path,
> +                                                     objects, NULL) != 0);
> 
> Please don't do that. The assert in the old code shouldn't have been there,
> either. Not finding the UI description is not a programmer error.
> 

Yes, makes sense in a plugin. What would you like instead? g_warning, g_critical (each with the error message) or nothing?

g_warning would be ok but you also need to make sure that the plugin gracefully handles the missing UI and doesn't just crash instead.

Created attachment 138209 [details] [review]
137998: Remove libglade from a11y-keyboard plugin

This lets the dialog show some error message instead, if the UI file couldn't be loaded.

I'd say that's a little overkill...

Created attachment 138221 [details] [review]
Remove libglade dependency from keyboard plugin

(In reply to comment #10)
> I'd say that's a little overkill...

Well, alternatively one could do pretty much nothing and have it just display the "Close" button. How about that?

Considering that this error should never occur when the package is properly installed, just printing the warning and getting out is sufficient IMO. I especially dislike adding translatable strings for errors like this. Ideally, we could also flag the a11y plugin as not started but that's a little difficult with the asynchronous setup we have here.

Hmm, not sure if that's entirely possible as it's happening in the dialog's _init function.

Just a note to say the appearance capplet has been converted to GtkBuilder now too. I also have the mouse capplet almost finished.

Created attachment 138665 [details] [review]
Remove libglade from a11y-keyboard plugin (more reduced error)

This one shows the dialog without the parts from the UI file if it can't be loaded (the Close button is all that's left). Completely skipping the dialog would require bigger changes, as you are not allowed to return NULL from g_object_new.

Ok, will do for now. Thanks.

(In reply to comment #16)
> Ok, will do for now. Thanks.
> 

Ok, paired it with the following commit:

commit d9b2dc9a8ec99fec4f95b4502f73ea58bc3b01d7
Author: Felix Riemann <>
Date:   Sat Jul 18 15:14:22 2009 +0200

    Remove libglade dependency from configure.ac
    
    This concludes bug #582502. G-S-D should be libglade-free now.

 configure.ac |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

gnome-settings-daemon should have no libglade dependency anymore now. :-)