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

Proposed by Chris Coulson on 2011-02-28
Status: Merged
Approved by: Ted Gould on 2011-03-01
Approved revision: 242
Merged at revision: 246
Proposed branch: lp:~chrisccoulson/libdbusmenu/lp723873
Merge into: lp:libdbusmenu/0.5
Diff against target: 78 lines (+18/-24)
1 file modified
libdbusmenu-glib/menuitem.c (+18/-24)
To merge this branch: bzr merge lp:~chrisccoulson/libdbusmenu/lp723873
Reviewer Review Type Date Requested Status
Ted Gould (community) 2011-02-28 Approve on 2011-03-01
Review via email: mp+51648@code.launchpad.net

Description of the change

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

To post a comment you must log in.
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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbusmenu-glib/menuitem.c'
2--- libdbusmenu-glib/menuitem.c 2011-02-25 03:30:16 +0000
3+++ libdbusmenu-glib/menuitem.c 2011-03-01 12:37:32 +0000
4@@ -1150,9 +1150,9 @@
5 DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
6 GVariant * default_value = NULL;
7
8+ const gchar * type = menuitem_get_type(mi);
9+
10 if (value != NULL) {
11- const gchar * type = menuitem_get_type(mi);
12-
13 /* Check the expected type to see if we want to have a warning */
14 GVariantType * default_type = dbusmenu_defaults_default_get_type(priv->defaults, type, property);
15 if (default_type != NULL) {
16@@ -1163,22 +1163,23 @@
17 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));
18 }
19 }
20+ }
21
22- /* Check the defaults database to see if we have a default
23- for this property. */
24- default_value = dbusmenu_defaults_default_get(priv->defaults, type, property);
25- if (default_value != NULL) {
26- /* Now see if we're setting this to the same value as the
27- default. If we are then we just want to swallow this variant
28- and make the function behave like we're clearing it. */
29- if (g_variant_equal(default_value, value)) {
30- g_variant_ref_sink(value);
31- g_variant_unref(value);
32- value = NULL;
33- }
34+ /* Check the defaults database to see if we have a default
35+ for this property. */
36+ default_value = dbusmenu_defaults_default_get(priv->defaults, type, property);
37+ if (default_value != NULL && value != NULL) {
38+ /* Now see if we're setting this to the same value as the
39+ default. If we are then we just want to swallow this variant
40+ and make the function behave like we're clearing it. */
41+ if (g_variant_equal(default_value, value)) {
42+ g_variant_ref_sink(value);
43+ g_variant_unref(value);
44+ value = NULL;
45 }
46 }
47
48+
49 gboolean replaced = FALSE;
50 gpointer currentval = g_hash_table_lookup(priv->properties, property);
51
52@@ -1371,9 +1372,7 @@
53 g_return_if_fail(DBUSMENU_IS_MENUITEM(mi));
54 g_return_if_fail(property != NULL);
55
56- DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
57-
58- g_hash_table_remove(priv->properties, property);
59+ dbusmenu_menuitem_property_set_variant(mi, property, NULL);
60
61 return;
62 }
63@@ -1753,13 +1752,8 @@
64 return FALSE;
65 }
66
67- currentval = dbusmenu_defaults_default_get(priv->defaults, menuitem_get_type(mi), property);
68- if (currentval != NULL) {
69- return TRUE;
70- }
71-
72- g_warn_if_reached();
73- return FALSE;
74+ /* If we haven't stored it locally, then it's the default */
75+ return TRUE;
76 }
77
78 /* Check to see if this menu item has been sent into the bus yet or

Subscribers

People subscribed via source and target branches