Fix an infinite loop when pressing alt

[zadjii-msft]

As discussed in #14051 (comment)

regressed in #15189

[lhecker]

But... what's the point of assigning to focusedObject now? Doesn't this break this new code for Alt+Space?

[DHowett]

alt-space seems to work on the build we just ran... huh

[zadjii-msft]

wait what is going on, that totally fixed it before. Hmmmmmmmm

[zadjii-msft]

WHAT

Open Terminal Stable.
Go to the sui. Make sure you can scroll the page.
Tap alt.
Bye bye scrolling

[lhecker]

wtf

[zadjii-msft]

okay what

There's something REAL fucky with the direct key handler, on the TerminalPage. Cause:

• Open the right click context menu
• tap alt
• press arrow key:
• a wild system menu appears

[zadjii-msft]

@michalnpl were you seeing an actual infinite loop in the debugger? Or did it just seem like the Terminal was stuck in some sort of UI loop?

With this PR, and even on 1.16, I'm seeing all this weird behavior where tapping alt, without a key, leaves the terminal in this weird state where the next keypress seems to go to... nowhere? The system menu? But you need to have like, a popup / flyout of some sort open. The Command Palette repros it, the control context menu, the Settings UI (of all things)

I'm gonna at least mark as a draft for now.

drafted

[michalnpl]

@zadjii-msft I've seen the infinite loop in the debugger. When I was "fixing" it locally, I had to add more than one break to make it break out in all cases. It wouldn't hang anymore, but the alt key bindings wouldn't bubble outside the settings page anyway.

[zadjii-msft]

If you have a better idea for a fix, I'm all ears. I'm sitting at like

index 56e87cd83..67a72996c 100644
--- a/src/cascadia/TerminalApp/TerminalWindow.cpp
+++ b/src/cascadia/TerminalApp/TerminalWindow.cpp
@@ -871,7 +871,15 @@ namespace winrt::TerminalApp::implementation
                         // We were unable to find a focused object. Default to the xaml root so that the alt+space menu still works.
                         if (!focusedObject)
                         {
-                            focusedObject = _root.try_as<IInspectable>();
+                            // focusedObject = _root.try_as<IInspectable>();^M
+                            if (auto keyListener{ _root.try_as<IDirectKeyListener>() })^M
+                            {^M
+                                if (keyListener.OnDirectKeyEvent(vkey, scanCode, down))^M
+                                {^M
+                                    return true;^M
+                                }^M
+                            }^M
+^M
                             break;
                         }
                     }

right now, and that feels wrong too. I don't think I've got my head around this one 🙃

[michalnpl]

@zadjii-msft I have this

under branch

https://github.com/michalnpl/terminal/tree/michalnpl/Entering_the_Settings_menu_in_fullscreen_can_become_stuck

but this is not a fix. This merely stops the hang. I am still trying to wrap my head around why there is a custom routine for bubbling routed events. :)

[DHowett]

I am still trying to wrap my head around why there is a custom routine for bubbling routed events. :)

Oh boy, you're gonna love this.

F7 (make) is intercepted high in the XAML stack and never forwarded on to applications. We added a custom event router in #4807 for it because otherwise it would display a dialog asking if you wanted to "enable caret browsing".

Alt (break) just never propagates to applications either, for no discernible reason (except that the XAML code eats it). We put it in the same routing regime as F7 in #6461 because console applications weren't receiving it.

[zadjii-msft]

Okay, so I do not understand the whole "alt then release then later type a space opens the system menu" thing. That seems like some other XAML islands thing.

But at least this:

• works for alt+space in the SUI
• Doesn't hard hang when tapping alt in the SUI.

[lhecker]

I'm not sure if it would make more sense, but technically you can put this fallback outside the loop, at the end of this function. I guess it would make some sense, given that it only accesses _root and has nothing to do with the actual UI tree anymore.