Embedding multible java applets and focus problems

[Leon Bottou]

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 );
  }

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.

----------------

Regarding Koos Vriezens patches proposed in 
<http://lists.kde.org/?l=kwin&m=106598534306845&w=2>.

I would vote for the second option (let ReparentNotify do the job).
In fact the embedding process has two steps:
- reparenting.
- once the reparenting is done, setting up the embedded window

Right now function embed() tries to do both. 
I believe it should only do the reparenting.
That would make it more similar to QXEmbed::embedClientIntoWindow
which is the other way to embed a window.
All the part of embed() that deals with setting up the embedded 
window should then be moved into the part of the code that deals
with X11 ReparentNotify messages.

I am sorry for giving little more than suggestions today.
I have not enough spare time at the moment.
At least I am trying to reply my email :-(.

--------------

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.

Best would be to radically simplify QXEmbed by moving
some of its chores into the Qt proper (instead of fighting
to make Qt do the right thing)

Hope this helps.

- L.