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

[Leon Bottou]

On Wednesday 29 October 2003 12:26, Lubos Lunak wrote:
> On Tuesday 14 of October 2003 00:42, Leon Bottou wrote:
> > On Monday 13 October 2003 04:35 pm, Leon Bottou wrote:
> > > As it is now, the XAddToSaveSet is only performed
> > > when the embedding is achieved by calling embed().
> > > Not when using QXEmbed::embedClientIntoWindow().
> > > Nobody noticed...
>
>  Well, well ... it may work with Java and so on, but not only them use
> QXEmbed. The reason why systray doesn't work anymore are these recent
> QXEmbed changes.

I am running it daily and did not notice anything special in the system tray.
However, I did not experience a panel crash or restart. 
I understand that a panel crash or restart can indeed cause problems:
The systray applications terminate. Restarting the panel does not restart them.
Logging out might save a session without these applications. Etc.

> The first reason is backwards compatibility.

The problem with qxembed is that what is backward compatible 
for one application is a bug for another.  The core problem is to 
define what qxembed should do when the embedding applications terminates.  
Not only was this behavior poorly specified, but also was it inconsistent: 
using embed() or embedClientIntoWindow() gave different behaviors.

Let's first see what can be implemented:

When the embedding application crashes, 
 1- we can let the embedded application crash, or
 2- we can make it survive the crash using XAddToSaveSet.
     the embedded application then gets reparented and mapped
     into the root window.

When the embedding application terminates, 
 a- we can let the embedded application crash, 
 b- we can reparent it into the root window and map it,
 c- we can reparent it into the root window, map it, and send a WM_DELETE message,
 d- we can reparent it into the root window and hide it, 
 e- we can reparent it into the root window, hide it, and send a WM_DELETE message.

This is pretty trivial to implement, but this also makes a lot of options...

The current QXEmbed only provides 1-a and 1-e.
By explicitly using XAddToSaveSet one can do 2-b or 2-e instead.
For reference, the old QXEmbed was doing 2-b or 2-e when 
using embed() and 1-b or 1-e when using embedClientIntoWindow().

- The system tray needs 2-b (or maybe 2-d).
- The java embedding needs 1-a (or maybe 1-e).
- The nsplugin embedding needs 1-e.
- The control panel (administrator mode) needs 1-c or 1-e.
- The vimpart embedding should work with either 1-c or 1-e.

> You forgot one special case, called crash. If Kicker crashes, the trayed
> icons should survive (and let me repeat 'backwards compatiblity' here
> again).

Not only should the trayed application survive, but they should
be re-embedded into the panel when a new panel appears.
I do not know this code vey well, but this seems to be achieved
by setting the _KDE_NET_WM_SYSTEM_TRAY_WINDOW_FOR 
property.   The systemtray applet recognizes this property
and embeds the corresponding windows when it comes up.

Since the systemtray applety already does this X11 magic,
my first choice would be to have it call XAddToSaveSet as well.
But I can easily see that this is not a very strong argument.

> Therefore I propose that XAddToSaveSet() goes back in 
> (and in the right place this time).

Out of the five embedding cases I know (systray, java, nsplugin, 
control panel, vimpart), that would fix one and break four.
Maybe I am forgetting some other cases...

>  Maybe it would be better to extend setAutoDelete() to have more states,
> and the old bool one would be marked deprecated. As that one wasn't
> documented anyway, I don't think there would be a problem, as long as they
> do reasonably similar things as they used to (which is not the case with
> the present state).

This is also a worthy option!
Currently we need 1-e, 1-a, and 2-b.
It makes sense to have 1-e as the default since
most cases seems to work best with that option.

> Problem with systray #2 - not really QXEmbed's fault. The XAddToSaveSet()
> above is only for cases when Kicker restarts or crashes. The rest of the
> problem is by removing some of the embedding code from QXEmbed::embed(),
> and doing it after getting ReparentNotify. That was a good thing, the real
> problem is that KDE's mechanism for the sytray icon is just one big race
> condition, and worked until recently only because QXEmbed tried harder than
> KWin to get the window, and was lucky. I think the right solution would be
> using the XDG systray spec, but given the close 3.2 release, I'm not sure
> if this option is possible.

Maybe kwin could recognize the _KDE_NET_WM_SYSTEM_TRAY_WINDOW_FOR 
property and leave such windows alone instead of trying to manage them.

This also sounds like a generic problem for all uses of embed().
Currently the function embed() tries very hard to withdraw the 
window before reparenting it.  Until today I thought that this had 
the effect of pushing the window manager away...

Hope this helps.

- Leon Bottou