Fcosuing (was Re: gtk_grab_add() and key press events)

[Owen Taylor <otaylor redhat com>]

Federico Mena Quintero <federico ximian com> writes:

> I have cooked up a patch heavily based on your excellent description
> of how GTK+ focusing works and on Jonathan's and Tim's patch.  The
> whole patch does this:
> 
> 	- Applies basically everything that JRB and Tim did.
> 
> 	- Fixes the nasty hit-Tab-on-the-last-widget-and-everything-
>           loses-focus situation.
> 
> 	- Makes arrow keys do more reasonable things when used in box
>           containers.
> 
> 	- Makes GtkWindow and other bits of the code not ignore the
>           return value from gtk_widget_activate().
> 
> 	- Misc. stuff I can't remember but is in the ChangeLog
>           anyways.
> 
> The patch is here:
> 
> 	http://primates.ximian.com/~federico/misc/gtk+-focus.patch
> 
> I don't think the ChangeLog is there, so I'll post it at the end of
> this mail.
> 
> I've been using it and it makes dialog boxes and windows *much* more
> pleasant to use.  I would like to think it is a good candidate for
> inclusion in both the stable and the development branches, so it would
                                       ^^^^^^^^^^^

You do release that I committed a patch to HEAD a couple of months
ago that included Jonathan and Tim's patch plus a _lot_ of other
focusing changes? (Look at my commit of October 25)

> be great if people could take a look.

> 2001-01-20  Federico Mena Quintero  <federico ximian com>
> 
> 	* gtk/gtkhbox.c (gtk_hbox_focus): New focus handler for horizontal
> 	boxes.
> 	(focus_along_children): Changes the focus for tab/left/right
> 	directions by walking the children list appropriately.
> 	(focus_outside): Changes the focus for up/down directions by a)
> 	giving up the focus in case we already have a (non-container)
> 	focus child; this makes pressing up/down in hboxes not focus
> 	another widget in the box; or b) by finding the child that is the
> 	most horizontally overlapping with the previously focused widget,
> 	which produces more intuitive results than the old method.
> 
> 	* gtk/gtkvbox.c: Analogously.
> 
> 	* gtk/gtkhbbox.c: Analogously.
> 
> 	* gtk/gtkvbbox.c: Analogously.

It's very ugly to repeat this three times when it could be done
once in gtkcontainer.c, right?

It sounds (and looks from the patch, without trying it out) that this
is basically the behavior that CVS head has, but the CVS head code
does it once in gtkcontainer.c.

> 2001-01-19  Federico Mena Quintero  <federico ximian com>
> 
> 	Fixing the basic focus mechanism.
> 
> 	* gtk/gtkcontainer.c (gtk_container_focus_up_down): Do not
> 	generate a list with NULL children by removing the invalid ones
> 	from it.  This means we must take a reference to the pointer to
> 	the first list element, not just a pointer to the first element.
> 	(gtk_container_focus_left_right): Likewise.

Heh, HEAD has the same change. But, hmmm, I did that because I 
was making other changes to this mechanism in gtkcontainer.c -
and I dont' actually see where it matters with your code.

> 	(gtk_container_focus_tab): Likewise.
> 	(gtk_container_focus_move): We do not handle NULL children
> 	anymore.
> 	(gtk_container_focus_move): Do not skip containers without
> 	HAS_FOCUS.
> 	(gtk_container_focus): Initialize return_val to FALSE for if
> 	anyone screws up in the focus handler.


> 	* gtk/gtkwindow.c (gtk_window_key_press_event): Whenever we call
> 	gtk_widget_activate(), use its return value as the criterion for
> 	having handled the event.
> 	(gtk_window_activate_focus): Likewise.
> 	(gtk_window_activate_default): Likewise.

This makes no difference right? Since the return value from 
gtk_window_key_press is ignored...

> 	(gtk_window_key_press_event): If our initial attempt at cycling
> 	the focus failed, restart from the beginning.

I think it is cleaner to do this in gtk_window_focus(). Which HEAD
does.
 
> 	* gtk/gtkentry.c (gtk_entry_key_press): Likewise.

This makes absolutely no difference, right, unless, I guess, you
derived from entry and removed the activate signal.

> 	* gtk/gtkplug.c (gtk_plug_key_press_event): Likewise.
> 	(gtk_plug_key_press_event): When we initially send the event to
> 	the focus widget, see if it is sensitive and that it is not the
> 	plug itself.  After that, when handling the spacebar, see if the
> 	focus widget is sensitive before we activate it.  All of this
> 	makes the behavior consistent with that of GtkWindow.

Fixed in my local tree, but then again, I've virtually rewritten
plug/socket there.


Hmmm, well, the main problem here is the divergence from HEAD.
I'm not very comfortable checking a 2000 line patch into the
stable branch that is completely different from a 3000 (or whatever)
line patch that went into HEAD.

Also, I'm not really convinced that the change to the behavior
of gtk_container_focus() can go into gtk-1-2. 

The question is, if someone had a cut-and-paste of, say, GtkCList
in their code, would it break it?

But I'm not awake enough to properly judge that. (And gtkclist.c /
gtknotebook.c were doing some quare stuff.)


Clearly, a few of the changes, like the activate ones, are pretty
harmless, and certainly should go into HEAD, and possibly into
stable if they make any actual difference to the operation of
GTK+.

Of course, if people want to test out Federico's patch and see
if it breaks things, that would be great, since it would give
information as to the safety of either applying this patch
or backmerging from HEAD.

Regards,
                                        Owen