On Mon, 2007-08-06 at 10:15 +0200, Danny Baumann wrote:> >> today, I noticed a problem in focus handling for shaded windows
which is
> >> pretty easy to reproduce:
> >>
> >> - Disable "Click to focus"
> >> - Shade a window
> >> - Hover another window
> >> - Hover back to the window frame of the shaded window
> >> - Press Ctrl+Alt+S
> >>
> >> Expected behaviour would be that the shaded window is unshaded.
What
> >> happens is that the last active window is shaded.
> >>
> >> I investigated this a bit and found the cause for this behaviour,
but
> >> wasn't able to work out a proper fix for that:
> >> - The function shade() uses d->activeWindow to determine the
window to
> >> (un)shade via the "window" option.
> >> - d->activeWindow is set from the FocusIn event handler, but
only if
> >> w->managed is set for the window which is hovered or whose
frame is
> >> hovered
> >> - As w->managed is unset in the UnmapNotify handler (and so
after a
> >> window is shaded), hovering a shaded window frame does not set
> >> d->activeWindow to that window.
> >>
> >> Any idea regarding that one?
>
> > w->managed should not be unset when getting an UnmapNotify event
cause
> > by shading. The window is still managed by compiz while being shaded.
> > w->pendingUnmaps should be greater than 1 when a window is shaded
but I
> > guess that's broken somehow...
>
> You are right. This also works as intended. I walked a false path there,
sorry.
> The real problem is that we don't select for FocusChange events for
frame windows and thus never get FocusIn events for them.
> Correcting this revealed another problem: The lastFoundWindow variable was
(IMO incorrectly) sometimes also set to frame windows.
> The attached patch seems to do the trick - any comments?
I applied the FocusChange change just before I realized that it
shouldn't be needed as we'll select for focus change events when the
window gets added using the addWindow function and this change shouldn't
make a difference.
lastFoundWindow is just an optimization to avoid walking the list of
windows when a window is looked up multiple times.
The way lastFoundWindow is updated right now is more correct than what
your patch will do.
-David