From e62a41c53a7e4a38e21fd0edfcd469398dbca4ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Wed, 15 Jul 2026 17:00:03 +0200 Subject: [PATCH] CmdPal / Settings: Prevent shortcut dialogs invocation from crashing the parent app (#49334) ## Summary of the Pull Request It looks like, when the stars align, changing the system theme can cause the ContentDialog to open multiple times, especially if the user repeatedly clicks the button that opens it. - Adds a ThreadStatic flag to ensure the dialog is opened only once. - Adds a try/catch guard in case my assumption about the flag is wrong. ## PR Checklist - [x] Closes: #49310 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .../ShortcutControl/ShortcutControl.xaml.cs | 37 ++++++++++++---- .../ShortcutControl/ShortcutControl.xaml.cs | 43 ++++++++++++++----- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/ShortcutControl.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/ShortcutControl.xaml.cs index 13d0588216..fc5774454c 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/ShortcutControl.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/ShortcutControl.xaml.cs @@ -4,6 +4,7 @@ using CommunityToolkit.Mvvm.Messaging; using CommunityToolkit.WinUI; +using ManagedCommon; using Microsoft.CmdPal.UI.Library; using Microsoft.CmdPal.UI.ViewModels.Settings; using Microsoft.PowerToys.Settings.UI.Helpers; @@ -28,6 +29,9 @@ public sealed partial class ShortcutControl : UserControl, IDisposable, IRecipie private bool _isActive; private bool disposedValue; + [ThreadStatic] + private static bool _isDialogOpen; + public string Header { get; set; } = string.Empty; public string Keys { get; set; } = string.Empty; @@ -404,16 +408,33 @@ public sealed partial class ShortcutControl : UserControl, IDisposable, IRecipie private async void OpenDialogButton_Click(object sender, RoutedEventArgs e) { - // c.Keys = null; - c.Keys = HotkeySettings?.GetKeysList() ?? new List(); + if (_isDialogOpen) + { + return; + } - // 92 means the Win key. The logic is: warning should be visible if the shortcut contains Alt AND contains Ctrl AND NOT contains Win. - // Additional key must be present, as this is a valid, previously used shortcut shown at dialog open. Check for presence of non-modifier-key is not necessary therefore - c.IsWarningAltGr = c.Keys.Contains("Ctrl") && c.Keys.Contains("Alt") && !c.Keys.Contains(92); + _isDialogOpen = true; + try + { + // c.Keys = null; + c.Keys = HotkeySettings?.GetKeysList() ?? new List(); - shortcutDialog.XamlRoot = this.XamlRoot; - shortcutDialog.RequestedTheme = this.ActualTheme; - await shortcutDialog.ShowAsync(); + // 92 means the Win key. The logic is: warning should be visible if the shortcut contains Alt AND contains Ctrl AND NOT contains Win. + // Additional key must be present, as this is a valid, previously used shortcut shown at dialog open. Check for presence of non-modifier-key is not necessary therefore + c.IsWarningAltGr = c.Keys.Contains("Ctrl") && c.Keys.Contains("Alt") && !c.Keys.Contains(92); + + shortcutDialog.XamlRoot = this.XamlRoot; + shortcutDialog.RequestedTheme = this.ActualTheme; + await shortcutDialog.ShowAsync(); + } + catch (Exception ex) + { + Logger.LogError("Failed to open shortcut dialog", ex); + } + finally + { + _isDialogOpen = false; + } } private void ShortcutDialog_Reset(ContentDialog sender, ContentDialogButtonClickEventArgs args) diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Controls/ShortcutControl/ShortcutControl.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/Controls/ShortcutControl/ShortcutControl.xaml.cs index ed328d64e9..7b749b0ef9 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Controls/ShortcutControl/ShortcutControl.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/Controls/ShortcutControl/ShortcutControl.xaml.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.Linq; using CommunityToolkit.WinUI; +using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Helpers; using Microsoft.PowerToys.Settings.UI.Library; using Microsoft.PowerToys.Settings.UI.Library.Telemetry.Events; @@ -42,6 +43,9 @@ namespace Microsoft.PowerToys.Settings.UI.Controls private bool _isActive; private bool disposedValue; + [ThreadStatic] + private static bool _isDialogOpen; + public string Header { get; set; } public string Keys { get; set; } @@ -670,20 +674,37 @@ namespace Microsoft.PowerToys.Settings.UI.Controls private async void OpenDialogButton_Click(object sender, RoutedEventArgs e) { - c.Keys = null; - c.Keys = HotkeySettings?.GetKeysList() ?? new List(); + if (_isDialogOpen) + { + return; + } - c.IgnoreConflict = IgnoreConflict; - c.HasConflict = hotkeySettings?.HasConflict ?? false; - c.ConflictMessage = hotkeySettings?.ConflictDescription; + _isDialogOpen = true; + try + { + c.Keys = null; + c.Keys = HotkeySettings?.GetKeysList() ?? new List(); - // 92 means the Win key. The logic is: warning should be visible if the shortcut contains Alt AND contains Ctrl AND NOT contains Win. - // Additional key must be present, as this is a valid, previously used shortcut shown at dialog open. Check for presence of non-modifier-key is not necessary therefore - c.IsWarningAltGr = c.Keys.Contains("Ctrl") && c.Keys.Contains("Alt") && !c.Keys.Contains(92); + c.IgnoreConflict = IgnoreConflict; + c.HasConflict = hotkeySettings?.HasConflict ?? false; + c.ConflictMessage = hotkeySettings?.ConflictDescription; - shortcutDialog.XamlRoot = this.XamlRoot; - shortcutDialog.RequestedTheme = this.ActualTheme; - await shortcutDialog.ShowAsync(); + // 92 means the Win key. The logic is: warning should be visible if the shortcut contains Alt AND contains Ctrl AND NOT contains Win. + // Additional key must be present, as this is a valid, previously used shortcut shown at dialog open. Check for presence of non-modifier-key is not necessary therefore + c.IsWarningAltGr = c.Keys.Contains("Ctrl") && c.Keys.Contains("Alt") && !c.Keys.Contains(92); + + shortcutDialog.XamlRoot = this.XamlRoot; + shortcutDialog.RequestedTheme = this.ActualTheme; + await shortcutDialog.ShowAsync(); + } + catch (Exception ex) + { + Logger.LogError("Failed to open shortcut dialog", ex); + } + finally + { + _isDialogOpen = false; + } } private void C_ResetClick(object sender, RoutedEventArgs e)