Application Menu Indicator

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

  1. desc-reference
  2. Merge into trunk.12.10
Proposed by Ted Gould
Status: Merged
Approved by: Charles Kerr
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) Approve
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.
Revision history for this message
Charles Kerr (charlesk) :
review: Approve
Revision history for this message
Charles Kerr (charlesk) wrote :

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

Revision history for this message
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