From 14d068f73b870ed43b0aaabab871d1bf1f5a5b2b Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 6 Oct 2021 04:33:05 -0700 Subject: [PATCH] Fix crash and empty action in SUI Actions Page (#11427) ## Summary of the Pull Request Fixes two issues related to SUI's Actions page: 1. Crash when adding an action and setting key chord to one that is already taken - **Cause**: the new key binding that was introduced with the "Add new" button appears in `_KeyBindingList` that we're iterating over. This has no `CurrentKeys()`, resulting in a null pointer exception. - **Fix**: null-check it 2. There's an action that appears as being nameless in the dropdown - **Cause**: The culprit seems to be `MultipleActions`. We would register it, but it wouldn't have a name, so it would appear as a nameless option. - **Fix**: if it has no name, don't register it. This is also future-proof in that any new nameless actions won't be automatically added. Closes #10981 Part of #11353 --- src/cascadia/TerminalSettingsEditor/Actions.cpp | 4 ++-- src/cascadia/TerminalSettingsModel/ActionMap.cpp | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/Actions.cpp b/src/cascadia/TerminalSettingsEditor/Actions.cpp index 4f65c8cba..8e3a06a07 100644 --- a/src/cascadia/TerminalSettingsEditor/Actions.cpp +++ b/src/cascadia/TerminalSettingsEditor/Actions.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT license. #include "pch.h" @@ -369,7 +369,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { const auto kbdVM{ get_self(_KeyBindingList.GetAt(i)) }; const auto& otherKeys{ kbdVM->CurrentKeys() }; - if (keys.Modifiers() == otherKeys.Modifiers() && keys.Vkey() == otherKeys.Vkey()) + if (otherKeys && keys.Modifiers() == otherKeys.Modifiers() && keys.Vkey() == otherKeys.Vkey()) { return i; } diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index 6472245aa..1bd4aa390 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -94,15 +94,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static void RegisterShortcutAction(ShortcutAction shortcutAction, std::unordered_map& list, std::unordered_set& visited) { const auto actionAndArgs{ make_self(shortcutAction) }; - if (actionAndArgs->Action() != ShortcutAction::Invalid) + /*We have a valid action.*/ + /*Check if the action was already added.*/ + if (visited.find(Hash(*actionAndArgs)) == visited.end()) { - /*We have a valid action.*/ - /*Check if the action was already added.*/ - if (visited.find(Hash(*actionAndArgs)) == visited.end()) + /*This is an action that wasn't added!*/ + /*Let's add it if it has a name.*/ + if (const auto name{ actionAndArgs->GenerateName() }; !name.empty()) { - /*This is an action that wasn't added!*/ - /*Let's add it.*/ - const auto name{ actionAndArgs->GenerateName() }; list.insert({ name, *actionAndArgs }); } }