Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-26 Thread Dominique Michel
Le Fri, 25 Nov 2011 09:18:52 +,
Thomas Adam tho...@fvwm.org a écrit :

 On Fri, Nov 25, 2011 at 12:47:06AM -0600, David Fries wrote:
  Sending a SIGTERM to FvwmButtons works to gracefully unswallow a
  program called stalonetray which is a system tray.  Setting
  -transient on the FvwmButtons panel is calling XDestroyWindow and
  in term causes the programs with items in the system tray to crash
  with a bad window error.  I'm removing the XDestroyWindow call and
  letting all transient
 
 Well, I suppose what's happening here is that if an application has
 registered with a systray which is then killed, then it's probably
 defined behaviour that those programs will die along with it,
 although that to me seems buggy behaviour.

It is a buggy behaviour. Such an app must wait for another systray app
to show up. As example of good behaviour, qjackctl and claws-mail work
that way. You can kill stalonetray and restart the systray with trayer
(or the contrary), and their icons will show up into the new systray.

Into fvwm-crystal, I have:

Test (EnvMatch NotificationAreaManager stalonetray) Test (!EnvMatch
trayer_width 0) *FvwmButtons-BottomNotifAera: (1x1, \
 Swallow (Close, Respawn, FvwmModule) FvwmStalonePanel TrayerPanel)
 
Test (EnvMatch NotificationAreaManager trayer) Test (!EnvMatch
 trayer_width 0) *FvwmButtons-BottomNotifAera: (1x1, \
 Swallow (Close, Respawn, FvwmModule) trayer TrayerPanel)

Module FvwmButtons FvwmButtons-BottomNotifAera

###
For stalonetray:

DestroyFunc TrayerPanel
AddToFunc TrayerPanel
+ I DestroyModuleConfig FvwmStalonePanel: *
...
+ I *FvwmStalonePanel: (1x1, Padding 0 0, Swallow (NoClose)
  stalonetray 'Exec stalonetray \
...
+ I Module FvwmButtons -g
  $[trayer_area_width]x$[trayer_area_heigth]$[trayer_x]$[trayer_y]
  FvwmStalonePanel

AddToFunc ExitFunction I Exec exec killall stalonetray

###
For trayer

DestroyFunc TrayerPanel
AddToFunc TrayerPanel
+ I Exec exec trayer \
...

AddToFunc ExitFunction I Exec exec killall trayer

###
Both stalonetray and trayer will be killed and restarted during a
restart. And it work fine with compliant applications.

It is some freedesktop specifications here:
http://www.freedesktop.org/wiki/Specifications/systemtray-spec
It say nothing about a restart. But from

An application wishing to provide an icon to the system tray should
first locate the system tray by requesting the owner window of the
manager selection. If the manager selection has no owner, clients may
use the method described in the ICCCM (watching for a MANAGER client
message) to be notified when a system tray appears.

we can extrapolate that an application that fail to locate a systray
should not crash... but be waiting to be notified when a system tray
appears.

Dominique

-- 
We have the heroes we deserve.



Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-26 Thread Thomas Adam
On Sat, Nov 26, 2011 at 12:37:16PM +0100, Dominique Michel wrote:
 Le Fri, 25 Nov 2011 09:18:52 +,
 Thomas Adam tho...@fvwm.org a écrit :
 
  On Fri, Nov 25, 2011 at 12:47:06AM -0600, David Fries wrote:
   Sending a SIGTERM to FvwmButtons works to gracefully unswallow a
   program called stalonetray which is a system tray.  Setting
   -transient on the FvwmButtons panel is calling XDestroyWindow and
   in term causes the programs with items in the system tray to crash
   with a bad window error.  I'm removing the XDestroyWindow call and
   letting all transient
  
  Well, I suppose what's happening here is that if an application has
  registered with a systray which is then killed, then it's probably
  defined behaviour that those programs will die along with it,
  although that to me seems buggy behaviour.
 
 It is a buggy behaviour. Such an app must wait for another systray app
 to show up. As example of good behaviour, qjackctl and claws-mail work

It will depend what happens to the application with XDestroyWindow() -- I
was referring to the fact that stalonetray is broken with this, which is not
surprising.  That other programs attached to it bomb out as well does not
surprise me, given what happens to stalonetray.

-- Thomas Adam

-- 
Deep in my heart I wish I was wrong.  But deep in my heart I know I am
not. -- Morrissey (Girl Least Likely To -- off of Viva Hate.)



Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-26 Thread Thomas Adam
On Sat, Nov 26, 2011 at 12:37:16PM +0100, Dominique Michel wrote:
 Both stalonetray and trayer will be killed and restarted during a
 restart. And it work fine with compliant applications.

Why?  The exit handling in FvwmButtons will take care of this, and forcibly
killing stalonetray or trayer in ExitFunction goes against settings one
might have in FvwmButtons as to what happens to the client it's going to
swallow.

Don't do this.

-- Thomas Adam

-- 
Deep in my heart I wish I was wrong.  But deep in my heart I know I am
not. -- Morrissey (Girl Least Likely To -- off of Viva Hate.)



Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-26 Thread Dominique Michel
Le Sat, 26 Nov 2011 11:46:07 +,
Thomas Adam tho...@fvwm.org a écrit :

 On Sat, Nov 26, 2011 at 12:37:16PM +0100, Dominique Michel wrote:
  Both stalonetray and trayer will be killed and restarted during a
  restart. And it work fine with compliant applications.
 
 Why?  The exit handling in FvwmButtons will take care of this, and
 forcibly killing stalonetray or trayer in ExitFunction goes against
 settings one might have in FvwmButtons as to what happens to the
 client it's going to swallow.

I don't remember why. But I remember than it is a huge difference of
behaviour between trayer and stalonetray, and it was difficult to get
stalonetray to work well in case of restart.

 
 Don't do this.
I will try when I get some time. Anyway, it look like it is other
simplifications I can do on this part of code.

 
 -- Thomas Adam
 


-- 
We have the heroes we deserve.



Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-26 Thread Dominique Michel
Le Sat, 26 Nov 2011 11:42:47 +,
Thomas Adam tho...@fvwm.org a écrit :

 On Sat, Nov 26, 2011 at 12:37:16PM +0100, Dominique Michel wrote:
  Le Fri, 25 Nov 2011 09:18:52 +,
  Thomas Adam tho...@fvwm.org a écrit :
  
   On Fri, Nov 25, 2011 at 12:47:06AM -0600, David Fries wrote:
Sending a SIGTERM to FvwmButtons works to gracefully unswallow a
program called stalonetray which is a system tray.  Setting
-transient on the FvwmButtons panel is calling XDestroyWindow
and in term causes the programs with items in the system tray
to crash with a bad window error.  I'm removing the
XDestroyWindow call and letting all transient
   
   Well, I suppose what's happening here is that if an application
   has registered with a systray which is then killed, then it's
   probably defined behaviour that those programs will die along
   with it, although that to me seems buggy behaviour.
  
  It is a buggy behaviour. Such an app must wait for another systray
  app to show up. As example of good behaviour, qjackctl and
  claws-mail work
 
 It will depend what happens to the application with XDestroyWindow()
 -- I was referring to the fact that stalonetray is broken with this,
 which is not surprising.  That other programs attached to it bomb out
 as well does not surprise me, given what happens to stalonetray.

Do you know if the developer of stalonetray is aware of this?
 
 
 -- Thomas Adam
 


-- 
We have the heroes we deserve.



Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-25 Thread David Fries
On Fri, Nov 25, 2011 at 09:18:52AM +, Thomas Adam wrote:
 On Fri, Nov 25, 2011 at 12:47:06AM -0600, David Fries wrote:
  Sending a SIGTERM to FvwmButtons works to gracefully unswallow a
  program called stalonetray which is a system tray.  Setting -transient
  on the FvwmButtons panel is calling XDestroyWindow and in term causes
  the programs with items in the system tray to crash with a bad window
  error.  I'm removing the XDestroyWindow call and letting all transient
 
 Well, I suppose what's happening here is that if an application has
 registered with a systray which is then killed, then it's probably defined
 behaviour that those programs will die along with it, although that to me
 seems buggy behaviour.  I'd be much more inclined to look at what can be
 done with Stalonetray.  I've tried before, and so far, got no where.
 
 Nevertheless, the point of XDestroyWindow() here is so that we really do
 unmap the window forcefully as we cannot rely on those programs we need to
 get rid of coming out of a transient FvwmButton state to have the necessary
 gumption to unmap their windows; I've seen a few applications like this
 before -- and doing so just leaves them running in the background.  Don't
 forget that we're responsible as the parent of these windows.

I took another look and did some tests.  DeadPipeCleanup is called
atexit().  With my patch and -transient, the Swallow flag Kill is
causing the X-server to close to the connection to that client, the
flag Close is doing something less forcefully, but in both cases
stalonetray and xclock have terminated when the button panel closes.
Compared to the previous XDestroyWindow behavior where the programs in
the tray are killed, the stalonetray window is destroyed (but it keeps
on running because it's too stupid to exit).

From what I can tell exiting the loop normally is doing the right
thing with Close and Kill, and unswallowing NoClose windows correctly.
What about about adding a XDestroyWindow call at the end of
DeadPipeCleanup?  Would that be sufficient for forcefully destroying
anything left after the other cleanup ran?

 So I think I'll look in to this in more detail.  I don't mind the intent of
 your patch, but I do not like your approach to it -- and continual use of
 isTerminated is icky and nasty in the case where we should behandling
 transient FvwmButtons here much more gracefully than having many exit points
 as we've now got in the code.
 
 So I'll rework this, and commit something over the weekend.  How's that
 sound?

That would be great, thanks.  If I knew what you were thinking I could
code up the changes myself and submit another patch.

-- 
David Fries da...@fries.netPGP pub CB1EE8F0
http://fries.net/~david/



Re: [PATCH] FvwmButtons transient avoid destroy

2011-11-25 Thread Thomas Adam
On 25 November 2011 17:53, David Fries da...@fries.net wrote:
 I took another look and did some tests.  DeadPipeCleanup is called
 atexit().  With my patch and -transient, the Swallow flag Kill is

The code is clear about this point.

 causing the X-server to close to the connection to that client, the
 flag Close is doing something less forcefully, but in both cases
 stalonetray and xclock have terminated when the button panel closes.

Yes.

 Compared to the previous XDestroyWindow behavior where the programs in
 the tray are killed, the stalonetray window is destroyed (but it keeps
 on running because it's too stupid to exit).

See previous comments on this.

 From what I can tell exiting the loop normally is doing the right
 thing with Close and Kill, and unswallowing NoClose windows correctly.
 What about about adding a XDestroyWindow call at the end of
 DeadPipeCleanup?  Would that be sufficient for forcefully destroying
 anything left after the other cleanup ran?

Nope, no more so than what is currently happening.   Also, you're not
considering enough of the b-flags in this which would need to be
checked before doing anything in DeadPipeCleanup().

Until I look in to this in more detail, I'm perhaps more inclined to
think the applications in question need fixing, not FvwmButtons, but
we'll see.  :)

-- Thomas Adam