Subject: StaysOnTop vs. override_redirect

From: Matthias Clasen <[email-masked]>
Date: Fri, 1 Jan 1999 23:28:10 +0100

Hi,

some days ago I have taken a look at the recent version of fvwm
(CVS from 30/12/1998). I noticed that it still has problems with 
the interaction between StaysOnTop and override_redirect windows
(that one is even in your FAQ, Q20).

Examples for the problems are eg FvwmPager when moving a window 
with button 2: The ballon window is unmapped during the drag, and 
when it is mapped again it is obscured by the pager window. 
Another example is Motif menus: Try Style Netscape StaysOnTop and 
open a menu.

I think I have found a solution to this problem, see my patch below.
Basically the idea is to raise windows above all *managed* windows,
not above all windows. I haven't had any problems with this patch
so far, but you may want to check that it doesn't have any bad
consequences (I haven't looked up all places where XRaiseWindow
or RaiseWindow are called).

Matthias

-- 
Matthias Clasen, 
Tel. 0761/203-5606
Email: [email-masked]
Mathematisches Institut, Albert-Ludwigs-Universitaet Freiburg

--- ChangeLog.orig	 Fri Jan  1 23:00:49 1999
+++ ChangeLog		 Fri Jan  1 23:05:55 1999
@@ -1,3 +1,10 @@
+1999-01-01  Matthias Clasen  <[email-masked]>
+
+	* fvwm/misc.c (RaiseWindow): fixed problem with interaction of
+	StaysOnTop and override_redirect windows. Keep StaysOnTop and
+	transient windows on top of the managed windows, but do not 
+	raise them above override_redirect windows. 
+	      
 1998-12-30  Dominik Vogt  <[email-masked]>
 
	* fvwm/menus.c (MakeMenu): fixed font-change bug with menus: If a menu
--- fvwm/misc.c.orig   Fri Jan  1 21:50:53 1999
+++ fvwm/misc.c	       Fri Jan  1 22:58:33 1999
@@ -961,7 +961,16 @@
 
   if(i > 0)
     {
-      XRaiseWindow(dpy,wins[0]);
+      /* [email-masked] - 01/01/1999 - 
+       simply calling XRaiseWindow(dpy,wins[0]); here will
+	 put StaysOnTop windows over override_redirect windows
+	  like FvwmPager ballon_win or Motif menus. 
+	   Instead raise wins[0] only above the topmost window 
+         which is managed by us. */
+      XWindowChanges changes;
+      changes.sibling = Scr.FvwmRoot.stack_prev->stack_next->frame;
+      changes.stack_mode = Above;
+      XConfigureWindow(dpy, wins[0], (CWSibling|CWStackMode), &changes);
       /*
           RBW - 11/13/1998 - new: maintain the stacking order chain.
       */
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked].

---

From: Dominik Vogt <[email-masked]>
Date: Sat, 2 Jan 1999 07:20:52 +0100

> or RaiseWindow are called).

Can any of my fellow maintainers comment on this? It sounds like
a good thing to fix and I'm willing to put it in 2.2. But I know
nothing about this bug and can't judge if the patch is OK.

Bye

Dominik

-- 
Dominik Vogt, Hewlett-Packard GmbH, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/14-4596, fax: 07031/14-3883, [email-masked]
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked].

---

From: Bob Woodside <[email-masked]>
Date: Sat, 2 Jan 1999 15:26:14 -0500 (EST)

Dominik Vogt <[email-masked]> wrote:
> > ...problems with the interaction between StaysOnTop and
> > override_redirect windows...

	This looks *almost* ok...the line

      changes.sibling = Scr.FvwmRoot.stack_prev->stack_next->frame;

needs to be changed to

      changes.sibling = Scr.FvwmRoot.stack_next->frame;

With this change it seems to work ok (i.e., in 5 minutes' worth of
superficial testing I haven't seen any ill effects, and the Netscape
example Matthias gave now has the menus behaving nicely.

	If you like, I'll put the revised patch into CVS later this
evening.

	Matthias: the reason for the change is that you weren't actually
picking up the top of the FVWM-managed window stack, so the patch as
originally written got the stacking order chain out of sync with the
real world and broke some other things, like RaiseLower behavior.

	Hmmm...wonder what the implications of this patch are for the
old "AutoRaise hides my transients under the parent window" bugaboo?
I'll have to do some more testing.

- Bob

---

From: Bob Woodside <[email-masked]>
Date: Sat, 2 Jan 1999 16:18:29 -0500 (EST)

Bob Woodside <[email-masked]> wrote:
> 	This looks *almost* ok....
> 

	Well, I spoke too soon. The behavior of the windows does *appear*
to be correct, but I find that I'm getting the following messages on the
console every time I raise a window:

[FVWM][FvwmErrorHandler]: <<ERROR>> *** internal error ***
[FVWM][FvwmErrorHandler]: <<ERROR>> Request 12, Error 8, EventType: 4

(This is with my revised version of Matthias' patch.)

	I'll have to look into this a bit further. It's not quite ready
for Prime Time.

- Bob

---

From: Matthias Clasen <[email-masked]>
Date: Sun, 3 Jan 1999 03:57:21 +0100

Now that you mention it, I have also seen these errors. Request 12
is ConfigureWindow and Error 8 is BadMatch.
Adding some debugging code quickly revealed that these are all caused
my the XConfigureWindow added by my patch which may be called with
window == sibling. It seems that X chokes on that. putting a line
 if (wins[0] != Scr.FvwmRoot.stack_next->frame) 
 before that XConfigureWindow call fixes the problem.

Matthias

---

From: Dominik Vogt <[email-masked]>
Date: Sun, 3 Jan 1999 04:25:27 +0100

> 	If you like, I'll put the revised patch into CVS later this

Well, I'm grateful if you take care of this, Bob. These problems
that came up were exactly the reason why I didn't want to touch it.
Feel free to commit the patch when you think it is ready.

Bye

Dominik

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]

---

From: Bob Woodside <[email-masked]>
Date: Sun, 3 Jan 1999 02:59:17 -0500 (EST)

>  before that XConfigureWindow call fixes the problem.

	I tried a slightly different approach, through trial and error
(lots of errors!), which seems to correct the ill-effects of the
original patch and the BadMatch errors introduced by my change. The
thing that bothers me is that I don't understand why.

	Try this, and see if eveything works OK for you. Replace the
line

from the original patch with this:

      if (wins[0] == t->frame)
        {
        }
      else
        {
          changes.sibling = 0;
        }

	I know this seems curious, but Scr.FvwmRoot isn't a real FvwmWindow,
and Scr.FvwmRoot.stack_prev->stack_next->frame evaluates to the same thing
as Scr.FvwmRoot->frame, which is always 0. For some reason, in the cases
where the first window in the wins array is *not* the window for which the
raise request was issued, this works. It "works" also, in the sense of not
generating an error, in the case where t->frame is the window for which we
issue the XConfigureWindow, but it fails to raise the window.

	I don't understand the BadMatch errors, since all the frame windows
*should* be siblings. I also don't understand why the XConfigureWindow with
the sibling set to 0 manages to avoid raising the windows above the
override_redirect windows. I'd feel much more comfortable about the whole
thing if I knew why it was so, but this version of the patch does seem to
work. Any ideas?

- Bob

---

From: Bob Woodside <[email-masked]>
Date: Sun, 3 Jan 1999 03:34:21 -0500 (EST)

Bob Woodside <[email-masked]> wrote:
> *should* be siblings. 

	The only thing I can think of is that perhaps X whines because the
window in question (it'll be a StaysOnTop or a transient) is *already*
above the top window in the stacking order chain, due to an oversight on
my part. My documentation doesn't show this as a possible cause for the
BadMatch, but I wonder....

	The stacking order chain is only modified for the window for which
the raise request is explicitly issued; it doesn't take into account the
logic that builds the stacking order list for XRestackWindows, with all
the StaysOnTop windows on top. 

	I'll look into this tomorrow.

- Bob

---

From: Matthias Clasen <[email-masked]>
Date: Mon, 4 Jan 1999 03:57:35 +0100

I think it is quite understandable that you get a BadMatch if
sibling == window. In my understanding, the sibling relation is
irreflexive, ...doesn't make much sense to place a window above
itself in the stacking order...

Upon reading man XConfigureWindow, I found among the possible errors:

       BadWindow A value for a Window argument does not name a
                 defined Window.

And XConfigureWindow does indeed produce these if sibling is zero. We
did not notice this since FvwmErrorHandler doesn't choke on BadWindow.
Thus using a zero sibling doesn't make XConfigureWindow treat override
redirects in any special way, its just that RaiseWindow works ok
without the XConfigureWindow call 90% of the time. This is not surprising,
if you remember that wins[0], the window being raised will usually
be one of the StaysOnTop windows and thus will already be high in the
stacking order.

The following version is still my favourite:

     if (wins[0] != Scr.FvwmRoot.stack_next->frame) {
        XWindowChanges changes;
        changes.sibling = Scr.FvwmRoot.stack_next->frame;
        changes.stack_mode = Above;
        XConfigureWindow(dpy, wins[0], (CWSibling|CWStackMode), &changes);
      }

---

From: Bob Woodside <[email-masked]>
Date: Mon, 4 Jan 1999 00:38:47 -0500 (EST)

Matthias Clasen <[email-masked]> wrote:
> sibling == window. 

	Excellent detective work! I had mistakenly thought that this
was happening in a case where sibling != window, but double checking
myself showed this not to be the case.

>     [snip]

	Yes, this seems to produce the desired results in all cases.
Unfortunately, I have a little more to do in this area, since, as I
mentioned, the stacking order chain code doesn't keep the StaysOnTop
windows at the top of the chain, so it gets out of sync with reality.

	I have a working version of the correction for this, but I'd
like to test and refine it for a day or two. Then I'll commit that
along with your change.

- Bob

---

From: Matthias Clasen <[email-masked]>
Date: Tue, 5 Jan 1999 00:28:53 +0100

> like to test and refine it for a day or two. Then I'll commit that

I have been thinking recently that it would be a simpler and more
consistent model to use a layered stacking order instead of the current
staysontop model. The problems with the implementation of staysontop
stem IMO from the fact that it is really an impossible task for the
wm to guarantee that all staysontop windows are fully visible at
all times. 

The alternative would be to assign a layer to each managed window
and guarantee only that a window in a higher layer is above all
lower layers. In this model, raising/lowering a window would only
raise/lower it in its layer and there would be separate operations
for changing the layer. Staysontop would simply be the topmost
layer and staysonbottom would come for free.

I think this would be relatively straightforward to implement if
you put an array of FvwmWindow * pointing to the topmost windows
in each layer. I think such a layered model is also part of the  
(otherwise not so great) Gnome WM Hints proposal.

What do you think ?

Matthias