On Sat, Jan 08, 2022 at 11:40:34AM -0800, robert wrote:
> This patch replaces the previous one I sent.
>
> The following changes are made in this patch:
> - Fix tracking of pressed buttons. Previously, pressing two buttons and
> then releasing one would make st think no buttons are pressed, which
> in particular broke MODE_MOUSEMOTION.
> - Always send the lowest-numbered pressed button on motion events; when
> no button is pressed for a motion event in MODE_MOUSEMANY, then send
> a release. This matches the behaviour of xterm. (Previously, st sent
> the most recently pressed button in the motion report.)
> - Remove UB (?) access to potentially inactive struct member
> e->xbutton.button of XEvent union.
> - Fix (unlikely) possibility of overflow for large button numbers.
>
> The one discrepancy I found between st and xterm is that xterm sometimes
> encodes buttons with large numbers (>5) strangely. E.g., xterm reports
> presses of buttons 8 and 9 as releases, whereas st properly (?) encodes
> them as presses.
> ---
> x.c | 86 ++++++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 51 insertions(+), 35 deletions(-)
>
> diff --git a/x.c b/x.c
> index 8a16faa..5f4ea73 100644
> --- a/x.c
> +++ b/x.c
> @@ -252,7 +252,7 @@ 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 */
> +static uint buttons; /* bit field of pressed buttons */
>
> void
> clipcopy(const Arg *dummy)
> @@ -364,61 +364,68 @@ mousesel(XEvent *e, int done)
> void
> mousereport(XEvent *e)
> {
> - int len, x = evcol(e), y = evrow(e),
> - button = e->xbutton.button, state = e->xbutton.state;
> + int len, btn, code;
> + int x = evcol(e), y = evrow(e);
> + int state = e->xbutton.state;
> char buf[40];
> static int ox, oy;
>
> - /* from urxvt */
> - if (e->xbutton.type == MotionNotify) {
> + if (e->type == MotionNotify) {
> if (x == ox && y == oy)
> 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)
> + /* MODE_MOUSEMOTION: no reporting if no button is pressed */
> + if (IS_SET(MODE_MOUSEMOTION) && buttons == 0)
> return;
> -
> - button = oldbutton + 32;
> - ox = x;
> - oy = y;
> + /* Set btn to lowest-numbered pressed button, or 12 if no
> + * buttons are pressed. */
> + for (btn = 1; btn <= 11 && !(buttons & (1<<(btn-1))); btn++)
> + ;
> + code = 32;
> } else {
> - if (!IS_SET(MODE_MOUSESGR) && e->xbutton.type == ButtonRelease)
> {
> - button = 3;
> - } else {
> - button -= Button1;
> - if (button >= 7)
> - button += 128 - 7;
> - else if (button >= 3)
> - button += 64 - 3;
> - }
> - if (e->xbutton.type == ButtonPress) {
> - oldbutton = button;
> - ox = x;
> - oy = y;
> - } else if (e->xbutton.type == ButtonRelease) {
> - oldbutton = 3;
> + btn = e->xbutton.button;
> + /* Only buttons 1 through 11 can be encoded */
> + if (btn < 1 || btn > 11)
> + return;
> + if (e->type == ButtonRelease) {
> /* MODE_MOUSEX10: no button release reporting */
> if (IS_SET(MODE_MOUSEX10))
> return;
> - if (button == 64 || button == 65)
> + /* Don't send release events for the scroll wheel */
> + if (btn == 4 || btn == 5)
> return;
> }
> + code = 0;
> }
>
> + ox = x;
> + oy = y;
> +
> + /* Encode btn into code. If no button is pressed for a motion event in
> + * MODE_MOUSEMANY, then encode it as a release. */
> + if ((!IS_SET(MODE_MOUSESGR) && e->type == ButtonRelease) || btn == 12)
> + code += 3;
> + else if (btn >= 8)
> + code += 128 + btn - 8;
> + else if (btn >= 4)
> + code += 64 + btn - 4;
> + else
> + code += btn - 1;
> +
> if (!IS_SET(MODE_MOUSEX10)) {
> - button += ((state & ShiftMask ) ? 4 : 0)
> - + ((state & Mod4Mask ) ? 8 : 0)
> - + ((state & ControlMask) ? 16 : 0);
> + code += ((state & ShiftMask ) ? 4 : 0)
> + + ((state & Mod4Mask ) ? 8 : 0)
> + + ((state & ControlMask) ? 16 : 0);
> }
>
> if (IS_SET(MODE_MOUSESGR)) {
> len = snprintf(buf, sizeof(buf), "\033[<%d;%d;%d%c",
> - button, x+1, y+1,
> - e->xbutton.type == ButtonRelease ? 'm' : 'M');
> + code, x+1, y+1,
> + e->type == ButtonRelease ? 'm' : 'M');
> } else if (x < 223 && y < 223) {
> len = snprintf(buf, sizeof(buf), "\033[M%c%c%c",
> - 32+button, 32+x+1, 32+y+1);
> + 32+code, 32+x+1, 32+y+1);
> } else {
> return;
> }
> @@ -461,9 +468,13 @@ mouseaction(XEvent *e, uint release)
> void
> bpress(XEvent *e)
> {
> + int btn = e->xbutton.button;
> struct timespec now;
> int snap;
>
> + if (1 <= btn && btn <= 11)
> + buttons |= 1 << (btn-1);
> +
> if (IS_SET(MODE_MOUSE) && !(e->xbutton.state & forcemousemod)) {
> mousereport(e);
> return;
> @@ -472,7 +483,7 @@ bpress(XEvent *e)
> if (mouseaction(e, 0))
> return;
>
> - if (e->xbutton.button == Button1) {
> + if (btn == Button1) {
> /*
> * If the user clicks below predefined timeouts specific
> * snapping behaviour is exposed.
> @@ -686,6 +697,11 @@ xsetsel(char *str)
> void
> brelease(XEvent *e)
> {
> + int btn = e->xbutton.button;
> +
> + if (1 <= btn && btn <= 11)
> + buttons &= ~(1 << (btn-1));
> +
> if (IS_SET(MODE_MOUSE) && !(e->xbutton.state & forcemousemod)) {
> mousereport(e);
> return;
> @@ -693,7 +709,7 @@ brelease(XEvent *e)
>
> if (mouseaction(e, 1))
> return;
> - if (e->xbutton.button == Button1)
> + if (btn == Button1)
> mousesel(e, 1);
> }
>
> --
> 2.17.1
>
>
Hi Robert,
Thanks for the patch, I've pushed it to the repo. I've tried to test all cases
and it works great.
--
Kind regards,
Hiltjo
