[PATCH] Don't remap windows for SaveSetUnmap

[Owen Taylor]

On Mon, 2008-09-22 at 10:14 -0700, Keith Packard wrote:
> On Mon, 2008-09-22 at 11:05 -0400, Adam Jackson wrote:
> > On Fri, 2008-09-19 at 17:27 -0400, Owen Taylor wrote:
> > > I found that 
> > > 
> > >    XFixesChangeSaveSet (..., SaveSetRoot, SaveSetUnmap);
> > > 
> > > isn't working as expected ... when the client terminates, the save-set
> > > window will be reparented to the root window, but the window manager
> > > will get a MapRequest and map the window. Here's a fix to unmap the
> > > window before calling ReparentWindow().
> > 
> > Looks right to me.  Applied, thanks!
> 
> Hmm. This doesn't quite match the intent of the XFixes change.
> 
> In the core protocol, when an unmapped window is reparented due to save
> set processing, it is forcibly mapped. This makes sure that iconified
> windows reappear when the window manager crashes.
> 
> The intent of the XFixes change was to allow embedded unmapped windows
> to be left unmapped so that they wouldn't suddenly appear at the top
> level when the application went away.

I think thing got a little confused along the way; the intent was
definitely to unmap in my original proposal:

 http://www.mail-archive.com/xpert@xfree86.org/msg05662.html

Suddenly appearing at the toplevel is just as much of a problem if the
embedded window was originally mapped. It has "popped out" of its
context, and you have a free floating notification area icon or
whatever.

(The problem wasn't very noticeable because most existing XEMBED clients
destroy themselves immediately after being unembedded, so you get only
a brief flash of the popped out icon. I noticed the problem mostly
because I tried removing that destroy from the GTK+ status tray icon,
thinking that it was unnecessary.)

> What you've done here is to make even mapped windows get unmapped when
> the application terminates without destroying the embedded windows. This
> doesn't match the XFixes description of the request, although does match
> the (shorter) specification of the request.
> 
> I think the change is a good and reasonable one though; it does mean
> that the embedded window will suddenly become unmapped when the
> embedding application terminates; this change assumes that the embedded
> application will be OK with this.

To quote my original proposal:

  ... (Note that unmapping and reparented back to the root window not result 
  in a "lost" window, since this is the normal termination of the XEmbed 
  protocol and clients are required to watch for it.)

> Can we get a change to fixesproto.txt which describes the new behaviour
> so that the documentation matches the implementation? I think the X
> server might also benefit from renaming the 'remap' field to something
> more descriptive of the new semantics, perhaps just 'map'?

Well, at least I understand now why you called it 'remap' :-) Patches
attached.

- Owen


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Clarify-the-meaning-of-map-Unmap-for-ChangeSaveSet.patch
Type: application/mbox
Size: 1058 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080922/cb7659b6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Change-remap-to-map-in-saveset-functions-macros.patch
Type: application/mbox
Size: 4330 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080922/cb7659b6/attachment-0001.bin>