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.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
> >
>
--
Kind regards,
Hiltjo
Received on Sat Jan 08 2022 - 11:39:26 CET