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

Reply via email to