Repository: microsoft/terminal (language: cpp)

Issue:
Preview: Selecting fonts
### Windows Terminal version

1.22.2702.0

### Windows build number

10.0.22635.0

### Other Software

I actually have more than one problem with the font selection in Windows Terminal Preview.

1. When trying to select fonts in Settings -> Profiles: Defaults -> Appearance: Font Face ... it crashes to desktop if I hit "Enter" while _typing in the font selection text box_

2. When typing in the font selection box, if I stop typing and select from the list, and then hit save, it reverts to only the part I typed (and then saves) resulting an error popup that the font doesn't exist.

### Steps to reproduce

1. Open Settings -> Profiles: Defaults -> Appearance ... and click in the Font Face text box, and press Enter.

Then reopen Terminal and try again ... 

1. Type part of a font name in the Font Face box
2. Stop typing and select your font from the list
3. Hit save



### Expected Behavior

I should be able to use my keyboard and mouse together for settings

### Actual Behavior

Terminal crashes and pops up error messages.
Here's a video of the second problem, in case I'm confusing

https://github.com/user-attachments/assets/8c6b5cd9-0c0f-43e0-b87b-b019976ef910



Discussion / hints:

Fix patch:
diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt
index 3a6ca5cf14e..43dc358050f 100644
--- a/.github/actions/spelling/expect/expect.txt
+++ b/.github/actions/spelling/expect/expect.txt
@@ -1460,6 +1460,7 @@ QWER
 Qxxxxxxxxxxxxxxx
 qzmp
 RAII
+Ralph
 RALT
 rasterbar
 rasterfont
@@ -2008,6 +2009,7 @@ WHelper
 wic
 WIDTHSCROLL
 Widthx
+Wiggum
 wil
 WImpl
 WINAPI
diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.cpp b/src/cascadia/TerminalSettingsEditor/Appearances.cpp
index 3fcbfb58a2c..4bab8d69a84 100644
--- a/src/cascadia/TerminalSettingsEditor/Appearances.cpp
+++ b/src/cascadia/TerminalSettingsEditor/Appearances.cpp
@@ -1017,22 +1017,18 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
     void Appearances::FontFaceBox_LostFocus(const IInspectable& sender, const RoutedEventArgs&)
     {
-        const auto appearance = Appearance();
-        const auto fontSpec = sender.as<AutoSuggestBox>().Text();
+        _updateFontName(sender.as<AutoSuggestBox>().Text());
+    }
 
-        if (fontSpec.empty())
-        {
-            appearance.ClearFontFace();
-        }
-        else
+    void Appearances::FontFaceBox_QuerySubmitted(const AutoSuggestBox& sender, const AutoSuggestBoxQuerySubmittedEventArgs& args)
+    {
+        // When pressing Enter within the input line, this callback will be invoked with no suggestion.
+        const auto font = unbox_value_or<Editor::Font>(args.ChosenSuggestion(), nullptr);
+        if (!font)
         {
-            appearance.FontFace(fontSpec);
+            return;
         }
-    }
 
-    void Appearances::FontFaceBox_SuggestionChosen(const AutoSuggestBox& sender, const AutoSuggestBoxSuggestionChosenEventArgs& args)
-    {
-        const auto font = unbox_value<Editor::Font>(args.SelectedItem());
         const auto fontName = font.Name();
         auto fontSpec = sender.Text();
 
@@ -1049,6 +1045,22 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
 
         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.
+        // Also, this causes the first tab in the application to be focused, so when you press Enter it'll switch tabs.
+        //
+        // You can't just assign focus back to the AutoSuggestBox, because the FocusState() within the GotFocus event handler
+        // contains random values. This prevents us from avoiding the IsSuggestionListOpen(true) in our GotFocus event handler.
+        // 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);
     }
 
     void Appearances::FontFaceBox_TextChanged(const AutoSuggestBox& sender, const AutoSuggestBoxTextChangedEventArgs& args)
@@ -1071,6 +1083,19 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         _updateFontNameFilter(filter);
     }
 
+    void Appearances::_updateFontName(hstring fontSpec)
+    {
+        const auto appearance = Appearance();
+        if (fontSpec.empty())
+        {
+            appearance.ClearFontFace();
+        }
+        else
+        {
+            appearance.FontFace(std::move(fontSpec));
+        }
+    }
+
     void Appearances::_updateFontNameFilter(std::wstring_view filter)
     {
         if (_fontNameFilter != filter)
diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.h b/src/cascadia/TerminalSettingsEditor/Appearances.h
index 27a58073a8f..2f5bfb7df5a 100644
--- a/src/cascadia/TerminalSettingsEditor/Appearances.h
+++ b/src/cascadia/TerminalSettingsEditor/Appearances.h
@@ -179,7 +179,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
         void FontFaceBox_GotFocus(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& e);
         void FontFaceBox_LostFocus(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& e);
-        void FontFaceBox_SuggestionChosen(const winrt::Windows::UI::Xaml::Controls::AutoSuggestBox&, const winrt::Windows::UI::Xaml::Controls::AutoSuggestBoxSuggestionChosenEventArgs&);
+        void FontFaceBox_QuerySubmitted(const winrt::Windows::UI::Xaml::Controls::AutoSuggestBox&, const winrt::Windows::UI::Xaml::Controls::AutoSuggestBoxQuerySubmittedEventArgs&);
         void FontFaceBox_TextChanged(const winrt::Windows::UI::Xaml::Controls::AutoSuggestBox&, const winrt::Windows::UI::Xaml::Controls::AutoSuggestBoxTextChangedEventArgs&);
         void DeleteFontKeyValuePair_Click(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& e);
         safe_void_coroutine BackgroundImage_Click(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& e);
@@ -214,9 +214,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         Windows::Foundation::Collections::IObservableVector<winrt::hstring> _FontFeaturesNames;
         std::wstring _fontNameFilter;
         bool _ShowAllFonts = false;
+        bool _suppressFontFaceBoxList = false;
 
         static void _ViewModelChanged(const Windows::UI::Xaml::DependencyObject& d, const Windows::UI::Xaml::DependencyPropertyChangedEventArgs& e);
 
+        void _updateFontName(winrt::hstring fontSpec);
         void _updateFontNameFilter(std::wstring_view filter);
         void _updateFilteredFontList();
         void _UpdateBIAlignmentControl(const int32_t val);
diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.xaml b/src/cascadia/TerminalSettingsEditor/Appearances.xaml
index a3f643c53c3..78d2c7f7f5a 100644
--- a/src/cascadia/TerminalSettingsEditor/Appearances.xaml
+++ b/src/cascadia/TerminalSettingsEditor/Appearances.xaml
@@ -202,7 +202,8 @@
             </local:SettingContainer>
 
             <!--  Font Face  -->
-            <local:SettingContainer x:Uid="Profile_FontFace"
+            <local:SettingContainer x:Name="FontFaceContainer"
+                                    x:Uid="Profile_FontFace"
                                     ClearSettingValue="{x:Bind Appearance.ClearFontFace}"
                                     HasSettingValue="{x:Bind Appearance.HasFontFace, Mode=OneWay}"
                                     SettingOverrideSource="{x:Bind Appearance.FontFaceOverrideSource, Mode=OneWay}"
@@ -214,10 +215,11 @@
                                     ItemTemplate="{StaticResource FontFaceComboBoxItemTemplate}"
                                     ItemsSource="{x:Bind FilteredFontList, Mode=OneWay}"
                                     LostFocus="FontFaceBox_LostFocus"
-                                    SuggestionChosen="FontFaceBox_SuggestionChosen"
+                                    QuerySubmitted="FontFaceBox_QuerySubmitted"
                                     Text="{x:Bind Appearance.FontFace, Mode=OneWay}"
                                     TextBoxStyle="{StaticResource TextBoxSettingStyle}"
-                                    TextChanged="FontFaceBox_TextChanged" />
+                                    TextChanged="FontFaceBox_TextChanged"
+                                    UpdateTextOnSelect="False" />
                     <CheckBox x:Name="ShowAllFontsCheckbox"
                               x:Uid="Profile_FontFaceShowAllFonts"
                               IsChecked="{x:Bind ShowAllFonts, Mode=TwoWay}" />


Test patch: