[Kwin] Re: Embedding multible java applets and focus problems

[Lubos Lunak]

On Monday 13 of October 2003 17:34, Leon Bottou wrote:
> On Monday 13 October 2003 09:55 am, Lubos Lunak wrote:
> > On Sunday 12 of October 2003 16:20, Koos Vriezen wrote:
> > > if you ALT-TAB to another window, so that the java text box hides
> > > behind the new active window, the konqueror taskbar item starts to
> > > blink a few time.
> >
> > ...
> > http://webcvs.kde.org/cgi-bin/cvsweb.cgi/kdelibs/kdeui/
> > qxembed.cpp.diff?r1=1.34&r2=1.35&f=h - this change is the reason for the
> > blinking. I'm not very familiar with QXEmbed internals, but I find the
> > log message 'Smallish improvement of focus signaling.' both useless and
> > not true. Definitely at least setting focus back after geting focus out
> > event is wrong, and the other changes are suspicious too. Hard to say for
> > sure without really knowing what the code is supposed to do - it seems to
> > be a common KDE standard not to bother with explanatory comments, but X11
> > related code is way too complicated to be understood just from reading
> > it.
>
> This is indeed a smallish improvement to the focus signaling.
>
> Explanation: QXEmbed maintains this focusProxy window which is supposed to
> have the X11 focus whenever the (non embedded) application window is
> active. This is not the way Qt normally works.  Until Qt changes in that
> respect, QXEmbed must perform black magic to achieve this.  The bad news is
> that this black magic depends on minute details of Qt's focus signalling...
>
> One of the most ugly aspect is that each QXEmbed instance has its own
> focusProxy window.  If you embed two windows, you get two focusProxy
> windows. We could think the QXEmbed instances should cooperate and select
> which one is in charge of maintaining the focusProxy window.  But what
> happens when the selected QXEmbed gets destroyed?
>
> In the current state of the code, both QXEmbed instances fight each other
> to set the X11 focus to its own focusProxy window.   To minimize the
> effects of this XSetInputFocus storm, we must at least make sure that
> (a) it terminates at some point, and (b) that it remains limited to
> children of a same toplevel window (in order to avoid
> further interaction with the window manager.)
>
> This patch Lubos discusses is indeed a smallish improvement to the focus
> signaling. Under certain conditions, the X11 focus might be reset to the
> toplevel window of an already active application.  Since the window is
> already active, there is no WindowActivate event.  The QXEmbed instance
> does not
> get a chance to set the focusProxy right.  Then everything gets messy.
>
> My solution was to reset the X11 focus to the focusProxy whenever
> a QXEmbed instance gains or loose the Qt focus.  This increases
> the risk of XSetInputFocus storm, but I thought it would be
> limited to children of the active window.
>
> The blink comes because I was wrong on this one.
> Qt's  focusInEvent and focusOutEvent do not only reflect
> the status of the Qt focus.  They also get sent when the toplevel
> window activation changes (even though the Qt focus does not change!)
>
> I suggest the following patch to correct this.
> ( but I am not set up to test it properly. )
>
> *** qxembed.cpp.~1.44.~	2003-09-25 14:16:54.000000000 -0400
> --- qxembed.cpp	2003-10-13 10:33:32.000000000 -0400
> ***************
> *** 678,683 ****
> --- 678,685 ----
>           send_xembed_message( window, XEMBED_FOCUS_OUT );
>       }
>       if ( !((QPublicWidget*) topLevelWidget())->topData()->embedded )
> +       if (QFocusEvent::reason != QFocusEvent::ActiveWindow &&
> +           QFocusEvent::reason != QFocusEvent::Popup )
>           XSetInputFocus( qt_xdisplay(), d->focusProxy->winId(),
>                           RevertToParent, qt_x_time );
>   }

 I'd rather go with

+ if( topLevelWidget()->isActive())

 as that's safer.

>
> Of course, the correct fix would be to have Qt handle the focusProxy window
> in all cases. It should also handle the embedded window side of the XEMBED
> protocol  as soon as embedding is detected.  Overall this sucks.

 If that's the correct fix, why not to do it that way then? Qt seems to have 
some support for being embedded already anyway. The focus proxy handling 
could be quite simple to solve properly, using a different X11 focus model, I 
could do that. I'm just not sure about the timeframe, now that we're close to 
3.2, and Qt X11 guys have few tough patches in their queue from me already.

[snip]
> Finally a word about CVS log messages ;-).
> A quick look at the cvs log indicates that my comment is not the shortest
> one there. But Lubos is right when he says that QXEmbed is a very
> complicated code. It deserves longer comments, either in the cvs logs or in
> the code.

 I actually meant comments in code, I just cited the cvs log message because 
in the commit there way no comment at all. And you have to admit setting 
focus back on focus out events looks weird. X11 related code can be a 
complicated thing even when commented.

-- 
Lubos Lunak
KDE developer
---------------------------------------------------------------------
SuSE CR, s.r.o.  e-mail: l.lunak at suse.cz , l.lunak at kde.org
Drahobejlova 27  tel: +420 2 9654 2373
190 00 Praha 9   fax: +420 2 9654 2374
Czech Republic   http://www.suse.cz/