On Sat, Nov 26, 2011 at 11:35:52AM +0000, Thomas Adam wrote:
> On Fri, Nov 25, 2011 at 07:37:17PM +0000, Thomas Adam wrote:
> > 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. :)
>
> So this problem shows much more that stalonetray is broken -- but I, others,
> and even the author knows this already.
>
> You're right about XDestroyWindow() not needed in the case of a transient
> FvwmButtons -- the DeadPipeCleanup() function suffices, and because there's
> a call to XKillClient() in there for the pathological case, we'll be fine.
>
> So just removing the XDestroyWindow() calls and allowing the atexit()
> handler to work for us instead is the right solution for all the tests I've
> done.
>
> For example, with the attached patch, this does the right thing when the
> transient FvwmButton closes:
>
> KillModule FvwmButtons a
> DestroyModuleConfig a:*
> *a: Columns 16
> *a: Rows 1
> *a: (16x1, Swallow `stalonetray` `Exec exec /usr/bin/stalonetray \
> --geometry 16x1 \
> --no-shrink --kludges force_icons_size -i 16`, Action (Mouse 1) Nop)
>
> I tried this with empathy -- without the patch attached, empathy dies along
> with the closing of the FvwmButtons instance because stalonetray sucks.
> With the patch, stalonetray is still running, but empathy is still present.
> I suspect (although now refuse to look) that stalonetray just unmaps its
> window and continues to run.
I decided to use wireshark to look at the X11 protocol and see what
was happening. I setup FvwmButtons swallowing xclock and stalonetray,
then just blueman-applet in the tray. "(NoClose NoKill Hints UseOld
NoTitle)" Without the patch stalonetray gets a DestroyNotify for both
the stalonetray and blueman-applet windows, along with a set of
PropertyNotify Deleted events. stalonetray requests
GetWindowAttributes and GetGeometry, on the blueman-applet window,
which gets BadWindow and BadDrawable, then the same on its window,
then it tries to UnmapWindow, ReparentWindow, ConfigureWindow, and
MapWindow on the blueman-applet window all of which get a BadWindow or
BadDrawable because it was already destroyed, and some other things
not of interest.
blueman-applet gets a DestroyNotify for its window and PropertyNotify
deletes, it makes SYNC DestroyCounter and RENDER FreePicture request,
with an Error render-Picture return, and it dies.
The problem I see with stalonetray is it keeps running after its
window was destroyed, but it didn't do anything that caused the
programs in the toolbar to exit, and I don't think there was anything
it could have done to prevent it either.
> So this seems to be doing the right thing in preserving the windows
> registered with stalonetray -- even if that client is broken.
>
> Note also the following case still works:
>
> KillModule FvwmButtons a
> DestroyModuleConfig a:*
> *a: Columns 16
> *a: Rows 1
> *a: (16x1, Swallow `stalonetray` `Exec exec /usr/bin/stalonetray \
> --geometry 16x1 \
> --no-shrink --kludges force_icons_size -i 16`, Action (Mouse 1) Nop)
> *a: (3x1, Swallow (NoClose, UseOld, NoKill) `xclock` \
> `Exec exec xclock -digital`)
>
>
> In that when the transient FvwmButtons window closes, empathy remains, and
> xclock is reparented correctly.
>
> I've also snuck in an additional change to this patch which makes the
> -transient option work to close the FvwmButtons instance regardless of
> whether a click to one of its buttons has an Action assigned to it or not.
> To me, it's odd that I can only close the -transient FvwmButtons window if
> the button I click on has an action.
>
> Does this still fix your case?
Yes with this patch, stalonetray is being unswallowed, and reswallowed
as I want it to work. Thanks.
Sorry to take so long to get back, I've had a very busy week out of
town on travel.
> Oh, and by the way, the XDestroyWindow() calls predate the point the signal
> handler was changed to cope with DeadPipeCleanup() -- they should likely
> always have been removed when DeadPipeCleanup() looks the way it does now;
> although at this point, I suppose we woulndn't normally care about X11
> errors, until broken applications such as stalonetray cause more problems.
>
> So I don't mind this change, but it is fixing a bug in stalonetray, which
> will likely now always be masked by this change. I do not always agree that
> a window manager should circumvent bugs in applications. But getting it
> fixed in stalonetray is painful.
>
> -- 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.)
> diff --git a/modules/FvwmButtons/FvwmButtons.c
> b/modules/FvwmButtons/FvwmButtons.c
> index 0b138b3..13c40b2 100644
> --- a/modules/FvwmButtons/FvwmButtons.c
> +++ b/modules/FvwmButtons/FvwmButtons.c
> @@ -995,13 +995,6 @@ void ButtonPressProcess (button_info *b, char **act)
> i2++;
> }
> strcat(tmp.name ,&(*act)[i2]);
> - if (is_transient)
> - {
> - /* delete the window before continuing
> - */
> - XDestroyWindow(Dpy, MyWindow);
> - XSync(Dpy, 0);
> - }
> SendText(fd,tmp.name,0);
> if (is_transient)
> {
> @@ -1020,13 +1013,6 @@ void ButtonPressProcess (button_info *b, char **act)
> SaveButtons(UberButton);
> else
> {
> - if (is_transient)
> - {
> - /* delete the window before continuing
> - */
> - XDestroyWindow(Dpy, MyWindow);
> - XSync(Dpy, 0);
> - }
> SendText(fd,*act,0);
> if (is_transient)
> {
> @@ -1283,6 +1269,10 @@ void Loop(void)
> Dpy, Event.xbutton.window, MyWindow, Event.xbutton.x,
> Event.xbutton.y, &x, &y, &dummy);
> }
> + if (is_transient)
> + {
> + exit(0);
> + }
> if (CurrentButton)
> {
> b = CurrentButton;
--
David Fries <da...@fries.net> PGP pub CB1EE8F0
http://fries.net/~david/
