mirror of
https://github.com/microsoft/terminal.git
synced 2026-07-08 18:16:28 +00:00
Fix font face keyboard selection crash (#20255)
## Summary Fixes a crash in the Settings UI font face control when selecting a font suggestion with the keyboard and pressing Enter. The font face `AutoSuggestBox` path was explicitly unfocusing the box after a chosen suggestion was submitted. During the keyboard suggestion commit path, that `Focus(FocusState::Unfocused)` call can trigger a WinUI/XAML crash before the settings model update completes. This change avoids explicitly unfocusing the `AutoSuggestBox` in the chosen-suggestion path and instead commits the selected font and moves focus to the parent container. Fixes #20245. ## Validation Manually validated with a local `CascadiaPackage` Debug x64 build: - Reproduced the crash before the fix. - Opened Settings → Defaults → Appearance → Font face. - Typed a partial font name. - Used arrow keys to select a suggestion. - Pressed Enter. - Verified Terminal no longer crashes. - Verified the selected font is applied and Save works.
This commit is contained in:
@@ -12,6 +12,7 @@
|
||||
#include "Appearances.g.cpp"
|
||||
|
||||
using namespace winrt::Windows::UI::Text;
|
||||
using namespace winrt::Windows::UI::Core;
|
||||
using namespace winrt::Windows::UI::Xaml;
|
||||
using namespace winrt::Windows::UI::Xaml::Controls;
|
||||
using namespace winrt::Windows::UI::Xaml::Data;
|
||||
@@ -1221,8 +1222,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
|
||||
fontSpec = fontName;
|
||||
}
|
||||
|
||||
sender.Text(fontSpec);
|
||||
|
||||
// Normally we'd just update the model property in LostFocus above, but because WinUI is the Ralph Wiggum
|
||||
// among the UI frameworks, it raises the LostFocus event _before_ the QuerySubmitted event.
|
||||
// So, when you press Save, the model will have the wrong font face string, because LostFocus was raised too early.
|
||||
@@ -1233,11 +1232,20 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
|
||||
// You can't just do IsSuggestionListOpen(false) either, because you can show the list with that property but not hide it.
|
||||
// So, we update the model manually and assign focus to the parent container.
|
||||
//
|
||||
// BUT you can't just focus the parent container, because of a weird interaction with AutoSuggestBox where it'll refuse to lose
|
||||
// focus if you picked a suggestion that matches the current fontSpec. So, we unfocus it first and then focus the parent container.
|
||||
_updateFontName(fontSpec);
|
||||
sender.Focus(FocusState::Unfocused);
|
||||
FontFaceContainer().Focus(FocusState::Programmatic);
|
||||
// Queue the selected-suggestion commit so AutoSuggestBox can finish processing Enter before we change its text/model.
|
||||
// Do not manually unfocus the AutoSuggestBox here. Its Focus(FocusState::Unfocused) path crashes during keyboard commits.
|
||||
Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis{ get_weak() }, weakSender{ winrt::make_weak(sender) }, fontSpec{ std::move(fontSpec) }]() {
|
||||
if (const auto self{ weakThis.get() })
|
||||
{
|
||||
if (const auto box{ weakSender.get() })
|
||||
{
|
||||
box.Text(fontSpec);
|
||||
}
|
||||
|
||||
self->_updateFontName(fontSpec);
|
||||
self->FontFaceContainer().Focus(FocusState::Programmatic);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
void Appearances::FontFaceBox_TextChanged(const AutoSuggestBox& sender, const AutoSuggestBoxTextChangedEventArgs& args)
|
||||
|
||||
Reference in New Issue
Block a user