Merge lp:~ted/indicator-appmenu/desc-reference into lp:indicator-appmenu/12.10

Proposed by Ted Gould on 2012-09-28
Status: Merged
Approved by: Charles Kerr on 2012-10-01
Approved revision: 211
Merge reported by: Lars Karlitski
Merged at revision: not available
Proposed branch: lp:~ted/indicator-appmenu/desc-reference
Merge into: lp:indicator-appmenu/12.10
Diff against target: 42 lines (+10/-1)
1 file modified
src/window-menu-dbusmenu.c (+10/-1)
To merge this branch: bzr merge lp:~ted/indicator-appmenu/desc-reference
Reviewer Review Type Date Requested Status
Charles Kerr (community) 2012-09-28 Approve on 2012-10-01
Review via email: mp+127056@code.launchpad.net

Commit message

Reference the variant used for the accessible description

Description of the change

There is a tricky race condition here where the label can be changed, but the signal to say so hasn't been notified. Basically this comes up when multiple properties are change, someone responds to them, and requires the accessible description before it knows that it's changed and the pointer has been updated. So, to ensure that the pointer remains valid we reference the variant that contians it, and then update as it changes.

To post a comment you must log in.
Charles Kerr (charlesk) :
review: Approve
Charles Kerr (charlesk) wrote :

Ted, how'd you find this one? :)

Ted Gould (ted) wrote :

On Mon, 2012-10-01 at 17:08 +0000, Charles Kerr wrote:
> Ted, how'd you find this one? :)

No comment. But let me say it's a bitch to recreate :-) But apparently
after a bit of usage on a slower computer and using a11y tech (which
seem to add some additional delay) it happens fairly often for folks.
There are a bunch of duplicates.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/window-menu-dbusmenu.c'
--- src/window-menu-dbusmenu.c 2012-03-18 18:13:55 +0000
+++ src/window-menu-dbusmenu.c 2012-09-28 19:50:27 +0000
@@ -53,6 +53,7 @@
53 gboolean hidden;53 gboolean hidden;
54 DbusmenuMenuitem * mi;54 DbusmenuMenuitem * mi;
55 WindowMenuDbusmenu * wm;55 WindowMenuDbusmenu * wm;
56 GVariant * vaccessible_desc;
56};57};
5758
58#define WINDOW_MENU_DBUSMENU_GET_PRIVATE(o) \59#define WINDOW_MENU_DBUSMENU_GET_PRIVATE(o) \
@@ -140,6 +141,9 @@
140 if (entry->accessible_desc != NULL) {141 if (entry->accessible_desc != NULL) {
141 entry->accessible_desc = NULL;142 entry->accessible_desc = NULL;
142 }143 }
144
145 g_clear_pointer(&wmentry->vaccessible_desc, g_variant_unref);
146
143 if (entry->image != NULL) {147 if (entry->image != NULL) {
144 g_object_unref(entry->image);148 g_object_unref(entry->image);
145 entry->image = NULL;149 entry->image = NULL;
@@ -658,7 +662,10 @@
658 wmentry->disabled = !g_variant_get_boolean(value);662 wmentry->disabled = !g_variant_get_boolean(value);
659 } else if (!g_strcmp0(property, DBUSMENU_MENUITEM_PROP_LABEL)) {663 } else if (!g_strcmp0(property, DBUSMENU_MENUITEM_PROP_LABEL)) {
660 gtk_label_set_text_with_mnemonic(entry->label, g_variant_get_string(value, NULL));664 gtk_label_set_text_with_mnemonic(entry->label, g_variant_get_string(value, NULL));
665
666 g_clear_pointer(&wmentry->vaccessible_desc, g_variant_unref);
661 entry->accessible_desc = g_variant_get_string(value, NULL);667 entry->accessible_desc = g_variant_get_string(value, NULL);
668 wmentry->vaccessible_desc = g_variant_ref(value);
662669
663 if (wmentry->wm != NULL) {670 if (wmentry->wm != NULL) {
664 g_signal_emit_by_name(G_OBJECT(wmentry->wm), WINDOW_MENU_SIGNAL_A11Y_UPDATE, entry, TRUE);671 g_signal_emit_by_name(G_OBJECT(wmentry->wm), WINDOW_MENU_SIGNAL_A11Y_UPDATE, entry, TRUE);
@@ -704,7 +711,9 @@
704 g_object_ref_sink(entry->label);711 g_object_ref_sink(entry->label);
705 }712 }
706713
707 entry->accessible_desc = dbusmenu_menuitem_property_get(newentry, DBUSMENU_MENUITEM_PROP_LABEL);714 g_clear_pointer(&wmentry->vaccessible_desc, g_variant_unref);
715 wmentry->vaccessible_desc = g_variant_ref(dbusmenu_menuitem_property_get_variant(newentry, DBUSMENU_MENUITEM_PROP_LABEL));
716 entry->accessible_desc = g_variant_get_string(wmentry->vaccessible_desc, NULL);
708717
709 entry->menu = dbusmenu_gtkclient_menuitem_get_submenu(priv->client, newentry);718 entry->menu = dbusmenu_gtkclient_menuitem_get_submenu(priv->client, newentry);
710719

Subscribers

People subscribed via source and target branches