Merge lp:~bratsche/appmenu-gtk/uimanager-fixes into lp:appmenu-gtk/0.4

[Cody Russell]

This fixes a number of issues that occur when building menus from GtkUIManager, including empty submenus in wrong places (see the Quit menu in Anjuta), crashes (see the File menu in Nautilus). It reverts the change in revision 30 where we skip separators that follow other separators (this was actually the cause of the Nautilus crash), and instead adds all separators and marks invisible the ones which are actually placeholders/smart separators.

[Ted Gould (ted)]

Looks good. Not for this review but I'm curious if we shoudln't replace the "recurse" variable with a GArray?

review: Approve

=== modified file 'src/bridge.c'
--- src/bridge.c	2010-06-29 16:08:41 +0000
+++ src/bridge.c	2010-07-02 02:00:43 +0000
@@ -92,8 +92,15 @@
 
   gint count;
   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 guint rebuild_id = 0;
@@ -448,7 +455,7 @@
 }
 
 static DbusmenuMenuitem *
-construct_dbusmenu_for_widget (GtkWidget *widget)
+construct_dbusmenu_for_widget (GtkWidget *widget, gboolean previous_separator)
 {
   DbusmenuMenuitem *mi = dbusmenu_menuitem_new ();
 
@@ -459,9 +466,19 @@
           dbusmenu_menuitem_property_set (mi,
                                           "type",
                                           "separator");
+
+          gint mode = GPOINTER_TO_INT (g_object_get_data (G_OBJECT (widget), "gtk-separator-mode"));
+
+          // g_print ("mode == %d, previous_separator == %d\n", mode, previous_separator);
+
+          dbusmenu_menuitem_property_set_bool (mi,
+                                               DBUSMENU_MENUITEM_PROP_VISIBLE,
+                                               mode == SEPARATOR_MODE_SMART && !previous_separator);
         }
       else
         {
+          gboolean visible = FALSE;
+
           g_signal_connect (widget,
                             "accel-closures-changed",
                             G_CALLBACK (accel_changed),
@@ -487,9 +504,14 @@
                                           "label",
                                           get_menu_label_text (widget));
 
+          if (!g_object_get_data (G_OBJECT (widget), "gtk-empty-menu-item") && !GTK_IS_TEAROFF_MENU_ITEM (widget))
+            {
+              visible = gtk_widget_get_visible (widget);
+            }
+
           dbusmenu_menuitem_property_set_bool (mi,
                                                DBUSMENU_MENUITEM_PROP_VISIBLE,
-                                               gtk_widget_get_visible (widget));
+                                               visible);
 
           dbusmenu_menuitem_property_set_bool (mi,
                                                DBUSMENU_MENUITEM_PROP_ENABLED,
@@ -519,13 +541,23 @@
   if (GTK_IS_CONTAINER (widget))
     {
       gboolean increment = GTK_IS_MENU_BAR (widget) || GTK_IS_MENU_ITEM (widget);
-      gboolean skip = FALSE;
+      gboolean previous_separator = FALSE;
 
       if (increment)
         recurse->count++;
 
       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)
@@ -535,14 +567,30 @@
 
                   if (g_strcmp0 (type, "separator") == 0)
                     {
-                      skip = TRUE;
+                      /* Get the previous separator mode. */
+                      gint mode = recurse->mode[recurse->count];
+
+                      if (mode == SEPARATOR_MODE_SMART)
+                        previous_separator = TRUE;
                     }
                 }
             }
 
-          if (!gtk_widget_get_visible (widget))
+          recurse->stack[recurse->count] = construct_dbusmenu_for_widget (widget, previous_separator);
+
+          if (GTK_IS_SEPARATOR_MENU_ITEM (widget))
             {
-              skip = TRUE;
+              /* 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))
@@ -551,22 +599,25 @@
                                 "notify",
                                 G_CALLBACK (menuitem_notify_cb),
                                 recurse->context);
-              skip = TRUE;
-            }
-
-          if (!skip)
-            {
-              recurse->stack[recurse->count] = construct_dbusmenu_for_widget (widget);
-
-              if (recurse->count > 0)
-                {
-                  if (recurse->count == 1)
-                    dbusmenu_menuitem_child_append (recurse->stack[recurse->count - 1],
-                                                    recurse->stack[recurse->count]);
-                  else
-                    dbusmenu_menuitem_child_prepend (recurse->stack[recurse->count - 1],
-                                                     recurse->stack[recurse->count]);
-                }
+            }
+
+          // g_print ("%d: %s %s\n", recurse->count, G_OBJECT_TYPE_NAME (widget), get_menu_label_text (widget));
+
+          if (GTK_IS_TEAROFF_MENU_ITEM (widget))
+            {
+              dbusmenu_menuitem_property_set_bool (recurse->stack[recurse->count],
+                                                   DBUSMENU_MENUITEM_PROP_VISIBLE,
+                                                   FALSE);
+            }
+
+          if (recurse->count > 0)
+            {
+              if (recurse->count == 1)
+                dbusmenu_menuitem_child_append (recurse->stack[recurse->count - 1],
+                                                recurse->stack[recurse->count]);
+              else
+                dbusmenu_menuitem_child_prepend (recurse->stack[recurse->count - 1],
+                                                 recurse->stack[recurse->count]);
             }
         }