Created attachment 30293 [details] [review]
Focus the default window when unshowing the desktop via the panel applet

Probably should just call focus_default_window from unshow_desktop rather than
go through the code and add that call right after every call to unshow_desktop.

That's a three-line patch, but I think you can handle it.  You're a smart guy :-)

I had considered that.  There are exactly three calls to unshow_desktop, the one
that isn't mentioned above is window.c's maybe_leave_show_desktop_mode().  I
tried following the cases through there and did notice that, for example,
meta_window_activate (which calls meta_screen_unshow_desktop) would also call
meta_window_focus on the focused window.  So we'd at least be wasting effort by
focusing the same window twice.  It seemed like there might be something more
convoluted there too, but I haven't followed all the cases through completely. 
And I didn't really want to check out all the cases completely because, contrary
to popular opinion, I'm a dumb guy and following all the cases might take me a
long time.  Since that might be a lame excuse, my official one is that it's
wasteful of resources.  :-)

But if focusing the same window twice isn't wasteful enough to care about, I can
try looking into it again.

Okay, I've thought it through some more.  I believe allowing unshow_desktop to
not focus a window may be what we want to do.  Although there are the two
obvious cases when unshow_desktop is used by itself (and thus the default focus
window is wanted), there are other cases (and more could arise in the future)
where unshow_desktop is used as part of a larger operation where a different
focus window could be wanted than the default one.

For example, launching a window or activating one will cause the "show desktop"
mode to be exited.  It used to be the case that when the "show desktop" mode was
exited in this way, all windows would be unminimized.  (And it's still not an
unreasonable choice of behavior, but someone decided that keeping them minimized
made more sense for Metacity.)  In such a case, when the windows were all
unminimized, unshow_desktop would cause the default window to be focused --and
the default one may not be the launched or activated one.  Yet the launched or
activated one is the one that should be focused.  This would result in
flickering, or perhaps even the wrong window having focus.

This is only one case and it can't currently happen--but there's always the
possibility that we would want to revert the unminimize choice which would make
it possible.  And, of course, other cases similar to this could potentially
arise in the future.

Of course, as you pointed out, Rob, it's probably also not wise to try to
remember to call focus_default_window in all the cases where we need to. 
Perhaps we can add a boolean parameter to unshow_desktop, and have
unshow_desktop call focus_default_window if it's true.  That way the functions
that don't want unshow_desktop to focus a window don't have to have that happen,
but it can be taken care of in the other cases where we clearly do want it.

Does this sound reasonable?  Am I making any sense?  Did I just get confused and
I'm missing something that makes me misunderstand what's going on?  (It's
entirely possible; for some reason I was getting a little perplexed trying to
track down a lot of different what-if's)

Comment on attachment 30293 [details] [review]
Focus the default window when unshowing the desktop via the panel applet

Um, yeah.  Actually I didn't put much that much thought into it.  I had just
failed to notice that there was a case where we weren't calling
focus_default_window.  But, as you point out, we're not and shouldn't be in the
window activation case, so I'd say the original patch is good.

committed.