GNOME Bugzilla – Bug 512649
display of (metacity's) accelerator in window-action-menu
Last modified: 2008-02-04 14:51:32 UTC
The window-action-menu does not display any accelerators (e.g. Alt+F7 for move, metacity-default) in the menu it creates. This is a drawback and makes discovery of these key-bindings difficult for the user. The supplied patch solves this. It allows to have accelerators be displayed if libwnck is compiled with "--enable-gconf". The gconf-keys read can easily be adjusted in the #defines. A side-effect of this patch is that it will display accelerators for compiz, since compiz is properly using libwnck for the action-menu, which metacity does not right now. Since compiz defaults to metacitiy's keybindings this is a desired effect. Furthermore it's now possible to change metacity to make use of wnck_action_menu_new() too.
Created attachment 103912 [details] [review] the patch mentioned in the bug-description
Comment on attachment 103912 [details] [review] the patch mentioned in the bug-description I still don't know how much we want this, since this will make people depending on libwnck in non-GNOME environment depend on gconf (unless distributors create two packages, which is really something I don't see happening). Anyway, some comments on the code (I think the configure.in part could be improved too, but I'd need to check): >@@ -77,6 +96,9 @@ > struct _WnckActionMenuPrivate > { > WnckWindow *window; >+#ifdef USE_GCONF >+ GtkAccelGroup *accel_group; >+#endif /* USE_GCONF */ > GtkWidget *minimize_item; > GtkWidget *maximize_item; > GtkWidget *above_item; Add accel_group at the end of the structure. >@@ -979,6 +1001,9 @@ > menu->priv = WNCK_ACTION_MENU_GET_PRIVATE (menu); > > menu->priv->window = NULL; >+#ifdef USE_GCONF >+ menu->priv->accel_group = NULL; >+#endif /* USE_GCONF */ > menu->priv->minimize_item = NULL; > menu->priv->maximize_item = NULL; > menu->priv->above_item = NULL; (move this at the end too) >@@ -996,6 +1021,38 @@ > menu->priv->idle_handler = 0; > } > >+#ifdef USE_GCONF >+static void >+wnck_add_accelerator (GConfClient *client, >+ gchar *gconf_key_path, >+ GtkWidget *menu_item, >+ GtkAccelGroup *accel_group) >+{ >+ GError *error = NULL; >+ guint accelerator_key = 0; >+ GdkModifierType accelerator_mods = 0; >+ >+ g_assert (client != NULL); >+ g_assert (gconf_key_path != NULL); >+ g_assert (menu_item != NULL); >+ g_assert (accel_group != NULL); >+ >+ gtk_accelerator_parse (gconf_client_get_string (client, >+ gconf_key_path, >+ &error), This is wrong. You need to do gconf_client_get_string(), check for error or null/empty string and then, if it's okay, call gtk_accelerator_parse(). >@@ -1027,12 +1086,10 @@ > g_object_weak_ref (G_OBJECT (menu), object_weak_notify, priv->window); > > priv->minimize_item = make_menu_item (MINIMIZE); >- > gtk_menu_shell_append (GTK_MENU_SHELL (menu), > priv->minimize_item); > > priv->maximize_item = make_menu_item (MAXIMIZE); >- > gtk_menu_shell_append (GTK_MENU_SHELL (menu), > priv->maximize_item); No such changes in patches, please :-) >@@ -1120,6 +1177,67 @@ > set_item_text (priv->close_item, _("_Close")); > set_item_stock (priv->close_item, WNCK_STOCK_DELETE); > >+#ifdef USE_GCONF >+ g_type_init (); Huh? Why would we need this here? >+ client = gconf_client_get_default (); >+ >+ priv->accel_group = gtk_accel_group_new (); >+ gtk_window_add_accel_group (GTK_WINDOW (gtk_widget_get_toplevel (priv->close_item)), Doesn't menu work, instead of tk_widget_get_toplevel (priv->close_item)?
Created attachment 104380 [details] [review] according to comments revised patch
Fixed all stated issues with first patch and attached new revised patch. g_type_init() is needed according to the API-reference of gconf_client_get_default(). GTK_MENU(priv->close_item) does not work. I tried that initally.
(In reply to comment #4) > Fixed all stated issues with first patch and attached new revised patch. > > g_type_init() is needed according to the API-reference of > gconf_client_get_default(). Right, but g_type_init() has already been called before. Else, GTK+ wouldn't work... > GTK_MENU(priv->close_item) does not work. I tried that initally. Sorry, I wasn't clear. I meant: gtk_window_add_accel_group (GTK_WINDOW (menu)). Hrm, thinking about it, it might not work. Anyway, using menu is better than some random item there. And this should work.
Indeed I can skip the call to g_type_init(). But for your suggested gtk_window_add_accel_group(GTK_WINDOW(menu)) I get runtime-warnings of invalid casts from glib... "invalid cast from WnckActionMenu to GtkWindow" and from gtk+ I get a critical warning "gtk_window_add_accel_group: assertion GTK_IS_WINDOW(window)" failed".