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
=== modified file 'libdbusmenu-gtk/parser.c'
--- libdbusmenu-gtk/parser.c 2012-03-09 15:10:02 +0000
+++ libdbusmenu-gtk/parser.c 2012-03-21 12:34:17 +0000
@@ -45,7 +45,6 @@
45 GtkWidget *image;45 GtkWidget *image;
46 AtkObject *accessible;46 AtkObject *accessible;
4747
48 guint theme_changed_sig;
49} ParserData;48} ParserData;
5049
51typedef struct _RecurseContext50typedef struct _RecurseContext
@@ -86,8 +85,6 @@
86static void item_removed_cb (GtkContainer * menu,85static void item_removed_cb (GtkContainer * menu,
87 GtkWidget * widget,86 GtkWidget * widget,
88 gpointer data);87 gpointer data);
89static void theme_changed_cb (GtkIconTheme * theme,
90 gpointer data);
91static void item_activated (DbusmenuMenuitem * item,88static void item_activated (DbusmenuMenuitem * item,
92 guint timestamp,89 guint timestamp,
93 gpointer user_data);90 gpointer user_data);
@@ -216,29 +213,11 @@
216 g_object_remove_weak_pointer(G_OBJECT(pdata->accessible), (gpointer*)&pdata->accessible);213 g_object_remove_weak_pointer(G_OBJECT(pdata->accessible), (gpointer*)&pdata->accessible);
217 }214 }
218215
219 if (pdata != NULL && pdata->theme_changed_sig != 0) {
220 g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
221 pdata->theme_changed_sig = 0;
222 }
223
224 g_free(pdata);216 g_free(pdata);
225217
226 return;218 return;
227}219}
228220
229static void
230widget_freed (gpointer data, GObject * obj)
231{
232 ParserData * pdata = (ParserData *)data;
233
234 if (pdata->theme_changed_sig != 0) {
235 g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
236 pdata->theme_changed_sig = 0;
237 }
238
239 return;
240}
241
242/* Called when the dbusmenu item that we're keeping around221/* Called when the dbusmenu item that we're keeping around
243 is finalized */222 is finalized */
244static void223static void
@@ -247,12 +226,7 @@
247 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(obj), PARSER_DATA);226 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(obj), PARSER_DATA);
248227
249 if (pdata != NULL && pdata->widget != NULL) {228 if (pdata != NULL && pdata->widget != NULL) {
250 if (pdata->theme_changed_sig != 0) {
251 g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
252 pdata->theme_changed_sig = 0;
253 }
254 g_object_steal_data(G_OBJECT(pdata->widget), CACHED_MENUITEM);229 g_object_steal_data(G_OBJECT(pdata->widget), CACHED_MENUITEM);
255 g_object_weak_unref(G_OBJECT(pdata->widget), widget_freed, pdata);
256 }230 }
257}231}
258232
@@ -294,7 +268,6 @@
294 g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free);268 g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free);
295269
296 g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL);270 g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL);
297 g_object_weak_ref(G_OBJECT(widget), widget_freed, pdata);
298271
299 pdata->widget = widget;272 pdata->widget = widget;
300 g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget);273 g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget);
@@ -757,10 +730,6 @@
757 we can't get it easily, and those mean it's not changed just the icon730 we can't get it easily, and those mean it's not changed just the icon
758 underneith, so we can ignore these larger impacts */731 underneith, so we can ignore these larger impacts */
759 if (image != GTK_IMAGE(pdata->image) && gmenuitem != NULL) {732 if (image != GTK_IMAGE(pdata->image) && gmenuitem != NULL) {
760 if (pdata->theme_changed_sig != 0) {
761 g_signal_handler_disconnect(gtk_icon_theme_get_default(), pdata->theme_changed_sig);
762 pdata->theme_changed_sig = 0;
763 }
764733
765 if (pdata->image != NULL) {734 if (pdata->image != NULL) {
766 g_signal_handlers_disconnect_by_func(pdata->image, G_CALLBACK(image_notify_cb), menuitem);735 g_signal_handlers_disconnect_by_func(pdata->image, G_CALLBACK(image_notify_cb), menuitem);
@@ -770,8 +739,6 @@
770 pdata->image = GTK_WIDGET(image);739 pdata->image = GTK_WIDGET(image);
771740
772 if (pdata->image != NULL) {741 if (pdata->image != NULL) {
773 pdata->theme_changed_sig = g_signal_connect(G_OBJECT(gtk_icon_theme_get_default()),
774 "changed", G_CALLBACK(theme_changed_cb), gmenuitem);
775 g_signal_connect (G_OBJECT (pdata->image),742 g_signal_connect (G_OBJECT (pdata->image),
776 "notify",743 "notify",
777 G_CALLBACK (image_notify_cb),744 G_CALLBACK (image_notify_cb),
@@ -1287,24 +1254,6 @@
1287 return;1254 return;
1288}1255}
12891256
1290static void
1291theme_changed_cb (GtkIconTheme *theme, gpointer data)
1292{
1293 GtkWidget *image;
1294
1295 image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM (data));
1296
1297 gpointer pmi = g_object_get_data(G_OBJECT(data), CACHED_MENUITEM);
1298 if (pmi != NULL) {
1299 ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(pmi), PARSER_DATA);
1300 update_icon(DBUSMENU_MENUITEM(pmi), pdata, NULL, GTK_IMAGE(image));
1301 }
1302
1303 /* Switch signal to new theme */
1304 g_signal_handlers_disconnect_by_func(theme, G_CALLBACK(theme_changed_cb), data);
1305 g_signal_connect(gtk_icon_theme_get_default(), "changed", G_CALLBACK(theme_changed_cb), data);
1306}
1307
1308static gboolean1257static gboolean
1309should_show_image (GtkImage *image)1258should_show_image (GtkImage *image)
1310{1259{

Subscribers

People subscribed via source and target branches

to all changes: