Mouse Movement Getting Interpreted as Scroll Wheel

[mattroseman]

Problem

Reproduce

There's a bug when I run tcell in my urxvt terminal (9.31) on Arch Linux where mouse movements register as scrolling. I think you can visualize this issue by running the mouse.go demo in an urxvt terminal. I've included a recording below.

1. Run mouse.go demo
2. scroll mouse wheel (up or down)
3. press c to clear the output in the demo
4. move the mouse to see the previous scroll even re-captured

https://user-images.githubusercontent.com/6654122/225418841-e20f4eac-3939-439f-99bb-193b3536c5f5.mp4
Obviously you can't see, but when I'm moving the mouse in a circle and the Wheel Up is popping up, I'm not actually scrolling; when the pointer is still and the WheelUp/WheelDown shows up, I'm actually scrolling the mouse wheel.

Why

I believe what's happening is that mouse wheel events are being recorded correctly here, but subsequent mouse movement events are still including the data of the previous mouse wheel event.

So if you scroll down, the WheelDown event is seen, then you stop scrolling and move the mouse, the mouse movement will still contain the WheelDown event.

I didn't see this issue on xterm on Arch Linux, or ITerm2 on macOS.

Possible Fix

I was messing with parseSgrMouse in tscreen.go and think I have an idea of how to resolve this bug.

If you don't set the buttondn boolean on WheelDown or WheelUp events, then those events should be cleared on subsequent mouse movements, i.e. when the next event goes to the else if motion conditional, it'll clear btn presses, including the WheelDown and WheelUp events.

motion = (btn & 32) != 0
scroll = (btn & 0x43) == 0x40 || (btn & 0x43) == 0x41 // NEW CODE
btn &^= 32
if b[i] == 'm' {
    // mouse release, clear all buttons
    btn |= 3
    btn &^= 0x40
    t.buttondn = false
} else if motion {
    /*
     * Some broken terminals appear to send
     * mouse button one motion events, instead of
     * encoding 35 (no buttons) into these events.
     * We resolve these by looking for a non-motion
     * event first.
     */
    if !t.buttondn {
	    btn |= 3
	    btn &^= 0x40
    }
} else if !scroll { // NEW CODE
    t.buttondn = true
}

2023-03-15.15-41-28.mp4

This recording shows that after a WheelDown event, the subsequent mouse movement will clear the WheelDown. This demo works how I'd expect, and how I see it work on xterm on the main branch.

My worry is that I'm unintentionally breaking the functionality of the buttondn boolean and this will cause issues I'm not aware of.
My understanding is that the buttondn logic is to avoid clearing the mouse buttons on motion if the mouse button is still pressed, e.g. click-and-drag. But in this case, it seems that you would want to clear the mouse button events if the "button" that was pressed was the wheel scrolling down/up.
I might not understand the code though, and could be completely off; I'm new to this code base so this is my best guess.