Key events should not be sent to an inactive screen

[darrenburns]

Key events get sent to non-active screens for a short period on calling dismiss, before the screen has been completely dismissed. Most people won't expect this and may be doing things in their code which assumes the screen is active - leading to crashes.

We should ensure key events are only delivered to the screen while it remains active.

Edit:

from textual.app import App, ComposeResult
from textual.screen import Screen
from textual.widgets import Label, Select

class MyScreen(Screen):
    def compose(self):
        yield Label("Screen")
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])
        yield Select([("bar", "bar")])

    async def on_key(self):
        await self.dismiss()

class MyApp(App):
    def compose(self):
        yield Label("App")
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])
        yield Select([("foo", "foo")])

    def on_key(self):
        self.push_screen(MyScreen())

app = MyApp()
app.run()

[merriam]

So, your example is "press a key twice and crash with 'TypeError: object NoneType...`on the self.dismiss()?

To clarify, you can remove all the Select controls.

The issue is using await on a non-async call. Instead of await self.dismiss(), just use self.dismiss().

Ref: https://stackoverflow.com/questions/56872323/typeerror-object-nonetype-cant-be-used-in-await-expression

[darrenburns]

Yeah, the example is actually Will's edit :)

Sorry, this issue is kind of confusing and without context, as it was a quick note I made for Will based on something we discussed last week.

The issue is that key events get sent to non-active screens for a short period, before the screen has been completely dismissed. Most people won't expect this and may be doing things in their code which assumes the screen is active - leading to crashes.

[merriam]

The claimed sequence is:

• self.dismiss() is called and returns.
• a key is pressed
• an on_key() handler is called inside the dismissed screen

So, something more like:

     def do_dismiss():
            self.in_dismissal = True
            self.dismiss()

      def on_key(self):
            if self.in_dismissal:
                 print("Error")

Then on a mouse move or timer, dismiss the window and pop it back, while pounding the keyboard and we should eventually hit "error"?

[willmcgugan]

The original issue was a deadlock which tripped deadlock detection. That no longer occurs, because we removed the deadlock detection after improving the way widgets are removed. However the deadlock remains in the above code.

Essentially, the call to await dismiss() on the screen was awaiting for itself to be removed. The solution would be to not await it, but that it's not great developer experience. I think either we report a helpful error, or make it work somehow. I'm working on that at the moment.

There is some other behavior that might need explaining, because it confused me initially. When you press a key on MyScreen, both on_key methods fire. This is because the event bubbles to the App. Obvious in retrospect.

[willmcgugan]

Hopefully this is clear...

[github-actions[bot]]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.