I realized I misinterpreted something I read about how DECSET 1002 (i.e., MODE_MOUSEMOTION) should behave, so my patch is wrong for a different reason. The motion event should encode one of the (possibly many) pressed buttons. I checked a couple other terminals, and they seem to be choosing the lowest-numbered button. E.g., with buttons 1 and 3 pressed, motion events should indicate that button 1 is held during the motion (and say nothing about button 3). To do this, I guess we need to track the state of every mouse button, or rather mouse buttons 1 through 11, because the encoding scheme doesn't support buttons past 11. If this all sounds right to you, I can submit a new patch.
Regards, Robert On Fri, Jan 7, 2022 at 3:33 PM robert <[email protected]> wrote: > > Prior to this commit, mousereport tracks the previous button > press/release in the oldbutton variable. When MODE_MOUSEMOTION is set, > mouse reports should be sent for motion events only if a mouse button is > held down, and mousereport uses oldbutton to decide if a button is > pressed. But this is buggy; e.g., try the following in st: > 1. Run "echo '\e[?1002h'" and "cat -v". > 2. Press buttons 1 and 2 and move your pointer around in st. Observe > that st sends mouse reports. > 3. Release button 2 and move your pointer around in st. Observe that > st is no longer sending mouse reports, despite button 1 still > being pressed. > 4. Interrupt cat and run "echo '\e[?1002l'" to disable mouse > reporting. > (You can also try this in xterm to see the proper behaviour.) > > The fix is simply to remove oldbutton. When MODE_MOUSEMOTION is set, the > window event_mask is set such that st receives MotionNotify events only > with at least one button pressed, so nothing more is required. > > Incidentally, oldbutton was used in one other way in mousereport: it was > added to the event code for motion events, so that motion reports had > the same two most significant bits and the same two least significant > bits as the previous button press/release report, but this seems > pointless, because the terminal client should ignore these bits for > motion events. > --- > x.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/x.c b/x.c > index 8a16faa..c31057f 100644 > --- a/x.c > +++ b/x.c > @@ -252,8 +252,6 @@ static char *opt_line = NULL; > static char *opt_name = NULL; > static char *opt_title = NULL; > > -static int oldbutton = 3; /* button event on startup: 3 = release */ > - > void > clipcopy(const Arg *dummy) > { > @@ -375,11 +373,8 @@ mousereport(XEvent *e) > return; > if (!IS_SET(MODE_MOUSEMOTION) && !IS_SET(MODE_MOUSEMANY)) > return; > - /* MOUSE_MOTION: no reporting if no button is pressed */ > - if (IS_SET(MODE_MOUSEMOTION) && oldbutton == 3) > - return; > > - button = oldbutton + 32; > + button = 32; > ox = x; > oy = y; > } else { > @@ -393,11 +388,9 @@ mousereport(XEvent *e) > button += 64 - 3; > } > if (e->xbutton.type == ButtonPress) { > - oldbutton = button; > ox = x; > oy = y; > } else if (e->xbutton.type == ButtonRelease) { > - oldbutton = 3; > /* MODE_MOUSEX10: no button release reporting */ > if (IS_SET(MODE_MOUSEX10)) > return; > -- > 2.17.1 >
