Merge lp:~mterry/appmenu-gtk/misc-fixes into lp:appmenu-gtk/0.4

[Michael Terry]

This branch does four things, all observed while working with the monster that is empathy.

1) Empathy changes the first label ("Context", "Contact", or "Group") of it's Edit menu by directly changing the label inside the menu item. So we need to watch for such direct edits as well. (this is the addition of label_notify_cb)

2) Empathy directly hides/shows the first label and first separator in its Edit menu on startup directly. It does not modify the backing GtkAction. So when we startup, we should take such possibilities in mind and prefer the visible/sensitive status of the widget directly over the action -- it is always likely to be more correct (though we should and do continue watching action status).

3) Empathy modifies the first label and separator of its Edit menu upon menu activation (a dynamic menu). We need to listen for about-to-show signals from dbusmenu and activate the menu for empathy, so it can modify the menu. There's a race here, because dbusmenu doesn't yet actually block on our being ready, but it works in practice, and is better than nothing right now.

4) Vastly simplify separator logic. There was existing code to monitor the 'smart' mode and tell when it should be shown. But GtkUIManager handles that for us by setting widget visibility state of the item. So instead, we should treat separators a bit more like normal items. So I attach the same notify callback to separators that were for other widgets before (letting me drop the separate child_notify_cb). This fixes issues in Empathy with showing the first separator in the Edit menu as well as the separators in the Help menu.

We still don't fully get the first label of the Edit menu right -- Empathy is actually setting a submenu. I'm working on another branch that watches the "submenu" property and tries to do intelligent things. But this work was splitable, so here it is.

[Michael Terry (mterry)]

Oh, whoops, and I forgot, this includes the approved branch https://code.launchpad.net/~mterry/appmenu-gtk/fix-sensitive-actions/+merge/47056

[Ted Gould (ted)]

Merged fix-sensitive-actions to trunk. Patch should regenerate shortly.

=== modified file 'src/bridge.c'
--- src/bridge.c	2011-01-14 17:25:07 +0000
+++ src/bridge.c	2011-01-21 21:35:56 +0000
@@ -88,15 +88,8 @@
   gint count;
   gboolean previous;
   DbusmenuMenuitem *stack[30];
-  gboolean          mode[30];
 } RecurseContext;
 
-enum {
-  SEPARATOR_MODE_SMART,
-  SEPARATOR_MODE_VISIBLE,
-  SEPARATOR_MODE_HIDDEN
-};
-
 G_DEFINE_DYNAMIC_TYPE(AppMenuBridge, app_menu_bridge, UBUNTU_TYPE_MENU_PROXY)
 
 static GHashTable *rebuild_ids = NULL;
@@ -480,17 +473,6 @@
   return label;
 }
 
-static const gchar *
-get_menu_label_text (GtkWidget *menuitem)
-{
-  GtkWidget *label = find_menu_label (menuitem);
-
-  if (label)
-    return gtk_label_get_text (GTK_LABEL (label));
-
-  return NULL;
-}
-
 static void
 item_activated (DbusmenuMenuitem *item, guint timestamp, gpointer user_data)
 {
@@ -508,6 +490,26 @@
 }
 
 static gboolean
+item_about_to_show (DbusmenuMenuitem *item, gpointer user_data)
+{
+  GtkWidget *child;
+
+  if (user_data != NULL)
+    {
+      child = (GtkWidget *)user_data;
+
+      if (GTK_IS_MENU_ITEM (child))
+        {
+          // Only called for items with submens.  So we activate it here in
+          // case the program dynamically creates menus (like empathy does)
+          gtk_menu_item_activate (GTK_MENU_ITEM (child));
+        }
+    }
+
+  return TRUE;
+}
+
+static gboolean
 should_show_image (GtkImage *image)
 {
   GtkWidget *item;
@@ -627,6 +629,41 @@
     {
       update_icon_name (child, widget);
     }
+  else if (pspec->name == g_intern_static_string ("parent"))
+    {
+      /*
+        * We probably should have added a 'remove' method to the
+        * UbuntuMenuProxy early on, but it's late in the cycle now.
+        */
+      if (gtk_widget_get_parent (widget) == NULL)
+        {
+          g_signal_handlers_disconnect_by_func (widget,
+                                                G_CALLBACK (widget_notify_cb),
+                                                child);
+
+          DbusmenuMenuitem *parent = g_object_get_data (G_OBJECT (child), "dbusmenu-parent");
+
+          if (DBUSMENU_IS_MENUITEM (parent) && DBUSMENU_IS_MENUITEM (child))
+            {
+              dbusmenu_menuitem_child_delete (parent, child);
+            }
+        }
+    }
+}
+
+static void
+label_notify_cb (GtkWidget  *widget,
+                 GParamSpec *pspec,
+                 gpointer    data)
+{
+  DbusmenuMenuitem *child = (DbusmenuMenuitem *)data;
+
+  if (pspec->name == g_intern_static_string ("label"))
+    {
+      dbusmenu_menuitem_property_set (child,
+                                      DBUSMENU_MENUITEM_PROP_LABEL,
+                                      gtk_label_get_text (GTK_LABEL (widget)));
+    }
 }
 
 static void
@@ -682,6 +719,12 @@
     {
       dbusmenu_menuitem_property_set_bool (mi,
                                            DBUSMENU_MENUITEM_PROP_ENABLED,
+                                           gtk_action_is_sensitive (action));
+    }
+  else if (pspec->name == g_intern_static_string ("visible"))
+    {
+      dbusmenu_menuitem_property_set_bool (mi,
+                                           DBUSMENU_MENUITEM_PROP_VISIBLE,
                                            gtk_action_is_visible (action));
     }
   else if (pspec->name == g_intern_static_string ("active"))
@@ -698,56 +741,26 @@
     }
 }
 
-/*
- * We probably should have added a 'remove' method to the UbuntuMenuProxy early on,
- * but it's late in the cycle now.
- */
-static void
-child_notify_cb (GtkWidget        *widget,
-                 GParamSpec       *pspec,
-                 DbusmenuMenuitem *mi)
-{
-  if (pspec->name == g_intern_static_string ("parent"))
-    {
-      if (gtk_widget_get_parent (widget) == NULL)
-        {
-          g_signal_handlers_disconnect_by_func (widget,
-                                                G_CALLBACK (child_notify_cb),
-                                                mi);
-
-          DbusmenuMenuitem *parent = g_object_get_data (G_OBJECT (mi), "dbusmenu-parent");
-
-          if (DBUSMENU_IS_MENUITEM (parent) && DBUSMENU_IS_MENUITEM (mi))
-            {
-              dbusmenu_menuitem_child_delete (parent, mi);
-            }
-        }
-    }
-}
-
 static DbusmenuMenuitem *
-construct_dbusmenu_for_widget (GtkWidget *widget, gboolean previous_separator)
+construct_dbusmenu_for_widget (GtkWidget *widget)
 {
   DbusmenuMenuitem *mi = dbusmenu_menuitem_new ();
 
   if (GTK_IS_MENU_ITEM (widget))
     {
+      gboolean visible = FALSE;
+      gboolean sensitive = FALSE;
       if (GTK_IS_SEPARATOR_MENU_ITEM (widget))
         {
           dbusmenu_menuitem_property_set (mi,
                                           "type",
                                           "separator");
 
-          gint mode = GPOINTER_TO_INT (g_object_get_data (G_OBJECT (widget), "gtk-separator-mode"));
-
-          dbusmenu_menuitem_property_set_bool (mi,
-                                               DBUSMENU_MENUITEM_PROP_VISIBLE,
-                                               mode == SEPARATOR_MODE_SMART && !previous_separator);
+          visible = gtk_widget_get_visible (widget);
+          sensitive = gtk_widget_get_sensitive (widget);
         }
       else
         {
-          gboolean visible = FALSE;
-          gboolean sensitive = FALSE;
           gboolean label_set = FALSE;
 
           g_signal_connect (widget,
@@ -799,23 +812,22 @@
                 }
             }
 
+          GtkWidget *label = find_menu_label (widget);
+
           dbusmenu_menuitem_property_set (mi,
                                           "label",
-                                          get_menu_label_text (widget));
+                                          label ? gtk_label_get_text (GTK_LABEL (label)) : NULL);
 
-          if (!g_object_get_data (G_OBJECT (widget), "gtk-empty-menu-item") && !GTK_IS_TEAROFF_MENU_ITEM (widget))
+          if (label)
             {
-              visible = gtk_widget_get_visible (widget);
-              sensitive = gtk_widget_get_sensitive (widget);
+              // Sometimes, an app will directly find and modify the label
+              // (like empathy), so watch the label especially for that.
+              g_signal_connect (G_OBJECT (label),
+                                "notify",
+                                G_CALLBACK (label_notify_cb),
+                                mi);
             }
 
-          dbusmenu_menuitem_property_set_shortcut_menuitem (mi, GTK_MENU_ITEM (widget));
-
-          g_signal_connect (G_OBJECT (widget),
-                            "notify",
-                            G_CALLBACK (widget_notify_cb),
-                            mi);
-
           if (GTK_IS_ACTIVATABLE (widget))
             {
               GtkActivatable *activatable = GTK_ACTIVATABLE (widget);
@@ -837,23 +849,36 @@
                 }
             }
 
-          dbusmenu_menuitem_property_set_bool (mi,
-                                               DBUSMENU_MENUITEM_PROP_VISIBLE,
-                                               visible);
+          if (!g_object_get_data (G_OBJECT (widget), "gtk-empty-menu-item") && !GTK_IS_TEAROFF_MENU_ITEM (widget))
+            {
+              visible = gtk_widget_get_visible (widget);
+              sensitive = gtk_widget_get_sensitive (widget);
+            }
 
-          dbusmenu_menuitem_property_set_bool (mi,
-                                               DBUSMENU_MENUITEM_PROP_ENABLED,
-                                               sensitive);
+          dbusmenu_menuitem_property_set_shortcut_menuitem (mi, GTK_MENU_ITEM (widget));
 
           g_signal_connect (G_OBJECT (mi),
                             DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK (item_activated),
                             widget);
+
+          g_signal_connect (G_OBJECT (mi),
+                            DBUSMENU_MENUITEM_SIGNAL_ABOUT_TO_SHOW,
+                            G_CALLBACK (item_about_to_show),
+                            widget);
         }
 
+      dbusmenu_menuitem_property_set_bool (mi,
+                                           DBUSMENU_MENUITEM_PROP_VISIBLE,
+                                           visible);
+
+      dbusmenu_menuitem_property_set_bool (mi,
+                                           DBUSMENU_MENUITEM_PROP_ENABLED,
+                                           sensitive);
+
       g_signal_connect (widget,
                         "notify",
-                        G_CALLBACK (child_notify_cb),
+                        G_CALLBACK (widget_notify_cb),
                         mi);
     }
 
@@ -867,7 +892,6 @@
   if (GTK_IS_CONTAINER (widget))
     {
       gboolean increment = GTK_IS_MENU_BAR (widget) || GTK_IS_MENU_ITEM (widget);
-      gboolean previous_separator = FALSE;
 
       if (increment)
         recurse->count++;
@@ -898,34 +922,6 @@
 
       if (recurse->count > -1 && increment)
         {
-          /* If this is a separator, find out if we've already displayed a visible separator during
-           * this run.  GtkUIManager internally defines the following separator modes:
-           *
-           * SEPARATOR_MODE_SMART
-           * SEPARATOR_MODE_VISIBLE
-           * SEPARATOR_MODE_HIDDEN
-           *
-           * construct_dbusmenu_for_widget() will mark a smart separator as visible in a run of
-           * separators unless it is following another smart separator anywhere in that run.
-           */
-          if (GTK_IS_SEPARATOR_MENU_ITEM (widget))
-            {
-              if (recurse->stack[recurse->count] != NULL)
-                {
-                  const gchar *type = dbusmenu_menuitem_property_get (recurse->stack[recurse->count],
-                                                                      DBUSMENU_MENUITEM_PROP_TYPE);
-
-                  if (g_strcmp0 (type, "separator") == 0)
-                    {
-                      /* Get the previous separator mode. */
-                      gint mode = recurse->mode[recurse->count];
-
-                      if (mode == SEPARATOR_MODE_SMART)
-                        previous_separator = TRUE;
-                    }
-                }
-            }
-
           DbusmenuMenuitem *dmi = g_hash_table_lookup (recurse->context->lookup, widget);
           if (dmi != NULL)
             {
@@ -936,25 +932,10 @@
             }
           else
             {
-              recurse->stack[recurse->count] = construct_dbusmenu_for_widget (widget, previous_separator);
+              recurse->stack[recurse->count] = construct_dbusmenu_for_widget (widget);
               g_hash_table_insert (recurse->context->lookup, widget, recurse->stack[recurse->count]);
             }
 
-          if (GTK_IS_SEPARATOR_MENU_ITEM (widget))
-            {
-              /* If the previous separator was mode 1, let's pretend that this separator is also mode 1.
-               * That means for the remainder of this run of separators, all will be marked as mode 1.
-               */
-              if (previous_separator)
-                {
-                  recurse->mode[recurse->count] = SEPARATOR_MODE_SMART;
-                }
-              else
-                {
-                  recurse->mode[recurse->count] = GPOINTER_TO_INT (g_object_get_data (G_OBJECT (widget), "gtk-separator-mode"));
-                }
-            }
-
           if (!gtk_widget_get_visible (widget))
             {
               g_signal_connect (G_OBJECT (widget),
@@ -1335,7 +1316,7 @@
 
           if (mi != NULL)
             {
-              DbusmenuMenuitem *child_dmi = construct_dbusmenu_for_widget (child, FALSE);
+              DbusmenuMenuitem *child_dmi = construct_dbusmenu_for_widget (child);
 
               g_object_set_data (G_OBJECT (child_dmi), "dbusmenu-parent", mi);
               dbusmenu_menuitem_child_add_position (mi,