Thinking about this more, I believe the specific problem is that 
GtkAccelLabel gets the accelerator in an idle. 

It does a non-idle refetch in its init() routine, but it might work 
better to do this in size_request(). Since I create the menu item, 
then set the accel path.

Well, you can work around it by calling gtk_accel_labeL_refetch()
yourself. 

The call to gtk_accel_label_refetch () in gtk_accel_label_init () is
quite pointless, btw, since accel_group is guaranteed NULL at that
point, so you're refetching nothing.

The API of gtkaccellabel is a bit strange, currently.
Since gtk_accel_label_set_accel_widget () refetches idly, 
you must call gtk_accel_label_refetch () in order to ensure that
accel_string is uptodate. Then you must call
gtk_accel_label_size_request () in order for accel_string_width to be
calculated.

The appended patch fixes my problems with accel labels by calling
gtk_accel_label_refetch () immediately before
gtk_accel_label_size_request () in gtk_menu_item_size_request ().

Created attachment 11860 [details] [review]
patch

I think the reasoning of the accellabel code is that since
the idle is added as with priority HIGH_IDLE it will be
called before the resize queue runs. But of course, 
gtk_widget_size_request() can be called other times then
in the resize queue ... in particular 

I think the right thing to do is to have an internal variant
of gtk_accel_label_refetch() that takes a gboolean as to
whether it is being called out of size_request, and when
called out of a size request
 
 a) Don't do anything unless a refetch has been queued
 b) Don't queue a resize, clear a redraw instead.

Then call that internal variant from gtk_accel_label_size_request().
It definitely should be fixed inside GtkAccelLabel.

Miles: is the context menu in gnome-terminal the only menu that shows
delayed accelerator rendering?
Matthias: the call to gtk_accel_label_refetch () in
gtk_accel_label_init () is not pointless, it initialized fields beyond
NULL.
Havoc: are you sure the flicker is due to the accel labels idle
handler, and not maybe setting up accel widgets or accelerators after
the first exposes? i think it's pretty unlikely this being the accel
labels idle handler, since it's run prior to the resize queue.
Matthias: "your problems" which the attached patch fixes, are those
the exact problems miles describes, or are you referring to your API
issues?
Owen: your sentences left too many words out to make much sense. in
any case, i'd like to know why gtk_accel_label_refetch() should be
called from size_request at all, since it's idler should always get
run prior to the resize queue. even if size_request is called out of
order on the accel label, queued refetches still fire prior to other
resizes and queue a new resize on the accel label.
the only cases i come up with that could break the idler are
GTK_RESIZE_IMMEDIATE scenarios, or forced resize+redraw like the
tear-off code does it.

> Havoc: are you sure the flicker is due to the accel labels idle
> handler, and not maybe setting up accel widgets or accelerators after
> the first exposes? i think it's pretty unlikely this being the accel
> labels idle handler, since it's run prior to the resize queue.

The accel widgets and accelerators are set up prior to gtk_menu_popup().
Are you sure creating a menu then immediately calling gtk_menu_popup()
will ever run idle handlers prior to mapping the menu?

> Are you sure creating a menu then immediately calling
> gtk_menu_popup() will ever run idle handlers prior to
> mapping the menu?
good point havoc. looking at gtkmenu.c, the new scrolling code in
gtk_menu_popup() actually forcefully size-requests and realizes the
menu, thus passing by any asyncronous updates queued on the widget
tree (even if queued with high priority) and despite resizing policy
set on the containers. i'm not quite sure why the call to
gtk_menu_scroll_to() (for which the realization is done) isn't simply
deferred until the end of menu realization. i suspect the code should
be fixed in that regard (if not for the accel label, for other
asyncronous actions that might be setup, e.g. image items, etc...).
non-withstanding possible gtkmenu.c fixes however, it looks like the
accel label issue can be fixed by forcing a refetch in the accel
labels size_request handler if it's idle handler was queued and
skipping the queueing of a new size-request at that point. i suspect
that's also what owen meant to suggest.

Tim - note that gtk_window_show() normally calls
results in calling gtk_widget_size_request() sychronously, 
on the contents of the window, so there is
nothing particularly different in what GtkMenu does
other than the details.

Widgets cannot, in general, rely on idles being run
before size_request/size_allocate, though it may be
useful to use a setup with high priority idles for
compression of expensive operations. (I have some doubts
that the operations for GtkAccelLabel are really
expensive enough to warrent extra complexity. But they
shouldn't hurt if we handle the case of size_request
being called before the idle.)

Tim, looking through gtk_accel_label_refetch (), I find that the call from 
gtk_accel_label_init () will do exactly 2 things:

a) accel_label-
>accel_string = g_strdup ("");
b) gtk_widget_queue_resize (GTK_WIDGET 
(accel_label));

So I fail to see where any fields would be non-trivially 
initialized 
here...

I think it would be much clearer to move these two directly 
to init
(if the resize is even wanted in that situation...)

b) isn't necessar either in an init - a resize will always
be queued if the menu item is added to a container.

Isn't the better to just remove all the idle thing entirely. As far as
I can see, nothing particularly expensive is going on in
gtk_accel_label_refetch().

Random fact: another symptom of this bug is that the the accel part of
the gnome terminal's context menu will not be clamped to the screen,
so if you right click with the terminal sufficiently close to the
right edge, part of menu will pop up outside the screen.

I am attaching a patch that removes the idle handler completely.

Created attachment 12973 [details] [review]
Patch that removes idle handler

One possible worry is that there might be reentrancy problems
of some sort in calling gtk_accel_group_find() immediately
on change notification. (I don't see that obviously, but
it woudl be another reason to do it out of an idle.)

I think what you have here is probably fine; my opinion of
the optimum method is to have the current
gtk_accel_label_queue_refetch() become:

static void
gtk_accel_label_reset (GtkAccelLabel *accel_label)
{
  if (label->string)
    {
      g_free (accel_label->accel_string);
      accel_label->accel_string = NULL;
    }

  gtk_widget_queue_resize (accel_label);
}

Then add 

static const gchar *
gtk_accel_label_get_string (GtkAccelLabel *accel_label)
{
  if (!accel_label->accel_string)
    {
      /* Compute accel string */
    }
 
   return accel_label->accel_string;
}

That's my favorite setup in this situation, anyways. (GtkLabel,
PangoLayout, GtkEntry, etc, all do something like that.)

Either way probably works; if you go with your current patch,
make sure you test both with gtk-can-change-accels=1 and the
funky stuff that the gnome-terminal bindings dialog does.

I went with my current patch for now.

Sat Dec 14 01:22:05 2002  Soeren Sandmann  <sandmann@daimi.au.dk>

	* gtk/gtkaccellabel.[ch] (gtk_accel_label_refetch): Don't
	recalculate the acceleration label in an idle handler.