uline accels, aka. mnemonic activators

[Tim Janik <timj gtk org>]

hey all,

after reading owen's proposal, alex patch, and havoc's comments thereof,
i think we're "slightly" offtrack with the implementaiton of underline
accelerators vs. normal accelerators. lemme briefly say what accelerators
in the original Gtk sense are, and then how uline accels fit in.


the ::activate (widget, data) signal was introduced on GtkWidget to support
installation of hotkeys within a window for generic activation of widgets.
mainly because menus aren't necessarily part of a window's widget heirachy,
the association for these hotkeys is maintained through an extra indirection
of accel groups, i.e.: window->accel_group->activate_widget.

that is so that popup menus can be shared between windows, supporting their
menu item accelerators by adding the menu's relevant accel group to all the
windows within the popup menu is used.

in 1.2, owen added "uline accels" for menus (labels) and maintained those
through accel groups as well.
that approach has significant downsides:
1) the accel groups for uline accels have to be passed around in the API which
   is an uneccessary burden, since for underline accelerators it's pretty clear
   that they always belong to their nextmost viewing unit (that's the window
   for ordinary widgets, and the menu for menu items)
2) they also suffer from a basic accel groups limitation (which was and is
   intentional design btw), that's that accel group entries have to be unique
   across all accel groups for all the widgets the accel groups got added to
3) in contrast to per-view hotkeys, uline accels should only be enabled as
   long as their corresponding widget is viewable.
   e.g. the hotkey C-X of a menu item "_Cut <Ctrl+X>" should always work,
   regardless of the menu item's menu being popped up or down (that's also
   why they have to be unique per dialog).
   while the menu item's uline accelerator M-X (for menu bars) or X (for plain
   menus) should only be supported as long as their parent (menu bar or
   menu) is visible.

in short, using accel groups for uline accels is simply not the right approach.
owen's recent proposal suggests that we hack existing accel groups to cover
up for that limitation if the user specifies, support another signal
signature out of the box etc.
conceptually, i dislike that aproach very much because the accel group
code is badly hacked for a use it simply wasn't designed for (and rightly
so), and that happens for no good reason (though that's obviously not aparent).

i think we need to take a step back and gain understanding that uline acclerators
are a very different thing from dialog hotkeys,
though they partially share similar conceptual ideas, such as listening to key
presses and "activating" a widget.
at the point where owen suggested that conflicting uline accels within a dialog
should not be handled by "randomly picking a widget for activation", but instead
by focus-cycling between the conflicting widgtes (which is IMHO quite sensible),
it became clear to me that what we really need is a clean conceptual distinction.

to give a point by point overview:

1) gloabl dialog hotkeys:
  a) should always activate widgets, regardless of their visibility (of course
     still honouring sensitivity, but that's an entirely different concept
     joining the game, it simply needs to be paid attention to by the
     implementation)
  b) usually don't correspond to a label letter and need their own way to be
     displayed, the GtkAccelLabel widget handles displaying an accelerator name
     besides the normal label text
  c) have to be unique within a dialog, and may even be installed on widgets
     that aren't directly part of the dialog (such as menu items of an
     unconnected popup)
  d) due to (c) need to be maintained through the extra indirection of accel
     groups being added to a widget. also, because of their uniqueness, they
     can support runtime changes triggered by users.
  e) always emit ::activate on a widget

2) while uline accelerators or, (based on havocs naming idea)
   "Mnemonic Activators"
  a) should only be enabled as long as their corresponding widget is visible
     (and sensitive, but that's an implmentation detail)
  b) do always belong to widgets that are actually part of their viewing
     entity (window or menu), and usually correspond to a label that displays
     the activation key underlined
  c) can conflict with other "Mnemonic Activators", and due to that do either
     (e) or (f)
  d) are "static" in the sense that a user can't change them during runtime.
     and because of (b) shouldn't be handled through an extra "accel group"
     indirection
  e) emit ::activate on a widget if no conflicts arose
  f) round-robin focus through the conflicting widgets otherwise


based on that comparison, i think we better leave global, visibility unaware
hotkeys (to be referred to as accelerators) where they belong: in accel groups.
accel groups will ensure uniqueness, and, in connection with code like that
in the item factory, will handle propagation of accelerator runtime changes
within corresponding dialogs (plus serialization of those changes).

and we handle uline accelerators (suggestion: to be referred to as mnemonic
activators) from topmost viewing entities (currently, that's either GtkMenu
or GtkWindow). this gets rid of the accel group arguments for uline related
bindings, interference with accelerator serialization and runtime changes
(since accelerators should not be allowed to runtime change to a key combo
that is used as a mnemonic activator). introducing a new signal on GtkWidget
can then handle the distinction between (2.e) and (2.f).
since GtkMenu and GtkWindow exist in distinct widget heirarchy branches, it's
probably best to introduce a new interface:

typedef struct _GtkMnemonicActivator      GtkMnemonicActivator;
typedef struct _GtkMnemonicActivatorIface GtkMnemonicActivatorIface;

struct _GtkMnemonicActivatorIface
{
  GTypeInterface parent_iface;
  
  void (*add_mnemonic)  (GtkMnemonicActivator *widget,
                         guint                 keyval,
                         guint                 modifier,
                         GtkWidget            *activatee); /* owen: naming idea? */
  void (*remove_mnemonic)  (GtkMnemonicActivator *widget,
                            guint                 keyval,
                            guint                 modifier,
                            GtkWidget            *activatee);
  void (*mnemonic_activate) (GtkMnemonicActivator *widget,
                             guint                 keyval,
                             guint                 modifier);
};


GtkWindow for instance will upon key press events do:

static gint
gtk_window_key_press_event (GtkWidget   *widget,
                            GdkEventKey *event)
{
[...]
    
+  if (!handled)
+    handled = gtk_mnemonic_activator_activate (GTK_MNEMONIC_ACTIVATOR (window),
+                                               event->keyval,
+                                               event->state);
+  /* we probably also should pass on unhandled keypresses to our child,
+   * if GTK_IS_MNEMONIC_ACTIVATOR (window->child), to handle torn-off
+   * menus correctly
+   */
  if (!handled)
    handled = gtk_accel_groups_activate (GTK_OBJECT (window), event->keyval, event->state);
[...]
   
gtk_mnemonic_activator_activate() keeps its round-robin
list per (keyval,modifier) and once it's figured the widget
to activate it invokes gtk_widget_mnemonic_activate():

gboolean
gtk_widget_mnemonic_activate (GtkWidget *widget,
                              gboolean   group_cycling,
         /* can have optional gpointer mnemonic_data, like owen suggested */)
{
  gboolean handled = FALSE;
  
  g_return_val_if_fail (GTK_IS_WIDGET (widget), FALSE);
  
  if (!GTK_WIDGET_IS_SENSITIVE (widget))
    return TRUE;
  if (!GTK_WIDGET_CAN_FOCUS (widget) && group_cycling)
    return FALSE;
  if (!group_cycling && !GTK_WIDGET_GET_CLASS (widget)->activate_signal)
    return FALSE;
    
  gtk_signal_emit_by_name (GTK_OBJECT (widget),
                           "mnemonic_activate",
                           group_cycling,
                           &handled);
  return handled;
}

static gboolean
gtk_widget_real_mnemonic_activate (GtkWidget *widget,
                                   gboolean   group_cycling)
{
  if (group_cycling)
    gtk_widget_grab_focus (widget);
  else
    gtk_widget_activate (widget);
  return TRUE;
}

what's left is what widgets mnemonic actiavtors are installed on.
owen (and havoc) suggested that we should follow the basic API of:

 label = gtk_label_new_with_mnemonic ("_Name");
 gtk_label_set_mnemonic_widget (GTK_LABEL (label), entry);
 
which seems sensible, considering that mnemonics usually should be
displayed by underlined label letters. he also suggested that we
could automatically figure the mnemonic_widget by either walking
the labels ancestors, or finding its next child.
walking the ancestors is quie often desired, e.g. for labels (or
GtkAccelLabel) being added to menu items.
magically activating the next child, i think, is not a good idea,
things become tricky for dialogs like

+--------hbox----------------------------------+
+                  +--------alignment---------+|
|  _Label          |                   |entry|||
|                  +--------------------------+|
+----------------------------------------------+

or better yet, if a focusable range is inserted between
label and the alignment.
so i think we should keep magic to a predictable extend,
that is "walk up ancestry if mnemonic_widget==NULL" and require
explicit setting if that behaviour is not desired (we probably want
to gdk_beep() if we walked up to the toplevel and didn't find
an activatable widget, so to indicate that a programming error
caused us to ignore the key press).

the implementation details, like ::heirarchy_changed installation
of mnemonics, how to find out if a widget is mnemonic activatable
(CAN_FOCUS or class->activate_signal!=0), etc. i pretty much agree
with in owen's porposal.


but the abuse of accel groups for menmonic activation handling is
something i'm very uncomfortable with, for reasons i partially
outlined already:
- there's no good reason to use accel groups for mnemonics at all,
  the very reason they exist is handling unique, visibility-unaware
  key bindings which doesn't at all apply to mnemonics
- it makes the accel group code and API much more complex
- it makes negotiation and propagation of runtime changable
  accelerators much more complex (especially the negotiation code
  which decides about validity of runtime accelerator changes needs
  to have an idea that mnemonics are present and to be considered
  static)
- it requires unecessary API fragments like gtk_window_get_uline_accel_group()
  (or s/_uline_// in that prototype), and requires us, at least internally
  to pass accel groups around
- it's conceptually wrong ;)

---
ciaoTJ