DBus Menu

Merge lp:~charlesk/libdbusmenu/lp-953509 into lp:libdbusmenu/0.6

  1. lp-953509
  2. Merge into trunk.0.6
Proposed by Charles Kerr
Status: Merged
Approved by: Charles Kerr
Approved revision: 389
Merged at revision: 388
Proposed branch: lp:~charlesk/libdbusmenu/lp-953509
Merge into: lp:libdbusmenu/0.6
Diff against target: 116 lines (+0/-51)
1 file modified
libdbusmenu-gtk/parser.c (+0/-51)
To merge this branch: bzr merge lp:~charlesk/libdbusmenu/lp-953509
Reviewer Review Type Date Requested Status
Michael Terry Approve
Lars Karlitski (community) Approve
Review via email: mp+98621@code.launchpad.net

Description of the change

Don't listen for "changed" events from the screen's default GtkIconTheme. Fixes lp bug #953509

The crash in bug #953509 was being caused by the last two lines of theme_changed_cb() (a) leaving a dangling handler id in priv.theme_changed_sig, and (b) not remembering the handler id of its own signal connection. However after testing I don't see any reason to keep any of the theme handling code at all.

"But wait," you say. "How will our menu icons follow the theme changes?" It works anyway because we always listen for property changes to our reference GtkImage, **and** we listen for property changes to its GtkImageMenuItem parent so that if the GtkImageMenuItem changes GtkImages we can stop listening to the old one and start listening to the new one.

To post a comment you must log in.
Revision history for this message
Lars Karlitski (larsu) wrote :

Yes!

review: Approve
Revision history for this message
Michael Terry (mterry) wrote :

Seems fine. Thanks for cleaning up a mess I believe I introduced! :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbusmenu-gtk/parser.c'
2--- libdbusmenu-gtk/parser.c 2012-03-09 15:10:02 +0000
3+++ libdbusmenu-gtk/parser.c 2012-03-21 12:34:17 +0000
4@@ -45,7 +45,6 @@
5 GtkWidget *image;
6 AtkObject *accessible;
7
8- guint theme_changed_sig;
9 } ParserData;
10
11 typedef struct _RecurseContext
12@@ -86,8 +85,6 @@
13 static void item_removed_cb (GtkContainer * menu,
14 GtkWidget * widget,
15 gpointer data);
16-static void theme_changed_cb (GtkIconTheme * theme,
17- gpointer data);
18 static void item_activated (DbusmenuMenuitem * item,
19 guint timestamp,
20 gpointer user_data);
21@@ -216,29 +213,11 @@
22 g_object_remove_weak_pointer(G_OBJECT(pdata->accessible), (gpointer*)&pdata->accessible);
23 }
24
25- if (pdata != NULL && pdata->theme_changed_sig != 0) {
26- g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
27- pdata->theme_changed_sig = 0;
28- }
29-
30 g_free(pdata);
31
32 return;
33 }
34
35-static void
36-widget_freed (gpointer data, GObject * obj)
37-{
38- ParserData * pdata = (ParserData *)data;
39-
40- if (pdata->theme_changed_sig != 0) {
41- g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
42- pdata->theme_changed_sig = 0;
43- }
44-
45- return;
46-}
47-
48 /* Called when the dbusmenu item that we're keeping around
49 is finalized */
50 static void
51@@ -247,12 +226,7 @@
52 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(obj), PARSER_DATA);
53
54 if (pdata != NULL && pdata->widget != NULL) {
55- if (pdata->theme_changed_sig != 0) {
56- g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
57- pdata->theme_changed_sig = 0;
58- }
59 g_object_steal_data(G_OBJECT(pdata->widget), CACHED_MENUITEM);
60- g_object_weak_unref(G_OBJECT(pdata->widget), widget_freed, pdata);
61 }
62 }
63
64@@ -294,7 +268,6 @@
65 g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free);
66
67 g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL);
68- g_object_weak_ref(G_OBJECT(widget), widget_freed, pdata);
69
70 pdata->widget = widget;
71 g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget);
72@@ -757,10 +730,6 @@
73 we can't get it easily, and those mean it's not changed just the icon
74 underneith, so we can ignore these larger impacts */
75 if (image != GTK_IMAGE(pdata->image) && gmenuitem != NULL) {
76- if (pdata->theme_changed_sig != 0) {
77- g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
78- pdata->theme_changed_sig = 0;
79- }
80
81 if (pdata->image != NULL) {
82 g_signal_handlers_disconnect_by_func(pdata->image, G_CALLBACK(image_notify_cb), menuitem);
83@@ -770,8 +739,6 @@
84 pdata->image = GTK_WIDGET(image);
85
86 if (pdata->image != NULL) {
87- pdata->theme_changed_sig = g_signal_connect(G_OBJECT(gtk_icon_theme_get_default()),
88- "changed", G_CALLBACK(theme_changed_cb), gmenuitem);
89 g_signal_connect (G_OBJECT (pdata->image),
90 "notify",
91 G_CALLBACK (image_notify_cb),
92@@ -1287,24 +1254,6 @@
93 return;
94 }
95
96-static void
97-theme_changed_cb (GtkIconTheme *theme, gpointer data)
98-{
99- GtkWidget *image;
100-
101- image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM (data));
102-
103- gpointer pmi = g_object_get_data(G_OBJECT(data), CACHED_MENUITEM);
104- if (pmi != NULL) {
105- ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(pmi), PARSER_DATA);
106- update_icon(DBUSMENU_MENUITEM(pmi), pdata, NULL, GTK_IMAGE(image));
107- }
108-
109- /* Switch signal to new theme */
110- g_signal_handlers_disconnect_by_func(theme, G_CALLBACK(theme_changed_cb), data);
111- g_signal_connect(gtk_icon_theme_get_default(), "changed", G_CALLBACK(theme_changed_cb), data);
112-}
113-
114 static gboolean
115 should_show_image (GtkImage *image)
116 {

Subscribers

People subscribed via source and target branches

to all changes: