Hitting ALT-F4 in the settings UI doesn't close WT

[sylveon]

Environment

Windows build number: 10.0.19042.0
Windows Terminal version: 1.6.10272.0

Steps to reproduce

1. Open the settings UI
2. Hit ALT-F4

Expected behavior

The window closes, prompting the user to close all tabs if there are more than 1

Actual behavior

Nothing happens

More details

This is seemingly because the current method to catch ALT-F4 uses a keyboard accelerator on the Terminal control itself (which doesn't have keyboard focus when the settings are opened, this is indeed reproducible when other XAML elements have the keyboard focus in a terminal tab).

A more reliable method would be to use code like the following before calling the XAML island's PreTranslateMessage:

// prevent XAML islands from capturing ALT-F4 because of
// https://github.com/microsoft/microsoft-ui-xaml/issues/2408
if (msg.message == WM_SYSKEYDOWN && msg.wParam == VK_F4)
{
	SendMessage(GetAncestor(msg.hwnd, GA_ROOT), msg.message, msg.wParam, msg.lParam);
	return FALSE;
}

and then in the hosting window, handle WM_CLOSE normally, transmitting the close request to the XAML code.

I can make a PR for this, provided that always binding ALT-F4 to close window is the desired path (because the code above will prevent assignment of a custom keybinding to ALT-F4)

[Don-Vito]

@sylveon - the current mechanism of allowing actions in settings UI is implemented in TerminalPage::_SUIPreviewKeyDownHandler where we decide what actions to dispatch:

if (actionAndArgs && (actionAndArgs.Action() == ShortcutAction::CloseTab || actionAndArgs.Action() == ShortcutAction::NextTab || actionAndArgs.Action() == ShortcutAction::PrevTab || actionAndArgs.Action() == ShortcutAction::ClosePane))
{
    _actionDispatch->DoAction(actionAndArgs);
    e.Handled(true);
}

The immediate workaround is to add ShortcutAction::CloseWindow to the list of the conditions.

The discussion about better solution is here: #8767
Consider moving the discussion there.

[sylveon]

There are cases where ALT-F4 isn't triggered besides the settings UI - for example if you click on the current active tab, the terminal loses focus (as shown by the cursor not flashing anymore) and ALT-F4 stops working too.

[Don-Vito]

There are cases where ALT-F4 isn't triggered besides the settings UI - for example if you click on the current active tab, the terminal loses focus (as shown by the cursor not flashing anymore) and ALT-F4 stops working too.

You are correct. This happens because the TabView is getting focus on click, introducing tons of bugs other than keybinding not working: e.g., you cannot type and you have no clue why 😄. We can potentially solve the keybindings on TabView by adding KeyHandler to the TabView as well.

I am not against finding a more robust way of doing it on the process level, but not sure if it worth it without preserving custom bindings. Of course this is just my humble opinion 😊

[sylveon]

In my opinion ALT-F4 shouldn't be something custom, it's a shortcut that's practically system-wide so I would be okay with hardcoding it - the code I've put in the original comment works reliably for me.

[Don-Vito]

In my opinion ALT-F4 shouldn't be something custom, it's a shortcut that's practically system-wide so I would be okay with hardcoding it - the code I've put in the original comment works reliably for me.

I do not disagree - I like your idea 😊

Just raising some concerns, because currently CloseWindow command is handled as regular command. Which means it can be bound to keys, and its keys theoretically should be unboundable.

[zadjii-msft]

You know, I think at the end of the day this is just #8767. closeWindow is another one of those actions that's viable in the settings UI. Moving the focus into the tab itself isn't something that really should be possible at all, but that's a lot trickier. For that you're looking at #6680, #3609.

/dup #8767

Thanks!

[ghost]

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!