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

[Dan Williams]

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.

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.

[Marco Trevisan (Treviño) (3v1n0)]

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]

478. By Dan Williams <email address hidden> on 2016-02-23

gtk: fix some GTKv3 deprecations and get rid of HAVE_GTK3

479. By Dan Williams <email address hidden> on 2016-02-23

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.

[Dan Williams (dcbw)]

> 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.

[Dan Williams (dcbw)]

review: Needs Resubmitting

[Dan Williams (dcbw)]

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

[Marco Trevisan (Treviño) (3v1n0)]

> 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

=== 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 @@
                                          glib-2.0 >= $GLIB_REQUIRED_VERSION,
                                          [have_gtk=yes]
 )
-         AC_DEFINE(HAVE_GTK3, 1, [whether gtk3 is available])
         ],
       [test "x$with_gtk" = x2],
         [PKG_CHECK_MODULES(DBUSMENUGTK,  gtk+-2.0 >= $GTK_REQUIRED_VERSION

=== 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 @@
 				   doesn't expose the right variables.  We need to figure
 				   this out as menus won't get grabs properly.
 				   TODO FIXME HELP ARGHHHHHHHH */
-#if (HAVE_GTK3 == 0)
+#if !GTK_CHECK_VERSION(3,0,0)
 				if (!GTK_MENU_SHELL (parent)->active) {
 					gtk_grab_add (parent);
 					GTK_MENU_SHELL (parent)->have_grab = TRUE;
@@ -1278,7 +1278,12 @@
 		gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
 
 		gtk_widget_set_size_request(GTK_WIDGET(gtkimage), width, height);
+#if GTK_CHECK_VERSION(3,0,0)
+		gtk_widget_set_halign(GTK_WIDGET(gtkimage), GTK_ALIGN_START);
+		gtk_widget_set_valign(GTK_WIDGET(gtkimage), GTK_ALIGN_CENTER);
+#else
 		gtk_misc_set_alignment(GTK_MISC(gtkimage), 0.0, 0.5);
+#endif
 	}
 
 	genericmenuitem_set_image(GENERICMENUITEM(gimi), gtkimage);

=== 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 @@
 /* GObject stuff */
 G_DEFINE_TYPE (Genericmenuitem, genericmenuitem, GTK_TYPE_CHECK_MENU_ITEM);
 
-#if HAVE_GTK3
+#if GTK_CHECK_VERSION(3,0,0)
 static void draw_indicator (GtkCheckMenuItem *check_menu_item, cairo_t *cr);
 static void (*parent_draw_indicator) (GtkCheckMenuItem *check_menu_item, cairo_t *cr) = NULL;
 #else
@@ -83,7 +83,7 @@
 	object_class->dispose = genericmenuitem_dispose;
 	object_class->finalize = genericmenuitem_finalize;
 
-#ifdef HAVE_GTK3
+#if GTK_CHECK_VERSION(3,2,0)
 	GtkWidgetClass * widget_class = GTK_WIDGET_CLASS(klass);
 
 	gtk_widget_class_set_accessible_role(widget_class, ATK_ROLE_MENU_ITEM);
@@ -115,7 +115,7 @@
 	self->priv->disposition = GENERICMENUITEM_DISPOSITION_NORMAL;
 	self->priv->label_text = NULL;
 
-#ifndef HAVE_GTK3
+#if !GTK_CHECK_VERSION(3,0,0)
 	AtkObject * aobj = gtk_widget_get_accessible(GTK_WIDGET(self));
 	if (aobj != NULL) {
 		atk_object_set_role(aobj, ATK_ROLE_MENU_ITEM);
@@ -148,7 +148,7 @@
 /* Checks to see if we should be drawing a little box at
    all.  If we should be, let's do that, otherwise we're
    going suppress the box drawing. */
-#if HAVE_GTK3
+#if GTK_CHECK_VERSION(3,0,0)
 static void
 draw_indicator (GtkCheckMenuItem *check_menu_item, cairo_t *cr)
 {
@@ -310,12 +310,12 @@
 			/* We need to put the child into a new box and
 			   make the box the child of the menu item.  Basically
 			   we're inserting a box in the middle. */
-			#ifdef HAVE_GTK3
+#if GTK_CHECK_VERSION(3,0,0)
 			GtkWidget * hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL,
 			                               get_toggle_space(GTK_WIDGET(menu_item)));
-			#else
+#else
 			GtkWidget * hbox = gtk_hbox_new(FALSE, get_toggle_space(GTK_WIDGET(menu_item)));
-			#endif
+#endif
 			g_object_ref(child);
 			gtk_container_remove(GTK_CONTAINER(menu_item), child);
 			gtk_box_pack_start(GTK_BOX(hbox), child, FALSE, FALSE, 0);
@@ -334,7 +334,12 @@
 		/* Build it */
 		labelw = GTK_LABEL(gtk_accel_label_new(local_label));
 		gtk_label_set_use_markup(GTK_LABEL(labelw), TRUE);
+#if GTK_CHECK_VERSION(3,0,0)
+		gtk_widget_set_halign(GTK_WIDGET(labelw), GTK_ALIGN_START);
+		gtk_widget_set_valign(GTK_WIDGET(labelw), GTK_ALIGN_CENTER);
+#else
 		gtk_misc_set_alignment(GTK_MISC(labelw), 0.0, 0.5);
+#endif
 		gtk_accel_label_set_accel_widget(GTK_ACCEL_LABEL(labelw), GTK_WIDGET(menu_item));
 
 		if (has_mnemonic(in_label, FALSE)) {
@@ -541,12 +546,12 @@
 			/* We need to put the child into a new box and
 			   make the box the child of the menu item.  Basically
 			   we're inserting a box in the middle. */
-			#ifdef HAVE_GTK3
+#if GTK_CHECK_VERSION(3,0,0)
 			GtkWidget * hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL,
 			                               get_toggle_space(GTK_WIDGET(menu_item)));
-			#else
+#else
 			GtkWidget * hbox = gtk_hbox_new(FALSE, get_toggle_space(GTK_WIDGET(menu_item)));
-			#endif
+#endif
 			g_object_ref(child);
 			gtk_container_remove(GTK_CONTAINER(menu_item), child);
 			gtk_box_pack_end(GTK_BOX(hbox), child, TRUE, TRUE, 0);

=== 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 @@
 static void           update_icon              (DbusmenuMenuitem *  menuitem,
                                                 ParserData *        pdata,
                                                 GtkImage *          image);
-static GtkWidget *    find_menu_label          (GtkWidget *         widget);
+static GtkWidget *    find_menu_child          (GtkWidget *         widget,
+                                                GType               child_type);
 static void           label_notify_cb          (GtkWidget *         widget,
                                                 GParamSpec *        pspec,
                                                 gpointer            data);
@@ -648,7 +649,7 @@
 
       gboolean visible = FALSE;
       gboolean sensitive = FALSE;
-      if (GTK_IS_SEPARATOR_MENU_ITEM (widget) || !find_menu_label (widget))
+      if (GTK_IS_SEPARATOR_MENU_ITEM (widget) || !find_menu_child (widget, GTK_TYPE_LABEL))
         {
           dbusmenu_menuitem_property_set (mi,
                                           DBUSMENU_MENUITEM_PROP_TYPE,
@@ -659,6 +660,8 @@
         }
       else
         {
+          GtkWidget *image = NULL;
+
           pdata->widget_accel_handler_id = g_signal_connect (widget, "accel-closures-changed",
                                                              G_CALLBACK (accel_changed), mi);
 
@@ -674,20 +677,26 @@
 
               pdata->widget_toggle_handler_id = g_signal_connect (widget, "activate", G_CALLBACK (checkbox_toggled), mi);
             }
-
-          if (GTK_IS_IMAGE_MENU_ITEM (widget))
+          else if (GTK_IS_IMAGE_MENU_ITEM (widget))
             {
-              GtkWidget *image;
 
               image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM (widget));
 
-              if (GTK_IS_IMAGE (image))
-                {
-                  update_icon (mi, pdata, GTK_IMAGE (image));
-                }
-            }
-
-          GtkWidget *label = find_menu_label (widget);
+            }
+          else
+            {
+              // GtkImageMenuItem is deprecated, so check regular GtkMenuItems
+              // for an image child too
+              image = find_menu_child (widget, GTK_TYPE_IMAGE);
+            }
+
+          if (GTK_IS_IMAGE (image))
+            {
+              update_icon (mi, pdata, GTK_IMAGE (image));
+            }
+
+
+          GtkWidget *label = find_menu_child (widget, GTK_TYPE_LABEL);
 
           // Sometimes, an app will directly find and modify the label
           // (like empathy), so watch the label especially for that.
@@ -907,7 +916,11 @@
                                              GTK_ICON_LOOKUP_FORCE_SIZE);
       if (info != NULL) {
         pixbuf = gtk_icon_info_load_icon (info, NULL);
+#if GTK_CHECK_VERSION(3,8,0)
+        g_object_unref (info);
+#else
         gtk_icon_info_free (info);
+#endif
       }
       break;
 
@@ -944,11 +957,11 @@
 }
 
 static GtkWidget *
-find_menu_label (GtkWidget *widget)
+find_menu_child (GtkWidget *widget, GType child_type)
 {
-  GtkWidget *label = NULL;
+  GtkWidget *child = NULL;
 
-  if (GTK_IS_LABEL (widget))
+  if (G_TYPE_CHECK_INSTANCE_TYPE (widget, child_type))
     return widget;
 
   if (GTK_IS_CONTAINER (widget))
@@ -960,16 +973,16 @@
 
       for (l = children; l; l = l->next)
         {
-          label = find_menu_label (l->data);
+          child = find_menu_child (l->data, child_type);
 
-          if (label)
+          if (child)
             break;
         }
 
       g_list_free (children);
     }
 
-  return label;
+  return child;
 }
 
 static void
@@ -1121,7 +1134,7 @@
     {
       DbusmenuMenuitem * item = DBUSMENU_MENUITEM(data);
       GtkWidget *widget = gtk_accessible_get_widget (GTK_ACCESSIBLE (accessible));
-      GtkWidget *label = find_menu_label (widget);
+      GtkWidget *label = find_menu_child (widget, GTK_TYPE_LABEL);
       const gchar *label_text = gtk_label_get_text (GTK_LABEL (label));
       const gchar *name = atk_object_get_name (accessible);
 
@@ -1335,7 +1348,7 @@
                GtkWidget *child,
                gpointer   data)
 {
-  if (find_menu_label (child) != NULL)
+  if (find_menu_child (widget, GTK_TYPE_LABEL) != NULL)
     handle_first_label (data);
 }
 
@@ -1433,6 +1446,9 @@
 
   item = gtk_widget_get_ancestor (GTK_WIDGET (image),
                                   GTK_TYPE_IMAGE_MENU_ITEM);
+  if (!item)
+    item = gtk_widget_get_ancestor (GTK_WIDGET (image),
+                                    GTK_TYPE_MENU_ITEM);
 
   if (item)
     {
@@ -1446,7 +1462,8 @@
       if (gtk_menu_images)
         return TRUE;
 
-      return gtk_image_menu_item_get_always_show_image (GTK_IMAGE_MENU_ITEM (item));
+      if (GTK_IS_IMAGE_MENU_ITEM (item))
+        return gtk_image_menu_item_get_always_show_image (GTK_IMAGE_MENU_ITEM (item));
     }
 
   return FALSE;