Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EricWF
Copy link

@EricWF EricWF commented Nov 7, 2023

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.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

@darrenburns darrenburns mentioned this pull request Nov 8, 2023
@davep
Copy link
Collaborator

davep commented Nov 9, 2023

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?

davep added a commit to davep/textual-sandbox that referenced this pull request Nov 9, 2023
@EricWF
Copy link
Author

EricWF commented Nov 10, 2023

Oh, good question. After some testing this appears to only reproduce inside Intellij's builtin terminal implementation.
It does not seem to reproduce with gnome's terminal implementation.
I'm not sure exactly what to provide, so let me know if you would like something in particular. Here are some relevant environment variables:

JEDITERM_SOURCE_ARGS=
FIG_JETBRAINS_SHELL_INTEGRATION=1
TERM=xterm-256color
LANG=en_US.UTF-8
XMODIFIERS=@im=ibus
XDG_SESSION_TYPE=x11
TERMINAL_EMULATOR=JetBrains-JediTerm
XDG_VTNR=2
VTE_VERSION=7400
SHELL=/usr/bin/fish
COLORTERM=truecolor
PLATFORM_TYPE=Linux

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.

@EricWF
Copy link
Author

EricWF commented Nov 11, 2023

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 Diagnostics

Versions

Name Value
Textual 0.41.0
Rich 13.6.0

Python

Name Value
Version 3.11.6
Implementation CPython
Compiler GCC 13.2.0
Executable /home/eric/.pyenvs/idea/bin/python3

Operating System

Name Value
System Linux
Release 6.5.0-2-amd64
Version #1 SMP PREEMPT_DYNAMIC Debian 6.5.6-1 (2023-10-07)

Terminal

Name Value
Terminal Application PyCharm
TERM xterm-256color
COLORTERM truecolor
FORCE_COLOR Not set
NO_COLOR Not set

Rich Console options

Name Value
size width=112, height=22
legacy_windows False
min_width 1
max_width 112
is_terminal False
encoding utf-8
max_height 22
justify None
overflow None
no_wrap False
highlight None
markup None
height None

Additionally, the output captured from _xterm_parser.py can be found below.
mousecodes.log

@EricWF
Copy link
Author

EricWF commented Nov 17, 2023

@davep Let me know if I can provide more.

@davep
Copy link
Collaborator

davep commented Nov 17, 2023

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.

@EricWF
Copy link
Author

EricWF commented Nov 18, 2023

@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.

@EricWF
Copy link
Author

EricWF commented Jan 4, 2024

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?

@willmcgugan
Copy link
Collaborator

We have a backlog. But I will be looking at this soon...

@EricWF
Copy link
Author

EricWF commented Jan 30, 2024

@willmcgugan Please let me know if I can help move things along.

@EricWF EricWF force-pushed the fix-terminal-movements branch 2 times, most recently from fc6ed1f to 8d719d2 Compare January 30, 2024 19:21
@willmcgugan
Copy link
Collaborator

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants