After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 512649 - display of (metacity's) accelerator in window-action-menu
display of (metacity's) accelerator in window-action-menu
Status: RESOLVED OBSOLETE
Product: libwnck
Classification: Core
Component: general
2.21.x
Other Linux
: Normal enhancement
: ---
Assigned To: libwnck maintainers
libwnck maintainers
Depends on:
Blocks:
 
 
Reported: 2008-01-28 20:37 UTC by Mirco Müller
Modified: 2018-01-24 13:43 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
the patch mentioned in the bug-description (7.21 KB, patch)
2008-01-28 20:38 UTC, Mirco Müller
needs-work Details | Review
according to comments revised patch (6.70 KB, patch)
2008-02-04 14:01 UTC, Mirco Müller
none Details | Review

Description Mirco Müller 2008-01-28 20:37:36 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.
Comment 1 Mirco Müller 2008-01-28 20:38:32 UTC
Created attachment 103912 [details] [review]
the patch mentioned in the bug-description
Comment 2 Vincent Untz 2008-02-04 11:29:37 UTC
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)?
Comment 3 Mirco Müller 2008-02-04 14:01:50 UTC
Created attachment 104380 [details] [review]
according to comments revised patch
Comment 4 Mirco Müller 2008-02-04 14:05:28 UTC
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. 
Comment 5 Vincent Untz 2008-02-04 14:31:44 UTC
(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.
Comment 6 Mirco Müller 2008-02-04 14:51:32 UTC
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".
Comment 7 GNOME Infrastructure Team 2018-01-24 13:43:15 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/libwnck/issues/100.