DBus Menu

Merge lp:~dcbw/libdbusmenu/libdbusmenu into lp:libdbusmenu/15.10

  1. libdbusmenu
  2. Merge into trunk.15.10
Proposed by Dan Williams
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: 479
Merged at revision: 479
Proposed branch: lp:~dcbw/libdbusmenu/libdbusmenu
Merge into: lp:libdbusmenu/15.10
Diff against target: 276 lines (+59/-33)
4 files modified
configure.ac (+0/-1)
libdbusmenu-gtk/client.c (+6/-1)
libdbusmenu-gtk/genericmenuitem.c (+15/-10)
libdbusmenu-gtk/parser.c (+38/-21)
To merge this branch: bzr merge lp:~dcbw/libdbusmenu/libdbusmenu
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Dan Williams (community) Needs Resubmitting
Review via email: mp+286843@code.launchpad.net

Commit message

gtk: look for GtkImages on regular GtkMenuItems too

GtkImageMenuItem is deprecated, and the recommended replacement
is a normal GtkMenuItem packed manually with a label and an image.
To ensure applications that use recommended GTK practices can still
show menu item images, check the children of a normal GtkMenuItem
for a GtkImage too, just like is done for the label child.

Description of the change

GtkImageMenuItem has been deprecated for a long time in upstream GTK; projects that use dbusmenu but do not want to depend on deprecated GTK functionality cannot use images in menu items because dbusmenu only looks for images on GtkImageMenuItems.

Upstream recommended replacement for GtkImageMenuItem is to simply pack your own label + image into a normal GtkMenuItem. Thus, make dbusmenu look for images in normal GtkMenuItems to.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Looks good, thanks for your contribution and the nice cleanup.

There's just one small fix to do: http://paste.ubuntu.com/15180692/ otherwise libappindicator based menus will be misaligned (http://i.imgur.com/DXqeWwy.png)

review: Needs Fixing
lp:~dcbw/libdbusmenu/libdbusmenu updated
478. By Dan Williams <email address hidden>

gtk: fix some GTKv3 deprecations and get rid of HAVE_GTK3

479. By Dan Williams <email address hidden>

gtk: look for GtkImages on regular GtkMenuItems too

GtkImageMenuItem is deprecated, and the recommended replacement
is a normal GtkMenuItem packed manually with a label and an image.
To ensure applications that use recommended GTK practices can still
show menu item images, check the children of a normal GtkMenuItem
for a GtkImage too, just like is done for the label child.

Revision history for this message
Dan Williams (dcbw) wrote :

> Looks good, thanks for your contribution and the nice cleanup.
>
> There's just one small fix to do: http://paste.ubuntu.com/15180692/ otherwise
> libappindicator based menus will be misaligned
> (http://i.imgur.com/DXqeWwy.png)

I was testing nm-applet (which does use libappindicator) using https://extensions.gnome.org/extension/615/appindicator-support/, obligatory screenshot without the halign here:

http://people.redhat.com/dcbw/nm-applet-indicator.png

but the changes don't seem to hurt anything and are likely correct. Repushed.

Revision history for this message
Dan Williams (dcbw) :
review: Needs Resubmitting
Revision history for this message
Dan Williams (dcbw) wrote :

No idea how to move the review back to 'pending' or whatever it is, so I tried 'resubmit'. If that's not correct, sorry!

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> No idea how to move the review back to 'pending' or whatever it is, so I tried
> 'resubmit'. If that's not correct, sorry!

It was already in "pending mode" (until the top value is under "Needs review"), no worries.

Anyway, thanks for the update!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'configure.ac'
--- configure.ac 2013-07-22 05:15:14 +0000
+++ configure.ac 2016-02-23 20:10:45 +0000
@@ -67,7 +67,6 @@
67 glib-2.0 >= $GLIB_REQUIRED_VERSION,67 glib-2.0 >= $GLIB_REQUIRED_VERSION,
68 [have_gtk=yes]68 [have_gtk=yes]
69)69)
70 AC_DEFINE(HAVE_GTK3, 1, [whether gtk3 is available])
71 ],70 ],
72 [test "x$with_gtk" = x2],71 [test "x$with_gtk" = x2],
73 [PKG_CHECK_MODULES(DBUSMENUGTK, gtk+-2.0 >= $GTK_REQUIRED_VERSION72 [PKG_CHECK_MODULES(DBUSMENUGTK, gtk+-2.0 >= $GTK_REQUIRED_VERSION
7473
=== modified file 'libdbusmenu-gtk/client.c'
--- libdbusmenu-gtk/client.c 2013-05-15 11:44:38 +0000
+++ libdbusmenu-gtk/client.c 2016-02-23 20:10:45 +0000
@@ -842,7 +842,7 @@
842 doesn't expose the right variables. We need to figure842 doesn't expose the right variables. We need to figure
843 this out as menus won't get grabs properly.843 this out as menus won't get grabs properly.
844 TODO FIXME HELP ARGHHHHHHHH */844 TODO FIXME HELP ARGHHHHHHHH */
845#if (HAVE_GTK3 == 0)845#if !GTK_CHECK_VERSION(3,0,0)
846 if (!GTK_MENU_SHELL (parent)->active) {846 if (!GTK_MENU_SHELL (parent)->active) {
847 gtk_grab_add (parent);847 gtk_grab_add (parent);
848 GTK_MENU_SHELL (parent)->have_grab = TRUE;848 GTK_MENU_SHELL (parent)->have_grab = TRUE;
@@ -1278,7 +1278,12 @@
1278 gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);1278 gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
12791279
1280 gtk_widget_set_size_request(GTK_WIDGET(gtkimage), width, height);1280 gtk_widget_set_size_request(GTK_WIDGET(gtkimage), width, height);
1281#if GTK_CHECK_VERSION(3,0,0)
1282 gtk_widget_set_halign(GTK_WIDGET(gtkimage), GTK_ALIGN_START);
1283 gtk_widget_set_valign(GTK_WIDGET(gtkimage), GTK_ALIGN_CENTER);
1284#else
1281 gtk_misc_set_alignment(GTK_MISC(gtkimage), 0.0, 0.5);1285 gtk_misc_set_alignment(GTK_MISC(gtkimage), 0.0, 0.5);
1286#endif
1282 }1287 }
12831288
1284 genericmenuitem_set_image(GENERICMENUITEM(gimi), gtkimage);1289 genericmenuitem_set_image(GENERICMENUITEM(gimi), gtkimage);
12851290
=== modified file 'libdbusmenu-gtk/genericmenuitem.c'
--- libdbusmenu-gtk/genericmenuitem.c 2012-11-21 18:13:53 +0000
+++ libdbusmenu-gtk/genericmenuitem.c 2016-02-23 20:10:45 +0000
@@ -62,7 +62,7 @@
62/* GObject stuff */62/* GObject stuff */
63G_DEFINE_TYPE (Genericmenuitem, genericmenuitem, GTK_TYPE_CHECK_MENU_ITEM);63G_DEFINE_TYPE (Genericmenuitem, genericmenuitem, GTK_TYPE_CHECK_MENU_ITEM);
6464
65#if HAVE_GTK365#if GTK_CHECK_VERSION(3,0,0)
66static void draw_indicator (GtkCheckMenuItem *check_menu_item, cairo_t *cr);66static void draw_indicator (GtkCheckMenuItem *check_menu_item, cairo_t *cr);
67static void (*parent_draw_indicator) (GtkCheckMenuItem *check_menu_item, cairo_t *cr) = NULL;67static void (*parent_draw_indicator) (GtkCheckMenuItem *check_menu_item, cairo_t *cr) = NULL;
68#else68#else
@@ -83,7 +83,7 @@
83 object_class->dispose = genericmenuitem_dispose;83 object_class->dispose = genericmenuitem_dispose;
84 object_class->finalize = genericmenuitem_finalize;84 object_class->finalize = genericmenuitem_finalize;
8585
86#ifdef HAVE_GTK386#if GTK_CHECK_VERSION(3,2,0)
87 GtkWidgetClass * widget_class = GTK_WIDGET_CLASS(klass);87 GtkWidgetClass * widget_class = GTK_WIDGET_CLASS(klass);
8888
89 gtk_widget_class_set_accessible_role(widget_class, ATK_ROLE_MENU_ITEM);89 gtk_widget_class_set_accessible_role(widget_class, ATK_ROLE_MENU_ITEM);
@@ -115,7 +115,7 @@
115 self->priv->disposition = GENERICMENUITEM_DISPOSITION_NORMAL;115 self->priv->disposition = GENERICMENUITEM_DISPOSITION_NORMAL;
116 self->priv->label_text = NULL;116 self->priv->label_text = NULL;
117117
118#ifndef HAVE_GTK3118#if !GTK_CHECK_VERSION(3,0,0)
119 AtkObject * aobj = gtk_widget_get_accessible(GTK_WIDGET(self));119 AtkObject * aobj = gtk_widget_get_accessible(GTK_WIDGET(self));
120 if (aobj != NULL) {120 if (aobj != NULL) {
121 atk_object_set_role(aobj, ATK_ROLE_MENU_ITEM);121 atk_object_set_role(aobj, ATK_ROLE_MENU_ITEM);
@@ -148,7 +148,7 @@
148/* Checks to see if we should be drawing a little box at148/* Checks to see if we should be drawing a little box at
149 all. If we should be, let's do that, otherwise we're149 all. If we should be, let's do that, otherwise we're
150 going suppress the box drawing. */150 going suppress the box drawing. */
151#if HAVE_GTK3151#if GTK_CHECK_VERSION(3,0,0)
152static void152static void
153draw_indicator (GtkCheckMenuItem *check_menu_item, cairo_t *cr)153draw_indicator (GtkCheckMenuItem *check_menu_item, cairo_t *cr)
154{154{
@@ -310,12 +310,12 @@
310 /* We need to put the child into a new box and310 /* We need to put the child into a new box and
311 make the box the child of the menu item. Basically311 make the box the child of the menu item. Basically
312 we're inserting a box in the middle. */312 we're inserting a box in the middle. */
313 #ifdef HAVE_GTK3313#if GTK_CHECK_VERSION(3,0,0)
314 GtkWidget * hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL,314 GtkWidget * hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL,
315 get_toggle_space(GTK_WIDGET(menu_item)));315 get_toggle_space(GTK_WIDGET(menu_item)));
316 #else316#else
317 GtkWidget * hbox = gtk_hbox_new(FALSE, get_toggle_space(GTK_WIDGET(menu_item)));317 GtkWidget * hbox = gtk_hbox_new(FALSE, get_toggle_space(GTK_WIDGET(menu_item)));
318 #endif318#endif
319 g_object_ref(child);319 g_object_ref(child);
320 gtk_container_remove(GTK_CONTAINER(menu_item), child);320 gtk_container_remove(GTK_CONTAINER(menu_item), child);
321 gtk_box_pack_start(GTK_BOX(hbox), child, FALSE, FALSE, 0);321 gtk_box_pack_start(GTK_BOX(hbox), child, FALSE, FALSE, 0);
@@ -334,7 +334,12 @@
334 /* Build it */334 /* Build it */
335 labelw = GTK_LABEL(gtk_accel_label_new(local_label));335 labelw = GTK_LABEL(gtk_accel_label_new(local_label));
336 gtk_label_set_use_markup(GTK_LABEL(labelw), TRUE);336 gtk_label_set_use_markup(GTK_LABEL(labelw), TRUE);
337#if GTK_CHECK_VERSION(3,0,0)
338 gtk_widget_set_halign(GTK_WIDGET(labelw), GTK_ALIGN_START);
339 gtk_widget_set_valign(GTK_WIDGET(labelw), GTK_ALIGN_CENTER);
340#else
337 gtk_misc_set_alignment(GTK_MISC(labelw), 0.0, 0.5);341 gtk_misc_set_alignment(GTK_MISC(labelw), 0.0, 0.5);
342#endif
338 gtk_accel_label_set_accel_widget(GTK_ACCEL_LABEL(labelw), GTK_WIDGET(menu_item));343 gtk_accel_label_set_accel_widget(GTK_ACCEL_LABEL(labelw), GTK_WIDGET(menu_item));
339344
340 if (has_mnemonic(in_label, FALSE)) {345 if (has_mnemonic(in_label, FALSE)) {
@@ -541,12 +546,12 @@
541 /* We need to put the child into a new box and546 /* We need to put the child into a new box and
542 make the box the child of the menu item. Basically547 make the box the child of the menu item. Basically
543 we're inserting a box in the middle. */548 we're inserting a box in the middle. */
544 #ifdef HAVE_GTK3549#if GTK_CHECK_VERSION(3,0,0)
545 GtkWidget * hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL,550 GtkWidget * hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL,
546 get_toggle_space(GTK_WIDGET(menu_item)));551 get_toggle_space(GTK_WIDGET(menu_item)));
547 #else552#else
548 GtkWidget * hbox = gtk_hbox_new(FALSE, get_toggle_space(GTK_WIDGET(menu_item)));553 GtkWidget * hbox = gtk_hbox_new(FALSE, get_toggle_space(GTK_WIDGET(menu_item)));
549 #endif554#endif
550 g_object_ref(child);555 g_object_ref(child);
551 gtk_container_remove(GTK_CONTAINER(menu_item), child);556 gtk_container_remove(GTK_CONTAINER(menu_item), child);
552 gtk_box_pack_end(GTK_BOX(hbox), child, TRUE, TRUE, 0);557 gtk_box_pack_end(GTK_BOX(hbox), child, TRUE, TRUE, 0);
553558
=== modified file 'libdbusmenu-gtk/parser.c'
--- libdbusmenu-gtk/parser.c 2015-03-04 22:18:54 +0000
+++ libdbusmenu-gtk/parser.c 2016-02-23 20:10:45 +0000
@@ -82,7 +82,8 @@
82static void update_icon (DbusmenuMenuitem * menuitem,82static void update_icon (DbusmenuMenuitem * menuitem,
83 ParserData * pdata,83 ParserData * pdata,
84 GtkImage * image);84 GtkImage * image);
85static GtkWidget * find_menu_label (GtkWidget * widget);85static GtkWidget * find_menu_child (GtkWidget * widget,
86 GType child_type);
86static void label_notify_cb (GtkWidget * widget,87static void label_notify_cb (GtkWidget * widget,
87 GParamSpec * pspec,88 GParamSpec * pspec,
88 gpointer data);89 gpointer data);
@@ -648,7 +649,7 @@
648649
649 gboolean visible = FALSE;650 gboolean visible = FALSE;
650 gboolean sensitive = FALSE;651 gboolean sensitive = FALSE;
651 if (GTK_IS_SEPARATOR_MENU_ITEM (widget) || !find_menu_label (widget))652 if (GTK_IS_SEPARATOR_MENU_ITEM (widget) || !find_menu_child (widget, GTK_TYPE_LABEL))
652 {653 {
653 dbusmenu_menuitem_property_set (mi,654 dbusmenu_menuitem_property_set (mi,
654 DBUSMENU_MENUITEM_PROP_TYPE,655 DBUSMENU_MENUITEM_PROP_TYPE,
@@ -659,6 +660,8 @@
659 }660 }
660 else661 else
661 {662 {
663 GtkWidget *image = NULL;
664
662 pdata->widget_accel_handler_id = g_signal_connect (widget, "accel-closures-changed",665 pdata->widget_accel_handler_id = g_signal_connect (widget, "accel-closures-changed",
663 G_CALLBACK (accel_changed), mi);666 G_CALLBACK (accel_changed), mi);
664667
@@ -674,20 +677,26 @@
674677
675 pdata->widget_toggle_handler_id = g_signal_connect (widget, "activate", G_CALLBACK (checkbox_toggled), mi);678 pdata->widget_toggle_handler_id = g_signal_connect (widget, "activate", G_CALLBACK (checkbox_toggled), mi);
676 }679 }
677680 else if (GTK_IS_IMAGE_MENU_ITEM (widget))
678 if (GTK_IS_IMAGE_MENU_ITEM (widget))
679 {681 {
680 GtkWidget *image;
681682
682 image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM (widget));683 image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM (widget));
683684
684 if (GTK_IS_IMAGE (image))685 }
685 {686 else
686 update_icon (mi, pdata, GTK_IMAGE (image));687 {
687 }688 // GtkImageMenuItem is deprecated, so check regular GtkMenuItems
688 }689 // for an image child too
689690 image = find_menu_child (widget, GTK_TYPE_IMAGE);
690 GtkWidget *label = find_menu_label (widget);691 }
692
693 if (GTK_IS_IMAGE (image))
694 {
695 update_icon (mi, pdata, GTK_IMAGE (image));
696 }
697
698
699 GtkWidget *label = find_menu_child (widget, GTK_TYPE_LABEL);
691700
692 // Sometimes, an app will directly find and modify the label701 // Sometimes, an app will directly find and modify the label
693 // (like empathy), so watch the label especially for that.702 // (like empathy), so watch the label especially for that.
@@ -907,7 +916,11 @@
907 GTK_ICON_LOOKUP_FORCE_SIZE);916 GTK_ICON_LOOKUP_FORCE_SIZE);
908 if (info != NULL) {917 if (info != NULL) {
909 pixbuf = gtk_icon_info_load_icon (info, NULL);918 pixbuf = gtk_icon_info_load_icon (info, NULL);
919#if GTK_CHECK_VERSION(3,8,0)
920 g_object_unref (info);
921#else
910 gtk_icon_info_free (info);922 gtk_icon_info_free (info);
923#endif
911 }924 }
912 break;925 break;
913926
@@ -944,11 +957,11 @@
944}957}
945958
946static GtkWidget *959static GtkWidget *
947find_menu_label (GtkWidget *widget)960find_menu_child (GtkWidget *widget, GType child_type)
948{961{
949 GtkWidget *label = NULL;962 GtkWidget *child = NULL;
950963
951 if (GTK_IS_LABEL (widget))964 if (G_TYPE_CHECK_INSTANCE_TYPE (widget, child_type))
952 return widget;965 return widget;
953966
954 if (GTK_IS_CONTAINER (widget))967 if (GTK_IS_CONTAINER (widget))
@@ -960,16 +973,16 @@
960973
961 for (l = children; l; l = l->next)974 for (l = children; l; l = l->next)
962 {975 {
963 label = find_menu_label (l->data);976 child = find_menu_child (l->data, child_type);
964977
965 if (label)978 if (child)
966 break;979 break;
967 }980 }
968981
969 g_list_free (children);982 g_list_free (children);
970 }983 }
971984
972 return label;985 return child;
973}986}
974987
975static void988static void
@@ -1121,7 +1134,7 @@
1121 {1134 {
1122 DbusmenuMenuitem * item = DBUSMENU_MENUITEM(data);1135 DbusmenuMenuitem * item = DBUSMENU_MENUITEM(data);
1123 GtkWidget *widget = gtk_accessible_get_widget (GTK_ACCESSIBLE (accessible));1136 GtkWidget *widget = gtk_accessible_get_widget (GTK_ACCESSIBLE (accessible));
1124 GtkWidget *label = find_menu_label (widget);1137 GtkWidget *label = find_menu_child (widget, GTK_TYPE_LABEL);
1125 const gchar *label_text = gtk_label_get_text (GTK_LABEL (label));1138 const gchar *label_text = gtk_label_get_text (GTK_LABEL (label));
1126 const gchar *name = atk_object_get_name (accessible);1139 const gchar *name = atk_object_get_name (accessible);
11271140
@@ -1335,7 +1348,7 @@
1335 GtkWidget *child,1348 GtkWidget *child,
1336 gpointer data)1349 gpointer data)
1337{1350{
1338 if (find_menu_label (child) != NULL)1351 if (find_menu_child (widget, GTK_TYPE_LABEL) != NULL)
1339 handle_first_label (data);1352 handle_first_label (data);
1340}1353}
13411354
@@ -1433,6 +1446,9 @@
14331446
1434 item = gtk_widget_get_ancestor (GTK_WIDGET (image),1447 item = gtk_widget_get_ancestor (GTK_WIDGET (image),
1435 GTK_TYPE_IMAGE_MENU_ITEM);1448 GTK_TYPE_IMAGE_MENU_ITEM);
1449 if (!item)
1450 item = gtk_widget_get_ancestor (GTK_WIDGET (image),
1451 GTK_TYPE_MENU_ITEM);
14361452
1437 if (item)1453 if (item)
1438 {1454 {
@@ -1446,7 +1462,8 @@
1446 if (gtk_menu_images)1462 if (gtk_menu_images)
1447 return TRUE;1463 return TRUE;
14481464
1449 return gtk_image_menu_item_get_always_show_image (GTK_IMAGE_MENU_ITEM (item));1465 if (GTK_IS_IMAGE_MENU_ITEM (item))
1466 return gtk_image_menu_item_get_always_show_image (GTK_IMAGE_MENU_ITEM (item));
1450 }1467 }
14511468
1452 return FALSE;1469 return FALSE;

Subscribers

People subscribed via source and target branches

to all changes: