Merge lp:~chrisccoulson/libdbusmenu/lp723873 into lp:libdbusmenu/0.5

[Chris Coulson]

Fix LP: #723873 - when a menuitems property is restored to a default value, the new state is not updated correctly on the listening client.

Make dbusmenu_menuitem_property_remove call dbusmenu_menuitem_property_set_variant with a NULL value rather than manipulating the properties directly. When removing a property that has a default value now, it will signal PROPERTY_CHANGED with the default value, which means that changing a property from non-default to default over the wire (which really just deletes the property) now works correctly. This is also now more aligned with how dbusmenu_menuitem_property_get* works, which will return the default value for a property after removing the property from the menuitem

[lp:~chrisccoulson/libdbusmenu/lp723873]

242. By Chris Coulson on 2011-03-01

The last commit causes the warning in dbusmenu_menuitem_property_is_default to be thrown
when removing a property that has no default value. This warning seems bogus though, as any property that is not
in the menuitems local property list is a default value (as that is what dbusmenu_menuitem_property_get* will return).
Simplify this function to work like this and drop the warning

[Ted Gould (ted)]

Okay, makes sense. I really think someone shouldn't be asking if it's a default property on something that isn't local and in the defaults database, but they could get out of sync (idle collection) and default is safer.

review: Approve

=== modified file 'libdbusmenu-glib/menuitem.c'
--- libdbusmenu-glib/menuitem.c	2011-02-25 03:30:16 +0000
+++ libdbusmenu-glib/menuitem.c	2011-03-01 12:37:32 +0000
@@ -1150,9 +1150,9 @@
 	DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
 	GVariant * default_value = NULL;
 
+	const gchar * type = menuitem_get_type(mi);
+
 	if (value != NULL) {
-		const gchar * type = menuitem_get_type(mi);
-
 		/* Check the expected type to see if we want to have a warning */
 		GVariantType * default_type = dbusmenu_defaults_default_get_type(priv->defaults, type, property);
 		if (default_type != NULL) {
@@ -1163,22 +1163,23 @@
 				g_warning("Setting menuitem property '%s' with value of type '%s' when expecting '%s'", property, g_variant_get_type_string(value), g_variant_type_peek_string(default_type));
 			}
 		}
+	}
 
-		/* Check the defaults database to see if we have a default
-		   for this property. */
-		default_value = dbusmenu_defaults_default_get(priv->defaults, type, property);
-		if (default_value != NULL) {
-			/* Now see if we're setting this to the same value as the
-			   default.  If we are then we just want to swallow this variant
-			   and make the function behave like we're clearing it. */
-			if (g_variant_equal(default_value, value)) {
-				g_variant_ref_sink(value);
-				g_variant_unref(value);
-				value = NULL;
-			}
+	/* Check the defaults database to see if we have a default
+	   for this property. */
+	default_value = dbusmenu_defaults_default_get(priv->defaults, type, property);
+	if (default_value != NULL && value != NULL) {
+		/* Now see if we're setting this to the same value as the
+		   default.  If we are then we just want to swallow this variant
+		   and make the function behave like we're clearing it. */
+		if (g_variant_equal(default_value, value)) {
+			g_variant_ref_sink(value);
+			g_variant_unref(value);
+			value = NULL;
 		}
 	}
 
+
 	gboolean replaced = FALSE;
 	gpointer currentval = g_hash_table_lookup(priv->properties, property);
 
@@ -1371,9 +1372,7 @@
 	g_return_if_fail(DBUSMENU_IS_MENUITEM(mi));
 	g_return_if_fail(property != NULL);
 
-	DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
-
-	g_hash_table_remove(priv->properties, property);
+	dbusmenu_menuitem_property_set_variant(mi, property, NULL);
 
 	return;
 }
@@ -1753,13 +1752,8 @@
 		return FALSE;
 	}
 
-	currentval = dbusmenu_defaults_default_get(priv->defaults, menuitem_get_type(mi), property);
-	if (currentval != NULL) {
-		return TRUE;
-	}
-
-	g_warn_if_reached();
-	return FALSE;
+	/* If we haven't stored it locally, then it's the default */
+	return TRUE;
 }
 
 /* Check to see if this menu item has been sent into the bus yet or