-
Notifications
You must be signed in to change notification settings - Fork 727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix erroneous ButtonDown mouse event reporting. #3647
base: main
Are you sure you want to change the base?
Conversation
I'm not seeing the effect you mention in my usual environment (macOS, kitty); so we can recreate and then test out this PR, what environment are you seeing this in? |
Oh, good question. After some testing this appears to only reproduce inside Intellij's builtin terminal implementation.
I could dig more into whether the bug is in Intellij's terminal. But the behavior seems potentially inline with the very hard to understand documentation about terminal modes. Testing with my fix applied causes mouse events in TextArea an Input to match across both terminal implementations, and brings the behavior with the Intellij terminal into line with Gnome terminals existing behavior. |
Ah, sorry I just saw your commit. Here's some of the information you need... You can find a video of the bug & fix here Here's the code I used to generate the video: from textual.app import App, ComposeResult
from textual.widgets import Input, TextArea
class InputApp(App):
def compose(self) -> ComposeResult:
yield TextArea("I'm not touching the mouse buttons.")
if __name__ == "__main__":
app = InputApp()
app.run() Textual DiagnosticsVersions
Python
Operating System
Terminal
Rich Console options
Additionally, the output captured from _xterm_parser.py can be found below. |
@davep Let me know if I can provide more. |
I suspect everything necessary is here; just needs one of us to find the time to recreate the problem and review (personally I'm on holiday at the moment). Might take a wee while (lots of holiday all around heading into this time of year, plus lots going on around Textual), but we'll get to it. |
@davep Sounds good. Enjoy your time away. I would love to land this before the next release. Because I use textual everywhere, having to patch it is :-( PS. I fixed the link for the video. |
Hey friends, Happy Holidays! I see there have been a couple of releases since this PR has been created. Unfortunately none of them contain a fix for the issues described here. How can I help this land? |
03793b6
to
96f0166
Compare
We have a backlog. But I will be looking at this soon... |
@willmcgugan Please let me know if I can help move things along. |
fc6ed1f
to
8d719d2
Compare
Would you mind creating an issue for this. Or linking to it if it already exists. |
After v0.38.0, I was seeing Input fields lose focus whenever the mouse left the input field area (but without having clicked anywhere). This was being caused because since the release Mouse ButtonDown events trigger loss of focus. But I wasn't pressing any mouse buttons. The erronious ButtonDown events were actually MouseMove events with no keys held down. According to the docs for xterm escape sequences, wher SGR (1006) mode is enabled, the encoded button value isn't always incremented by 32 for move movements events. [1] I believe the correct fix to this issue is to detect mouse movement events with no button down by checking if the "pressed button" is 0. Which appears to indicate that no mouse button is pressed (in this specific case). Whereas, the button value will be set to 1 when left click is pressed for example. I'm not sure if this change is fully correct for terminals which don't support SGR mode. [1] https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Extended-coordinates
e47d0ba
to
53af952
Compare
After v0.38.0, I was seeing Input fields lose focus whenever the mouse left the input field area (but without having clicked anywhere). This behavior started after the release v0.38.0 release when Mouse ButtonDown began to trigger loss of focus (This change seemed intentional). However, I wasn't pressing the mouse button down.
The erronious ButtonDown events were actually MouseMove events with no keys held down. According to the docs for xterm escape sequences, when SGR (1006) mode is enabled, the encoded button value isn't always incremented by 32 for move movements events. [1]
I believe the correct fix to this issue is to detect mouse movement events with no button down by checking if the "pressed button" is 0. Which appears to indicate that no mouse button is pressed (in this specific case). Whereas, the button value will be set to 1 when left click is pressed for example.
I'm not sure if this change is fully correct for terminals which don't support SGR mode.
[1] https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Extended-coordinates
Please review the following checklist.