This has bugged me a few times as well.  My current guess at the fix without
thinking about it too much is to shuffle the parents of a transient in the MRU
list (in addition to the transient itself) whenever the transient is focused.

cc'ing myself because I have seen this a couple times today. It's not a big
deal, but it is surprising when it happens.

Okay, so I've thought about this quite a number of times, and I finally
discovered that a solution I rejected earlier (a bad assumption led me to
believe it would fail in some edge cases), will actually work and that I was
mistaken to reject it.  I think two of the solutions are way too complicated,
but I spent enough time thinking about the general ideas and approaches--which
may be useful for something else--that I don't want to lose them.  So I'm just
going to jot them down here in bugzilla.

Solution 1 (My original idea):
  Whenever a window is shuffled in order in the MRU list, shuffle all it's
  transients (and ancestors?) as well.

  Consequences:
  A window is always near its relatives in the alt-tab list.  This may be
  cool, but it would probably be considered a UI change which means that we
  couldn't fix it in time for Gnome 2.10.  Also, although I think this might
  be more intuitive for users, it's also possible this might be confusing
  instead; hard to tell without trying it.

  Lots of MRU shuffling occuring, which could be slow.  Currently, focusing a
  new window is an O(N) operation in regards to the MRU list because we just
  have to find the window in the list, and then move it to the front.  This
  proposal would change that to an O(MN) operation (this is because finding
  all the transients of a window is an O(M) operation, where M is the number
  of windows _on the entire display_ (not just the workspace), and for each
  transient we find we have to locate it in the MRU list and then move it).

  We would need to write special mru-shuffling functions.  There are lots
  of places we're shuffling the mru lists with various different circumstances
  and we'd have to think about it very carefully to get all the cases right.

  It seems kind of error prone; I'm worried about stuff like bug 122016.

Solution 2 (modification of solution 1 to fix speed issues):
  Change the mru_list from being a list of windows to being a list of a
  new-things-that-I-dont-have-a-name-for.  These 
  new-things-that-I-dont-have-a-name-for would be lists of windows--where each
  window would be an ancestor or transient of the others in the list.

  Consequences:
  All the consequences from solution 1, except the speed problems; though this
  would require an even bigger overhaul of the code.

  As a side effect, this could speed up the routines to find all transients or
  ancestors in window.c quite a bit as well.

  There could be problems with windows that are transient for more than one
  window, making these things be a tree of windows instead of a list.  That
  could be confusing.

Solution 3 (simple and elegant):
  s/focus_mru_window/focus_ancestor_or_mru_window/

  This had occurred to me before, but I previously thought it would fail
  because of stuff like bug 125492 making it so that the focus_mru_window
  routine wouldn't know the previous focus window.  However, that case only
  happens when both the window and it's parent disappear simultaneously (in
  which case we have nothing but the mru window to fall back to anyway).  I
  grepped through the code to check out all the cases, and it turns out that
  we know the previous focus window in all cases where we would want to focus
  an ancestor of it.  Sweet.

I'll attach a patch for solution 3 in a minute.

Created attachment 36697 [details] [review]
focus_mru_window -> focus_ancestor_or_mru_window

Includes an update for how-to-get-focus-right.txt.  :)

I am curious about the alt-tab order thing and whether that might be worth
trying to code for.  Comment/thoughts/suggestions appreciated.  Also, although I
like the current solution, I'm open to hearing others if anyone can think of any.

ping?  Ok to apply?

Comment on attachment 36697 [details] [review]
focus_mru_window -> focus_ancestor_or_mru_window

should put "static" on focus_ancestor_or_mru_window

Looks good.

committed (with the static fix)...

One quick question, though--should workspace.c:meta_workspace_queue_calc_showing
also be declared static?  It only appears in workspace.c...

Probably. My nitpick was simpler, just "anything exported should have the namespace"

Uhm... sorry to bug you by reopening the bug, but I can definately still
reproduce this with 2.9.13:


Just open gedit, use Ctrl+F to open the search dialog, switch to devhelp using
the window list, switch back to gedit using the window list. Result: the search
dialog is focused in front of metacity.


to show that the proper version is running:

paolo@murdock:~$ ps aux | grep metac
paolo    10571  0.2  1.3  12396  6980 ?        Ss   09:10   0:00
/usr/bin/metacity --sm-client-id=default1
paolo    10786  0.0  0.1   2944   780 pts/0    S+   09:14   0:00 grep metac
paolo@murdock:~$ /usr/bin/metacity --version
metacity 2.9.13
Copyright (C) 2001-2002 Havoc Pennington, Red Hat, Inc., and others
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.
paolo@murdock:~$

You mean in front of gedit, not in front of Metacity, correct?

Anyway, it's a totally different issue.  You haven't dimissed (i.e. closed or
minimized any windows) so it's not related to this bug at all.  The behavior
you're describing is due to libwnck, and unless the search dialog appears in the
tasklist, it's probably also wontfix.  Please open another bug against libwnck
and provide the output from xprop on the gedit search dialog in it.

> You mean in front of gedit, not in front of Metacity, correct?

Nope, I meant in front of devehelp.

the expected result would have been to have the dialog *and* the main gedit
window show up, with the dialog focused. When the dialog is dismessed the main
gedit window should get the focus.

So to clear up things:

A and B are applications, dA is a dialog of the app A.

steps:
1) open A
2) open B
3) switch back to A and open the dA dialog
4) switch to B
5) switch back to A

behavior when I first reported the bug:
when doing 5 both A and dA are shown, with focus on dA. closing dA causes B to
get focus

behaviour now:
when doing 5 *only* dA is shown while B is kept on the background. Closing dA
causes A to be shown and get the focus.

desired behavior:
when doing 5 both A and dA are shown, with focus on dA. closing dA causes A to
get focus


hope it is clearer now

First, I don't know if I believe your old claimed behavior:

  when doing 5 both A and dA are shown

I don't remember ever seeing that behavior; as far as I know, your current
claimed behavior of

  when doing 5 *only* dA is shown

has always been the way it worked, at least in my experience.  Maybe your usage
patterns exposes something else though, so could you run this with both the old
and new version and screenshot it to show me how you're getting something
different here?


Also, this bug is only about the 
  closing dA causes B to get focus
versus
  Closing dA causes A to be shown and get the focus.

so you'll need to file a separate bug about the other stuff against libwnck.

Yep, I understood that I have to file a separate bug... in fact I didn't reopen,
I didn't mean to be annoying :)

I'll try an old metacity tarball to see if the "old" behavior I described ever
existed or if I just dreamed about it...

You weren't annoying, I just discovered from your comment 12 that you meant
something slightly different than I had in mind after first reading comment 10.
 In fact, it reminded me of the wnck_window_activate_transient comment in
libwnck stating that perhaps part of that function should be implemented in the
WM.  So filing it against Metacity wouldn't be unreasonable, but I was just
restating that I think libwnck would be a bit better so we can think through
everything we need on that side and what we want to do.  I probably should have
added this comment when I did so, to make it clear that I considered the scope
of the bug to be a bit different due to your new comments and was just
clarifying that the larger scope bug would probably still be best filed against
libwnck.  Sorry about not making that clear.  :)

for the record, the bug I opened is http://bugzilla.gnome.org/show_bug.cgi?id=166826