On Fri, Jan 07, 2022 at 09:16:15PM -0800, Robert Russell wrote: > 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. >
Hi Robert, Thanks for the report and initial patch. Please feel free to work further on the patch and make it work with atleast the limited X10- and also SGR encoding. It would be great if it matches the xterm mouse behaviour in this case. Thank you, > Regards, > Robert > > On Fri, Jan 7, 2022 at 3:33 PM robert <robertrussell.72...@gmail.com> 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 > > > -- Kind regards, Hiltjo