I guess the difference is the submitter wants us to focus the default
window both on workspace switch and on release of the modifier.

Maybe a better solution would be to honor EnterNotify/LeaveNotify
events for windows while we have a keyboard grab.

Focusing windows while doing mouse movement during workspace switching
would be better than what I proposed, but what I proposed was easy,
and solved the problem of focus being wrong when focus mattered.

(This bug isn't bothering me as much lately, since I'm not currently
using a Kinesis keyboard, which makes it really easy to do workspace
switching (either horizontal or vertical, but not a combination) with
one hand (holding down both Ctrl and Alt with one thumb).  But I
suspect others with similar setups would notice it.)

I've spent some time looking at this, and I think I can clear up some of the
confusion, but I have some questions of my own that I hope someone can answer.

First, David's patch adds a comment:
  /* meta_workspace_activate will no-op if we're already on the
  workspace in question, but we need to focus */
I believe that comment should have an s/if/since/.  The switch_to_workspace()
just before that comment (which has since become a meta_workspace_activate()) is
not needed.  The actual workspace switch happens when various keybindings are
pressed (elsewhere in that function), not here when the modifiers are released.
 Perhaps there's an edge cases where both meta_workspace_activate's are
necessary, but I'm having a hard time thinking of one and it really seems odd to
have both.

Second, the real line that David adds is:
   meta_screen_focus_default_window (target_workspace->screen, NULL);
This function no longer exists, but even if it did it should not be used here. 
The reason is that it would change the behavior for click-to-focus too so that
moving the mouse (without clicking) could cause a different window to be
focused, i.e. click-to-focus would behave like mouse focus in this case.  Using
meta_workspace_focus_default_window instead would work for all focus modes
(though in many cases it would result in the same window being focused twice). 
Note that I just recently mentioned this same fix in bug 124798, since I wasn't
aware of David's patch.

Third, I've tried to solve things the way Rob mentioned, because I think that
would be a much cleaner fix.  I think there's two parts to making such a fix,
because there appears to be a synchronous grab or something causing EnterNotify
and LeaveNotify events to be queued until after I release Ctrl & Alt.  So, we
want EnterNotify & LeaveNotify events to be generated from mouse movement while
the workspace switcher popup is showing (as far as I can tell, we don't ever get
these from the server at all), and we also want them to not be queued.  After
twiddling for a number of hours on this, I still can't track down how to get
either of these two parts to work.  Rob, Havoc: Any hints on what I should look
at or how I might get this to work?

In principle, when the pointer is ungrabbed we should get an EnterNotify which
should focus a window. Maybe we are just overriding that with the MRU list, for
example.

I have a patch that adds "strict pointer mode" on top of sloppy/mouse focus
modes, where all keyboard heuristics get skipped and keynav basically becomes
hellish. But it does "fix" this. ;-) This patch is from a patch set for a Red
Hat customer that I need to merge during the GNOME 2.10 cycle.

Actually, I think the EnterNotify is being overridden by the METACITY_SENTINEL
(see bug 124798, and the reference to bug 110970).  But even making that
EnterNotify not be overridden would not "fix" this, because the focusing of the
window would be delayed until the ungrab happens.  (If I merely want a delayed
focusing of the window--which, honestly, may be good enough--I can use the
one-line patch in bug 124798; it works perfectly well).

Your statement that keynav becomes nasty with that patch scares me; I use both
mouse focus and keynav a lot and I really believe that most of the current
problems can be fixed without regressing anything or requiring the user to
choose between those two.  I'd be very interested in taking a look at your patch
and the list of requirements it is meant to satisfy, to see if I can get all the
requirements working without regressing anything else.

Okay, I have a fix.  Basically, here's the thought process: The user is using
keynav (meaning that we tend to ignore the mouse, but we're merely attempting to
get one of those "mixing of navigation methods" cases right).  The real focus
window is the workspace switcher popup while it remains up.  When the user
releases Ctrl and Alt, the user then wants to interact with the default window
on that workspace and we should focus it.  I'll attach a patch in a minute that
does so (it's quite similar to David's).

Created attachment 31495 [details] [review]
Focus the default window after switching workspaces upon key release

Comment on attachment 31495 [details] [review]
Focus the default window after switching workspaces upon key release

The g_assert scares me a bit; won't we process any events we get between key
press and this code? Surely one of those events could be e.g. a client message
from the workspace switcher, or devil's pie, for example, changing the space.

I might just change the code to use screen->active_workspace rather than
target_workspace, with a comment that we expect but can't guarantee they are
the same?

Maybe I'm just paranoid

Created attachment 31497 [details] [review]
Replace assert with a warning

I'm going to throw this on the Gnome 2.8.x milestone; I'm hoping it's reasonable
to try to get all the remaining focus bugs (well, other than the focus stealing
stuff) fixed for the Gnome 2.8.1 release.

ping?

Comment on attachment 31497 [details] [review]
Replace assert with a warning

Okie, thanks.