On Mon, Jul 08, 2019 at 01:31:51PM +0200 I heard the voice of
Frank Steiner, and lo! it spake thus:
>
> Now with that Ungrab commented out, [...]
>
> This is correct, of course, because the left mouse button is still
> hold and then xvkbd "executes" the right mouse button, [...]
>
> Thus, the XUngrabPointer in the f.exec allows the command that is
> executed by f.exec to simulate mouse clicks even if f.exec was
> started by a mouse click. This makes sense with my xvkbd example.

Sounds like a reasonable analysis.


> So the bug is somewhere else: Looking at xev, the first click of the
> mouse button bound to the f.exec  shows the ButtonRelease event, as
> it should.

This is where the wheels fall off.  It _shouldn't_.  xev only sees the
Release event because we UnGrab'd, which is why ctwm doesn't.  If you
comment out the UnGrab, xev _won't_ see it anymore, because ctwm gets
it and handles the Release.

With the ungrab in place, ctwm never gets the release event, which is
why xev sees it.  The second time you click, we hit the error path in
HandleButtonPress(), which specifically Grab's the pointer and
subscribes to ButtonPress/Release events (and then _doesn't_ proceed
to the f.exec which Ungrab's), which is why we see the release the
second time.   (see long stuff at the end of mail for instrumentation
results)

Now, when running from a menu, we're already in the Release handler
when we f.exec.  So when the exec finishes, we already know we're
Release'ing, so we do all the cleanup.  So that's why _that_ works as
expected.


(In case you're skimming; the below patches are all fun things to mess
with to see why things are happening; none of them are solutions to
the problem, or even things you should run with in production!)


So, we can work around that by re-Grab'ing after executing the
function, Just In Case(tm) we ungrab'd in the process (patch below).
That's not actually a solution though, since we it's only in _that_
case that we should re-grab.  For instance, with the following patch,
try doing a Press-related action (like moving a window by clicking the
titlebar), then notice that a click anywhere acts as a click on the
root (e.g., pulling up a menu).

And we don't want to re-grab inside the f.exec handler itself unless
we know we should have been grabbed beforehand, since we might not get
there as a result of a ButtonPress/Release anyway; what if it were a
keybinding?  So, we probably need to get even extra smartererer about
knowing whether we _should_ ungrab in f.exec, and if so, how we should
re-grab afterward.  Unfortunately, there's no
XPlzToTellMeMyCurrentPointerGrabbingStatus().  Mmmlech.  Just what we
need, another random magic-state-tracking global var...


-----------
=== modified file 'event_handlers.c'
--- event_handlers.c    2019-07-11 02:13:12 +0000
+++ event_handlers.c    2019-07-15 00:55:47 +0000
@@ -2995,6 +2995,10 @@
 #endif /* EWMH */
                                        ExecuteFunction(func,
                                                        Action, 
Event.xany.window, Tmp_win, &Event, Context, false);
+XGrabPointer(dpy, Scr->Root, True,
+             ButtonReleaseMask | ButtonPressMask,
+             GrabModeAsync, GrabModeAsync,
+             Scr->Root, None, CurrentTime);
                                }
                }
        }

-----------




The following is just a dump of some of the extra verbose bits I went
through along the way, leading to the above analysis.  It doesn't add
anything much, except for how I got to the above.

I added little instrumenting (trivial patch below) of DispatchEvent()
to describe every event we see.  When I captured the below, I also had
extra lines on the Press/Release handlers (left over from previous
iteration; doesn't add anything here).  I tested mappings with f.beep
and a f.exec.  I was using an xev window, so I could see both sides at
once.  Here I'm only including the ctwm outputs.  The f.whatever
execution would be happening within the HandleButtonPress() call.

With f.beep, when pressing the button, you see:

DispatchEvent(LeaveNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(ButtonPress) on 'Event Tester'
HandleButtonPress()
DispatchEvent(LeaveNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on '(nowin)'

And then you release, and get:

DispatchEvent(ButtonRelease) on '(nowin)'
HandleButtonRelease()
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on 'Event Tester'                                    
      DispatchEvent(FocusIn) on 'Event Tester'
DispatchEvent(PropertyNotify) on '(nowin)'


OK, about what you expect.  Now let's try f.exec.  Press:

DispatchEvent(LeaveNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(ButtonPress) on 'Event Tester'
HandleButtonPress()
DispatchEvent(LeaveNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on '(nowin)'

OK, exactly the same.  Now we release:

(  ...  crickets  ...  )

No events at all.  That's why xev sees the ButtonRelease, instead of
only Enter like it does in the normal case when ctwm's intercepting
them.


So, let's Press the second time:

DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(FocusIn) on 'Event Tester'
DispatchEvent(PropertyNotify) on '(nowin)'
           [ Hey, those first 4 are the same as we otherwise get
             following the ButtonRelease! ]

DispatchEvent(LeaveNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(ButtonPress) on 'Event Tester'
HandleButtonPress()
DispatchEvent(LeaveNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on '(nowin)'
           [ And then the ones we expect to normally get on Press ]

Then the following Release is normal:

DispatchEvent(ButtonRelease) on '(nowin)'
HandleButtonRelease()
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(EnterNotify) on 'Event Tester'
DispatchEvent(FocusIn) on 'Event Tester'
DispatchEvent(PropertyNotify) on '(nowin)'


Wacky wacky.


Dump events:

-----------
=== modified file 'event_core.c'
--- event_core.c        2018-11-12 00:27:58 +0000
+++ event_core.c        2019-07-14 23:58:54 +0000
@@ -41,6 +41,7 @@
 #include "event_handlers.h"
 #include "event_internal.h"
 #include "event_names.h"
+#include "event_names_table.h"
 #include "functions.h"
 #include "iconmgr.h"
 #include "image.h"
@@ -308,6 +309,7 @@
        StashEventTime(&Event);
        Tmp_win = GetTwmWindow(w);
        thisScr = GetTwmScreen(&Event);
+fprintf(stderr, "%s(%s) on '%s'\n", __func__, event_names[Event.type], 
(Tmp_win ? Tmp_win->name : "(nowin)"));
 
        dumpevent(&Event);
 

-----------



-- 
Matthew Fuller     (MF4839)   |  fulle...@over-yonder.net
Systems/Network Administrator |  http://www.over-yonder.net/~fullermd/
           On the Internet, nobody can hear you scream.

Reply via email to