Hi,

> > > > 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.
> > 
> > It does make a difference. We're currently selecting for FocusChange
> > events of the _window_, not the _frame_.
> > The problem is that although we correctly set the input focus to the
> > frame window if the window is shaded (window.c:2870), we never get a
> > FocusIn event in response and thus never will set the shaded window
> > active (where findTopLevelWindowAtDisplay will retrieve the main window
> > for the focussed frame). That's why we IMO need to select for
> > FocusChange events also for the frame window.
> 
> We do select for FocusChange events on the frame window at
> window.c:1974. The frame window will be added to the window list just as
> all other windows and we'll then select for FocusChange events, which is
> why we don't have to do this when creating the window.

Oh, yes, you are right ... I overlooked that addWindow is called for
frame windows as well. Sorry.

> > > 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.
> > 
> > I disagree. lastFoundWindow is used for both findWindowAtDisplay/Screen
> > and findTopLevelWindowAtDisplay/Screen. This has the side effect that
> > the behaviour of findTopLevelWindow depends on which window was
> > processed by findWindow before:
> > - Assumption: findWindowAtScreen is called for a frame window (which is
> > a valid scenario)
> 
> sure.
> 
> > - findWindowAtScreen returns the frame window CompWindow struct and sets
> > lastFoundWindow to that
> 
> yes.
> 
> > - if findTopLevelWindowAtScreen is called after that for the window the
> > frame window belongs to, the frame window is returned
> 
> No, how can that happen? lastFoundWindow->id will not match the id
> passed to findTopLevelWindowAtScreen so it will walk the window list
> until it finds the appropriate window with a matching id and return it.

My point is valid - my explanation is not (dunno what I thought when I
wrote that :-/ ). What I wanted to say:
- findWindowAtDisplay (some_frame_window) will return some_frame_window
- findTopLevelWindowAtDisplay (some_frame_window) will return
some_frame_window after that although it should return the main window
belonging to it.

But it doesn't matter, as ...

> I think I just found the bug that is causing the problem.
> findWindowAtScreen and findTopLevelWindowAtScreen are fine. However,
> findTopLevelWindowAtDisplay is broken. It shouldn't check
> lastFoundWindow. I pushed out changes that will fix this problem and
> clean up findTopLevelWindowAtScreen so it will be a bit easier to
> understand how lastFoundWindow is used.
> 
> Let me know if this fixed your problem.

... this change fixes the problem as findTopLevelWindowAtDisplay no
longer immediately returns lastFoundWindow. Thanks.

Regards,

Danny

_______________________________________________
compiz mailing list
compiz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/compiz

Reply via email to