Merge lp:~mterry/appmenu-gtk/fix-alt-shortcuts into lp:appmenu-gtk/0.4

[Michael Terry]

When we moved the parser to dbusmenu, we forgot about the lookup table for widgets to menu items in appmenu-gtk. This lookup table is used to support key presses like Alt+F for the menus.

To fix this, I used the sorta-secret gobject data "dbusmenu-gtk-parser-cached-item" saved on GtkMenuItems by the parser to map to Dbusmenuitems. This may be considered bad?

If it is considered bad to use that string directly, we should either expose the string in a header or introduce a convenience function to dbusmenu to grab the data.

[Ted Gould (ted)]

On Wed, 2011-03-09 at 20:12 +0000, Michael Terry wrote:
> To fix this, I used the sorta-secret gobject data "dbusmenu-gtk-parser-cached-item" saved on GtkMenuItems by the parser to map to Dbusmenuitems. This may be considered bad?
>
> If it is considered bad to use that string directly, we should either expose the string in a header or introduce a convenience function to dbusmenu to grab the data.

I think it's better to expose via API. What do you think about:

  lp:~ted/dbusmenu/get-cached-menuitem

I think that works and is API we could support for a while.

[Michael Terry (mterry)]

Looks fine. I updated this branch to use the new function.

[lp:~mterry/appmenu-gtk/fix-alt-shortcuts]

128. By Michael Terry on 2011-03-10

update to use new get_cached_item function

129. By Michael Terry on 2011-03-10

whoops, remove some debugging code I accidentally committed

[Ted Gould (ted)]

review: Approve

=== modified file 'src/bridge.c'
--- src/bridge.c	2011-03-09 15:56:32 +0000
+++ src/bridge.c	2011-03-10 14:06:48 +0000
@@ -75,7 +75,6 @@
   gchar            *path;
   gboolean          registered;
   AppMenuBridge    *bridge;
-  GHashTable       *lookup;
   GCancellable     *cancel;
 };
 
@@ -108,24 +107,14 @@
                GtkWidget     *widget,
                gpointer       user_data)
 {
-  GList *list;
+  DbusmenuMenuitem *mi = dbusmenu_gtk_parse_get_cached_item (widget);
 
-  for (list = bridge->priv->windows; list != NULL; list = list->next)
+  if (mi != NULL)
     {
-      AppWindowContext *context = list->data;
-
-      if (context->lookup)
-        {
-          DbusmenuMenuitem *mi = g_hash_table_lookup (context->lookup, widget);
-
-          if (mi != NULL)
-            {
-              dbusmenu_menuitem_show_to_user (mi, 0);
-
-              return;
-            }
-        }
+      dbusmenu_menuitem_show_to_user (mi, 0);
     }
+
+  return;
 }
 
 static void
@@ -178,12 +167,6 @@
       g_object_remove_weak_pointer(G_OBJECT(context->window), (gpointer *)&(context->window));
       context->window = NULL;
     }
-
-  if (context->lookup != NULL)
-    {
-      g_hash_table_unref (context->lookup);
-      context->lookup = NULL;
-    }
 }
 
 static void
@@ -678,7 +661,6 @@
     {
       context = g_new0 (AppWindowContext, 1);
       context->bridge = bridge;
-      context->lookup = g_hash_table_new (g_direct_hash, g_direct_equal);
       bridge->priv->windows = g_list_prepend (bridge->priv->windows, context);
     }
 
@@ -849,17 +831,20 @@
   else if (GTK_IS_MENU (parent))
     {
       GtkWidget *attach = NULL;
-      GList *tmp = NULL;
-      AppWindowContext *context = NULL;
 
       g_object_get (parent, "attach-widget", &attach, NULL);
 
-      /* First attempt to setup the menuitem immediately if possible. */
-      for (tmp = bridge->priv->windows; tmp != NULL; tmp = tmp->next)
-        {
-          context = (AppWindowContext *)tmp->data;
-
-          DbusmenuMenuitem *mi = (DbusmenuMenuitem *)g_hash_table_lookup (context->lookup, attach);
+      if (attach == NULL)
+        {
+          g_signal_connect (G_OBJECT (parent),
+                            "notify",
+                            G_CALLBACK (attach_notify_cb),
+                            bridge);
+          return;
+        }
+      else
+        {
+          DbusmenuMenuitem *mi = dbusmenu_gtk_parse_get_cached_item (attach);
 
           if (mi != NULL)
             {
@@ -871,18 +856,7 @@
                                                     position);
               return;
             }
-        }
 
-      if (attach == NULL)
-        {
-          g_signal_connect (G_OBJECT (parent),
-                            "notify",
-                            G_CALLBACK (attach_notify_cb),
-                            bridge);
-          return;
-        }
-      else
-        {
           rebuild (bridge, toplevel);
         }
     }