I started going over a patch from Jonathan and Tim this morning to fix some problems with focusing, and I realized, that the patch, while it was fixing a few bugs, was basically fixing them in the wrong way. In order to figure out what was wrong, I had to study the focus code and write up some documentation for myself. So here, is some documenation on how focusing works - mostly from the perspective of somebody writing a new container widget with a custom ::focus method, along with a description of some problems in focusing currently, and my proposed solutions. Focus Navigation in GTK+ ======================== The focus widget, in GTK+ is the widget to which keystrokes are directed. Each toplevel window may have a focus widget within it, and when that toplevel window is focused by the window manager, all keystrokes are sent to the focus widget for that toplevel window. Like most other toolkits, GTK+ allows moving the focus widget with the keyboard. Tab and Shift-Tab are used to move the focus forward and backwards through all focusable widgets. However, GTK+ also allows focus navigation using the arrow keys. The arrow keys move the focus around the window directionally, instead of forwards and backwards through the focus chain. The other way that GTK+ differs from other toolkits is that the focus chain (and the arrow key behavior) is not a static object. It is, instead determined dynamically by making callbacks on containers that then decide the ordering of their children in the focus chain. There are essentially three places where state is stored in GTK+ that relates to focusing: - the HAS_FOCUS flag on each widget stores whether the widget is the current focus - that is, not only that it is the focus widget for its toplevel window, but also that the toplevel widget has the focus from the window manager. gtk_widget_focus_in() and gtk_widget_focus_out() give notification of the widget having HAS_FOCUS set and unset. - Each toplevel window stores a pointer the current focus widget for that window, if any, in window->focus_window. - If the focus widget is a descendent of a container, than container->focus_child hold the immediate child in which the focus widget is to be found. Note that, at least in the static case, the focus_child pointers contain no information beyond that provided by window->focus_window; they are just an optimization. The actual keyboard navigation is done by gtk_container_focus(), a virtual function. When the toplevel window receives a key press that it interprets as moving the focus, it converts it to one value of an enumeration: GTK_DIR_FORWARDS GTK_DIR_BACKWARDS GTK_UP GTK_DOWN GTK_LEFT GTK_RIGHT And then calls gtk_container_focus(window, direction). These calls to gtk_container_focus() propagate down recursively and move the focus widget. The basic plan of an implementation of gtk_container_focus() is: 1) it identifies the current focused location within the container. This might be: - a child container that has the focused widget for the toplevel widget in it. (container->focus_child == child) - a non-container child that has the focus. - the container itself if it is the focused widget for its toplevel. Some widgets, for instance have multiple conceptual locations within the container itself - for instance, the GtkCList has a focus location for each row. 2) it identifies a chain of focusable locations within the widget; this contents and ordering of the chain may depend on the direction and the current focused location. The only constraint is that it produces a reasonable result for the user, and if there is a current focused location, it must be in the chain. 3) it iterates through the chain starting at the location after current focused widget, or the first widget in the chain, if there is no focused widget. For each location it: - If location is the widget itself, it calls gtk_widget_grab_focus() on the widget if it isn't already the focus widget and returns TRUE. [ Note that this doesn't work currently - currently you always have to call grab_focus() - see below. ] - If the location is a child container, it calls gtk_container_focus (child, dir) on the child, and if that returns TRUE, returns TRUE. - If the location is a non-container child that has CAN_FOCUS set, it calls gtk_widget_grab_focus() on the child and returns TRUE. 4) If the iteration in 3) did not have a result, return FALSE Note that the "chain" above can be purely conceptual - there is no need to actually create such a list, though a robust implementation will deal with the fact that calling gtk_container_focus() on the child could result in the widget heirarchy being changed. [ Should we simply disallow this as pathalogical? ] To put it a little less abstractly, ::focus moves the focus between focusable locations in the widget, and if that motion takes the focus out of the container, it returns FALSE. The default gtk_container_focus() implementation uses as its chain: a) if the container is CAN_FOCUS, only the container itself b) if the container is not CAN_FOCUS, the children of the container, sorted geometrically: DIR_RIGHT/DIR_FORWARD: in rows, left->right top->bottom DIR_FORWARD/DIR_BACKWARDS: in rows, right->left bottom->top DIR_DOWN: in columns, top->bottom, left->right DIR_UP: in columns, bottom->top, right->left Current Problems and Proposed solutions ======================================= 1) One problem that exists throughout a bunch of the GTK+ focusing code is that it is using GTK_WIDGET_HAS_FOCUS() to determine if a widget is the focused widget within its toplevel. This is wrong, and GTK_WIDGET_HAS_FOCUS() should only be used to determine if the focus indicator should be drawn for a widget. I'd like to add a gtk_widget_is_focus() call that checks to see if the widget is the focus widget for its toplevel window. And then we need to go through and systematically evaluate all calls to GTK_WIDGET_HAS_FOCUS. 2) The biggest problem right now is that the responsibility of maintaining the focus chain is not kept completely within the core parts of GTK+, but also is partially the responsibility of the implementation of GtkContainer::focus. In particular, there are two constraints on the ::focus implementation. - If container->focus_child is currently set for the container, container->focus_child must be set to NULL before calling gtk_widget_grab_focus() or gtk_container_focus() again. - The implementation must do one of: - Call gtk_widget_grab_focus() on a widget - Receive a TRUE return when it calls gtk_container_focus() on a child. - Return FALSE So, basically, the focus chain is partially destroyed during ::focus propagation and then recreated when gtk_widget_grab_focus() is called. I believe simply forbidding containers to call gtk_container_set_focus_child() themselves, and doing all updating of the focus chain if in gtk_grab_widget_focus() works fine. There is no reason to expose partially invalid focus chains. 3) Another problem is that currently the HAS_FOCUS flag is maintained not by GTK+, but by the widget-specific implementations of gtk_widget_focus_in() and gtk_widget_focus_out(). For instance, gtk_button_focus_in() looks like: static gint gtk_button_focus_in (GtkWidget *widget, GdkEventFocus *event) { g_return_val_if_fail (widget != NULL, FALSE); g_return_val_if_fail (GTK_IS_BUTTON (widget), FALSE); g_return_val_if_fail (event != NULL, FALSE); GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS); gtk_widget_draw_focus (widget); return FALSE; } This is just silly. GTK+ should take care of updating the flag itself. We can change GTK+ to do that without breaking backwards compatibility without any problems. 4) There are two focus states for GtkWindow - focus on a child, and no focus. This ends up with the fact that for every toplevel window, the focus chain has a NULL state with the focus nowhere. This is a bug - it should be impossible to Tab the focus to this location. 5) Hitting the down arrow key within a hbox moves right, and so forth. This is quite confusing to users. Moving down from an hbox should move off the end of the hbox and to the next widget downwards in the parent container. Fixing this basically means revising the default container focus implementation to remove the wrapping around when you get to one side of the container. 6) Finally, overriding the focus chain from an application is crazily hard. Even if focus chain maintainence is fixed, it still is a lot of nonobvious code to override ::focus. So, I want to add the ability to store a custom focus chain on a container. /* Set the focus chain to the given list of widgets. A copy * of the list is made. The widgets must be descendents * but not necessarily immediate children of the container * * If there are focusable descendents of the container * which are not in the focus chain or descendents of * widgets in the focus chain, then focus behavior * will be unpredictable. */ gtk_container_focus_chain_set (GtkContainer *container, GList *widgets); /* Restore default focus chain handling */ gtk_container_focus_chain_unset (GtkContainer *container); (Note that unset and empty are different.) If focus chain is set, then ::focus for DIR_BACKWARDS/FORWARDS moves among the widgets in the focus chain, not the children of the container, with the ordering being given by the order in the list. Handling of DIR_UP/DOWN/LEFT/RIGHT is done geometrically, as currently, but again uses the widgets in the focus chain, not the child widgets. Jonathan and Tim's patch ======================== [ Here mostly to get some feedback from Tim. ] The patch I was evaluating (attached to this message for the reference enthusiastic readers) had essentially three components: - It fixed a problem where the implementation of the defualt behavior for gtk_container_focus() was split between the default handler and the focus() implementation for the parent, which made it impossible to implement non-default container focus behaviors properly. - It fixed a bunch of problems with GtkNotebook and GtkCList. - It fiddled around some with rebuilding the focus chain. @@ -2634,11 +2634,20 @@ { widget = GTK_WINDOW (toplevel)->focus_widget; - if (widget == focus_widget) + if (widget == focus_widget && + GTK_CONTAINER (toplevel)->focus_child && + GTK_CONTAINER (focus_widget->parent)->focus_child == focus_widget) { - /* We call gtk_window_set_focus() here so that the - * toplevel window can request the focus if necessary. - * This is needed when the toplevel is a GtkPlug + /* if focus should be grabbed for a widget that already + * has the focus, with the GtkContainer.focus_child chain + * being intact, we call gtk_window_set_focus() here so that + * the toplevel window can request the focus if necessary, + * this is needed when the toplevel is a GtkPlug. + * if focus should be grabbed for a widget that already + * has the focus, and the GtkContainer.focus_child chain + * being broken, we're probably in GtkContainer::focus + * propagation, and a widget wants to keep its focus, but * has to rebuild the chain. The idea of the original code is that if we have a GtkPlug (a toplevel inside a toplevel, essentially), then clicking on plug->focus_widget might need to trigger some action on the GtkPlug that requests the focus from the toplevel container. I think the idea of the change is to workaround the fact that when moving from one logical focused location to another within the container, you need to call gtk_widget_grab_focus(container) to rebuild the focus chain - apparently the call to gtk_window_set_focus() was confusing things somehow, though I don't see how. The first two changes are still good, the third change I don't think is necessary, especially if we fix things so that gtk_widget_grab_focus() is the only thing that ever changes the focus chain. Regards, Owen
Attachment:
patch
Description: Binary data