[hackers] Re: [st][PATCH] Fix MODE_MOUSEMOTION reporting

From: Robert Russell <robertrussell.72001_AT_gmail.com>
Date: Fri, 7 Jan 2022 21:16:15 -0800

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 <robertrussell.72001_AT_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
> _AT_@ -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)
> {
> _AT_@ -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 {
> _AT_@ -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
>
Received on Sat Jan 08 2022 - 06:16:15 CET