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

Mouse movement control sequences leak to terminal at app shutdown #4418

Closed
slawek-es opened this issue Apr 15, 2024 · 19 comments · Fixed by #4681
Closed

Mouse movement control sequences leak to terminal at app shutdown #4418

slawek-es opened this issue Apr 15, 2024 · 19 comments · Fixed by #4681

Comments

@slawek-es
Copy link

If the user is moving the mouse while the textual application exits, terminal control sequences from the mouse movement may leak from the application and get into the terminal.

This happens on all terminals I have tested: gnome-terminal and Terminator on Ubuntu Linux, and the Terminal on macOS. The issue can be easily reproduced using the examples from the textual repository. Here's a reproduction with five_by_five.py, where I press q on the keyboard while moving the mouse around the terminal:

control-seq-leak-2024-04-15_10.56.24.mp4

Please find the diagnostics output below:

textual diagnose output
<!-- This is valid Markdown, please paste the following directly in to a GitHub issue -->
# Textual Diagnostics

## Versions

| Name    | Value  |
|---------|--------|
| Textual | 0.56.4 |
| Rich    | 13.7.1 |

## Python

| Name           | Value                                        |
|----------------|----------------------------------------------|
| Version        | 3.11.7                                       |
| Implementation | CPython                                      |
| Compiler       | GCC 9.4.0                                    |
| Executable     | /home/saf/code/misc/textual/.venv/bin/python |

## Operating System

| Name    | Value                                        |
|---------|----------------------------------------------|
| System  | Linux                                        |
| Release | 5.4.0-176-generic                            |
| Version | #196-Ubuntu SMP Fri Mar 22 16:46:39 UTC 2024 |

## Terminal

| Name                 | Value          |
|----------------------|----------------|
| Terminal Application | Terminator     |
| TERM                 | xterm-256color |
| COLORTERM            | truecolor      |
| FORCE_COLOR          | *Not set*      |
| NO_COLOR             | *Not set*      |

## Rich Console options

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

Copy link

We found the following entries in the FAQ which you may find helpful:

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

@slawek-es
Copy link
Author

I took a shot at diagnosing this issue, and the issue disappears if we modify the shutdown sequence of the input thread to loop a few times more to drain the mouse movement sequences out from the input descriptor:

class LinuxDriver(Driver):
    ...
    def run_input_thread(self) -> None:
        ...
        countdown = 3

        try:
            with open("/tmp/textual_out", "a") as f:
                while not self.exit_event.is_set() or countdown > 0:
                    selector_events = selector.select(0.1)
                    for _selector_key, mask in selector_events:
                        if mask & EVENT_READ:
                            rdd = read(fileno, 1024)
                            unicode_data = decode(rdd, final=self.exit_event.is_set())
                            for event in feed(unicode_data):
                                self.process_event(event)
                    if self.exit_event.is_set():
                        print(
                            f"Exit event set, countdown: {countdown}, data: {rdd if mask & EVENT_READ else None}",
                            file=f,
                        )
                        termios.tcflush(fileno, termios.TCIFLUSH)
                        rdd = ""
                        countdown -= 1
        finally:
            selector.close()

I'm getting the following output in /tmp/textual_out if I move the mouse while quitting the application:

Exit event set, countdown: 3, data: b'\x1b[<35;101;46M'
Exit event set, countdown: 2, data: b'\x1b[<35;111;45M'
Exit event set, countdown: 1, data: 

so it looks like new mouse movement control sequences may appear on the linux driver's terminal file descriptor, even though the control sequence for disabling mouse support is already sent by disable_input, and the descriptor's input queue is flushed. I'm not sure how to proceed from here: this "draining" mechanism works to resolve this issue, but it looks very odd that we would have to do it.

@TomJGooding
Copy link
Contributor

TomJGooding commented Apr 15, 2024

This issue does seem specific to certain terminal emulators, at least from running quick tests with:

  • xterm
  • urxvt
  • alacritty
  • terminator
  • wezterm
  • Xfce Terminal

@davep
Copy link
Contributor

davep commented Apr 15, 2024

FWIW I couldn't reproduce it with kitty on macOS.

@TomJGooding
Copy link
Contributor

@davep How about with macOS Terminal (just for confirmation as not one I'm able to check)?

@davep
Copy link
Contributor

davep commented Apr 15, 2024

Didn't test with anything else; I'll try and remember to have a look the next time I'm at a machine.

@TomJGooding
Copy link
Contributor

I've also now tested with Xfce Terminal which shows the same issue, which makes me wonder if perhaps specific to VTE-based terminal emulators? (Though this wouldn't explain issues with macOS terminals.)

@piankma
Copy link

piankma commented May 6, 2024

for more macOS terminal emulators:
issue can be reproduced on Hyper, but not on iTerm2 and Terminal.app

@willmcgugan
Copy link
Collaborator

I can't seem to reproduce it at all with the latest Textual (on macOS).

@willmcgugan
Copy link
Collaborator

Actually, I did manage to reproduce it after a few attempt on Hyper. This should help me track it down!

@Zatfer17
Copy link

Zatfer17 commented Jun 5, 2024

Happens the same to me on Windows PowerShell

@willmcgugan
Copy link
Collaborator

There were some recent changes to mouse handling and I think this issue is fixed (I can no longer reproduce it).

Feel free to re-open if anyone can reproduce with 0.69.0 or later.

Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@slawek-es
Copy link
Author

I can still reproduce this with five_by_five.py on latest main branch (commit 4f5a8ae). It seems I don't have permissions to re-open the ticket, and I don't want to create another one. Could one of the maintainers re-open this please?

@willmcgugan willmcgugan reopened this Jun 18, 2024
@TomJGooding
Copy link
Contributor

TomJGooding commented Jun 19, 2024

Has anyone managed to reproduce this apart from the five_by_five.py example?

EDIT: Never mind, after a number of attempts I was also able to reproduce with python3 -m textual in terminator.

@TomJGooding
Copy link
Contributor

TomJGooding commented Jun 20, 2024

I have noticed something in terminator after many attempts to reproduce this, but I'm not sure really how useful this is to diagnose the issue.

When the mouse escape sequences leak after app shutdown, I noticed in terminator that a light-bulb icon is briefly displayed in the title bar. You can see this in the original screen recording. This icon is apparently the default setting in terminator for the terminal bell.

Sorry for the noise if this was already obvious. I did try investigating exactly what would trigger the terminal bell without much success.

@slawek-es
Copy link
Author

Has anyone managed to reproduce this apart from the five_by_five.py example?

I can reproduce this fairly easily with the following app:

import asyncio

from textual.app import App, ComposeResult
from textual.widgets import Button, Header, Footer


class ReproApp(App[None]):

    CSS = """
    Screen {
        layout: grid;
        grid-size: 5;
    }

    Button {
        width: 1fr;
        height: 1fr;
    }
    """

    BINDINGS = [
        ("q", "quit", "Quit"),
    ]

    def __init__(self) -> None:
        super().__init__()
        self.dark = False

    def compose(self) -> ComposeResult:
        yield Header()
        for i in range(20):
            yield Button(f"Button {i}")
        yield Footer()


if __name__ == "__main__":
    asyncio.run(ReproApp().run_async())

This issue may be correlated with the amount of "stuff" that needs to happen as you move the mouse around, such as changing button highlights. I'm attaching a video where this reproduces for me three times in a row in Terminator; all I'm doing is wiggling the mouse while I press the q button.

control-seq-leak-2-2024-06-26_20.00.51.mp4

@TomJGooding
Copy link
Contributor

TomJGooding commented Jun 27, 2024

Sorry yes I edited my message after I later managed to reproduce this with the Textual demo (python -m textual) after a few attempts.

This issue may be correlated with the amount of "stuff" that needs to happen as you move the mouse around, such as changing button highlights.

That's my suspicion too, but using the demo or even a blank app without any widgets, I was very, very occasionally still able to reproduce by jiggling the mouse in empty space.

This issue does seem specific to certain terminal emulators, but I'm struggling to find any info on why mouse handling might be different.

Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

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 a pull request may close this issue.

6 participants