I have the same problem, and I am trying to debug it now, the accelerator seems to work just fine, it seems its the menu item that is not showing the accel label properly

Created attachment 212462 [details] [review]
Test case patch for gtk

Created attachment 212463 [details] [review]
Test program using an accelerator with builder

Created attachment 212464 [details]
builder file for test program

Created attachment 213710 [details] [review]
Proposed patch

This patch adds accel-group property to GtkActionGroup which is needed to call
gtk_accel_group_connect_by_path() in gtk_action_group_buildable_custom_tag_end()
for the accel label to work in menu items.
This only works if we actually set the accel group in the action group, but I guess action group could create one itself, which would have to be set in a window to work anyway.

Review of attachment 213710 [details] [review]:

thanks for the patch.

it would be great if you attached a patch generated by a git commit - either using git format-patch or git bz - so that we can also review the commit message as well as properly credit your contribution.

this is mostly a preliminary review: you'll need another one.

thanks again!

::: gtk/gtkactiongroup.c
@@ +244,3 @@
 							 TRUE,
 							 GTK_PARAM_READWRITE));
+  g_object_class_install_property (gobject_class,

the property should be documented.

@@ +571,3 @@
 
+  if (private->accel_group)
+    g_object_unref (private->accel_group);

you can use g_clear_object() instead.

@@ +787,3 @@
+  private = action_group->priv;
+
+ */

you can use:

  return action_group->priv->accel_group;

here, without using a separate variable.

@@ +831,3 @@
 /**
+ * gtk_action_group_set_accel_group:
+ * @action_group: the action group

the common pattern is to use "a #TypeName" in the annotation for the first argument

@@ +832,3 @@
+ * gtk_action_group_set_accel_group:
+ * @action_group: the action group
+ * @accel_group:

missing documentation for accel_group

@@ +834,3 @@
+ * @accel_group:
+ *
+ * Sets the accelerator group to use. 

a more descriptive annotation would be nice; why would you want to do this, for instance.

@@ +847,3 @@
+
+  private = action_group->priv;
+void

you should check if accel_group is the same one stored inside the private data structure, and return if so, to avoid resetting the same AccelGroup instance.

@@ +848,3 @@
+  private = action_group->priv;
+
+void

always put the statement on a separate line; and you can use g_clear_object() instead.

@@ +851,3 @@
+
+  private->accel_group = accel_group;
+                                  GtkAccelGroup  *accel_group)

you don't need to cast to GObject; and you can assign the value returned by g_object_ref() instead of separating the lines.

::: gtk/gtkactiongroup.h
@@ +166,3 @@
 void            gtk_action_group_set_visible             (GtkActionGroup             *action_group,
 							  gboolean                    visible);
+GtkAccelGroup  *gtk_action_group_get_accel_group         (GtkActionGroup             *action_group);

you should add the GDK_AVAILABLE_IN_3_6 annotation

@@ +168,3 @@
+GtkAccelGroup  *gtk_action_group_get_accel_group         (GtkActionGroup             *action_group);
+void            gtk_action_group_set_accel_group         (GtkActionGroup             *action_group,
+                                                          GtkAccelGroup              *accel_group);

same as above, you should add the GDK_AVAILABLE_IN_3_6 annotation

*** Bug 681894 has been marked as a duplicate of this bug. ***

Created attachment 221438 [details] [review]
Proposed patch 2nd try

Hello Emmanuele, I think I fixed all your recommendations.
Anyways my biggest concern is if adding a property is the right way to fix this issue or should we do some default magic instead.

Review of attachment 221438 [details] [review]:

So... one stylistic comment... is that you should not cause white noise in the patch because you prefer to use the name 'private' instead of 'priv'...

The other thing is, this can be done without modifying gtkmenuitem.c (with all the accelerator semantics, I'm paranoid about changing code and I think it should be done by changing the least lines of code as possible)

The only requirement is that gtk_action_set_accel_group() must be called for every action added to the GtkActionGroup.

So my suggestion, is yes... add 'accel-group' property to GtkActionGroup and:

  a.) When actions are added to the group, set the action group's accel-group on each
      added action using gtk_action_set_accel_group()

  b.) When gtk_action_group_set_accel_group() is called; iterate over all the owned
      actions and call gtk_action_set_accel_group() with the new accel-group for each owned action

AND, to make sure GtkUIManager does not break... GtkUIManager should also set it's privately created
accel group on the action groups it creates (since right now, there is a risk that the GtkActions
which get setup directly by uimanager might get thier accel groups unset by the GtkActionGroup).

Since... this is the way that UIManager always setup accelerators on actions, I feel like this
would be safer than modifying gtkmenuitem.c to do something different.

Review of attachment 221438 [details] [review]:

So... one stylistic comment... is that you should not cause white noise in the patch because you prefer to use the name 'private' instead of 'priv'...

The other thing is, this can be done without modifying gtkmenuitem.c (with all the accelerator semantics, I'm paranoid about changing code and I think it should be done by changing the least lines of code as possible)

The only requirement is that gtk_action_set_accel_group() must be called for every action added to the GtkActionGroup.

So my suggestion, is yes... add 'accel-group' property to GtkActionGroup and:

  a.) When actions are added to the group, set the action group's accel-group on each
      added action using gtk_action_set_accel_group()

  b.) When gtk_action_group_set_accel_group() is called; iterate over all the owned
      actions and call gtk_action_set_accel_group() with the new accel-group for each owned action

AND, to make sure GtkUIManager does not break... GtkUIManager should also set it's privately created
accel group on the action groups it creates (since right now, there is a risk that the GtkActions
which get setup directly by uimanager might get thier accel groups unset by the GtkActionGroup).

Since... this is the way that UIManager always setup accelerators on actions, I feel like this
would be safer than modifying gtkmenuitem.c to do something different.

Review of attachment 221438 [details] [review]:

So... one stylistic comment... is that you should not cause white noise in the patch because you prefer to use the name 'private' instead of 'priv'...

The other thing is, this can be done without modifying gtkmenuitem.c (with all the accelerator semantics, I'm paranoid about changing code and I think it should be done by changing the least lines of code as possible)

The only requirement is that gtk_action_set_accel_group() must be called for every action added to the GtkActionGroup.

So my suggestion, is yes... add 'accel-group' property to GtkActionGroup and:

  a.) When actions are added to the group, set the action group's accel-group on each
      added action using gtk_action_set_accel_group()

  b.) When gtk_action_group_set_accel_group() is called; iterate over all the owned
      actions and call gtk_action_set_accel_group() with the new accel-group for each owned action

AND, to make sure GtkUIManager does not break... GtkUIManager should also set it's privately created
accel group on the action groups it creates (since right now, there is a risk that the GtkActions
which get setup directly by uimanager might get thier accel groups unset by the GtkActionGroup).

Since... this is the way that UIManager always setup accelerators on actions, I feel like this
would be safer than modifying gtkmenuitem.c to do something different.

Created attachment 224135 [details] [review]
Proposed patch 3rd try

Same patch but with no menuitem modification.
Works like a charm.

Review of attachment 224135 [details] [review]:

Looks nice, provided the question raised by Tristan about how this interacts with GtkUIManager is not a problem.
Some comments below:

::: gtk/gtkactiongroup.c
@@ +844,3 @@
+  private = action_group->priv;
+
+ * 

The return should be on a new line

@@ +848,3 @@
+  g_clear_object (&private->accel_group);
+
+ */

Since gtk_action_set_accel_group() allows for a NULL accel group to be set on it, I think we should keep the same semantics here.
This would mean
- not unconditionally ref the object here
- add an (allow none) annotation to the gtk-doc block, and an "or %NULL" string to the setter and the getter functions

::: gtk/gtkactiongroup.h
@@ +166,3 @@
 void            gtk_action_group_set_visible             (GtkActionGroup             *action_group,
 							  gboolean                    visible);
+GtkAccelGroup  *gtk_action_group_get_accel_group         (GtkActionGroup             *action_group) GDK_AVAILABLE_IN_3_6;

We usually put the annotation before the function, like

GDK_AVAILABLE_IN_3_6
void foo(bar *baz);

ops somehow I though g_object_ref() would return NULL on NULL; so that is a bug.
About UImanager, it wont interfere with it as long as you do not set an accel group on an action group you are going to use with a uimanager.
Do you think I should modify uimanager in this very same patch?
all we would have to do is use this new api gtk_action_group_set_accel_group() instead of setting the accel group on each action manually

Created attachment 224266 [details] [review]
improved 3rd try

So I took a look at gtkuimanager.c and it only call gtk_action_set_accel_group() in one place update_node() and since it already iterates over every action it is not worth to use this new API. BTW there is no way this patch can interfere with the uimanager even if you set an accel group on the action group since the uimanager will set its own accel group.

What I don't like about this patch is that it makes accel group handling more explicit - yet more places that references to accel groups are stored (in accel labels, menu items, actions, ui manager...and now action groups). Despite the fact that we really want to move in the opposite direction...

Anyway, everybody else seems to like it, so lets go with it.

Do not get me wrong I do not like accel groups either :)
Sometimes I wish we could associate actions groups directly with windows.

In any case what I am trying to do here is fix the fact that there is not way to get all this working using just GtkBuilder.

I think this is an area we have to improve/simplify in the next API break.

BTW commited in master