Merge lp:~smspillaz/compiz-core/compiz-core.fix_936781 into lp:compiz-core

[Sam Spilsbury]

Check if the mutter key exists before using it

[Daniel van Vugt (vanvugt)]

I thought the fallback draggable border width should come from the theme definition. Or is it not in the theme?

review: Needs Information

[Sam Spilsbury (smspillaz)]

On Mon, 20 Feb 2012, Daniel van Vugt wrote:

> Review: Needs Information
>
> I thought the fallback draggable border width should come from the theme definition. Or is it not in the theme?

Not anymore, I dropped that patch

> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_936781/+merge/93776
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.fix_936781.
>

[Daniel van Vugt (vanvugt)]

I think it would be cleaner to use the built-in error detection (last parameter)...
    new_width = gconf_client_get_int (client, MUTTER_DRAGGABLE_BORDER_WIDTH_KEY, &err);
instead of ignoring all the error codes ("NULL").

That would allow you to remove:
    if (gconf_client_dir_exists (client, "/apps/mutter/general", NULL))

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

I had a look a the gconf documentation while I was writing this and it wasn't exactly all that clear as to what conditions the error bit would be set. Gconf has some weird semantics with schemaless keys (which is why gsettings exists), and really what I care about is whether or not that option actually exists in the directory, or if gconf is going to give me some default value. As far as I could tell, all it does is give you a default value of zero, which isn't particularly useful.

The reason why I chose to use that API is because 1) that API is there and seems to serve the purpose of checking whether or not a particular key exists in the database and 2) There is no documentation as to when Gconf will return an error condition for a missing key.

Of course, if you can point to something to the contrary I'd be glad to use that.

[Sam Spilsbury (smspillaz)]

Ooook, didrocks says the GError gets set in this case. Soooo lets change that :)

[Daniel van Vugt (vanvugt)]

Looks good and tests OK.

review: Approve

[Alan Griffiths (alan-griffiths)]

LGTM

review: Approve

=== modified file 'gtk/window-decorator/settings.c'
--- gtk/window-decorator/settings.c	2011-10-13 12:22:14 +0000
+++ gtk/window-decorator/settings.c	2012-02-21 10:23:49 +0000
@@ -21,10 +21,6 @@
 
 #include "gtk-window-decorator.h"
 
-/* TODO: Trash all of this and use a window property
- * instead - much much cleaner!
- */
-
 gboolean
 shadow_property_changed (WnckScreen *s)
 {
@@ -151,12 +147,19 @@
 static gboolean
 mutter_draggable_border_width_changed (GConfClient *client)
 {
+    static const unsigned int default_draggable_border_width = 7;
     int      new_width;
     int      width = settings->mutter_draggable_border_width;
+    GError   *error = NULL;
 
     new_width = gconf_client_get_int (client,
-				      MUTTER_DRAGGABLE_BORDER_WIDTH_KEY,
-				      NULL);
+			              MUTTER_DRAGGABLE_BORDER_WIDTH_KEY,
+			              &error);
+    if (error)
+    {
+	new_width = default_draggable_border_width;
+	g_error_free (error);
+    }
 
     if (new_width != width)
     {