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
1=== modified file 'src/window-menu-dbusmenu.c'
2--- src/window-menu-dbusmenu.c 2012-03-18 18:13:55 +0000
3+++ src/window-menu-dbusmenu.c 2012-09-28 19:50:27 +0000
4@@ -53,6 +53,7 @@
5 gboolean hidden;
6 DbusmenuMenuitem * mi;
7 WindowMenuDbusmenu * wm;
8+ GVariant * vaccessible_desc;
9 };
10
11 #define WINDOW_MENU_DBUSMENU_GET_PRIVATE(o) \
12@@ -140,6 +141,9 @@
13 if (entry->accessible_desc != NULL) {
14 entry->accessible_desc = NULL;
15 }
16+
17+ g_clear_pointer(&wmentry->vaccessible_desc, g_variant_unref);
18+
19 if (entry->image != NULL) {
20 g_object_unref(entry->image);
21 entry->image = NULL;
22@@ -658,7 +662,10 @@
23 wmentry->disabled = !g_variant_get_boolean(value);
24 } else if (!g_strcmp0(property, DBUSMENU_MENUITEM_PROP_LABEL)) {
25 gtk_label_set_text_with_mnemonic(entry->label, g_variant_get_string(value, NULL));
26+
27+ g_clear_pointer(&wmentry->vaccessible_desc, g_variant_unref);
28 entry->accessible_desc = g_variant_get_string(value, NULL);
29+ wmentry->vaccessible_desc = g_variant_ref(value);
30
31 if (wmentry->wm != NULL) {
32 g_signal_emit_by_name(G_OBJECT(wmentry->wm), WINDOW_MENU_SIGNAL_A11Y_UPDATE, entry, TRUE);
33@@ -704,7 +711,9 @@
34 g_object_ref_sink(entry->label);
35 }
36
37- entry->accessible_desc = dbusmenu_menuitem_property_get(newentry, DBUSMENU_MENUITEM_PROP_LABEL);
38+ g_clear_pointer(&wmentry->vaccessible_desc, g_variant_unref);
39+ wmentry->vaccessible_desc = g_variant_ref(dbusmenu_menuitem_property_get_variant(newentry, DBUSMENU_MENUITEM_PROP_LABEL));
40+ entry->accessible_desc = g_variant_get_string(wmentry->vaccessible_desc, NULL);
41
42 entry->menu = dbusmenu_gtkclient_menuitem_get_submenu(priv->client, newentry);
43

Subscribers

People subscribed via source and target branches