From 03f11eea7819ed8f2cd5f3a91dd40d75de5a6b3e Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Fri, 8 Nov 2024 19:32:00 +0800
Subject: [PATCH] Exclude letter hotkeys from Shift fallback

There is a fallback mechanism here designed to catch cases
where a hotkey is bound to a key that needs Shift to be
input (e.g. ? requires Shift on many non-US keyboards,
but the hotkey for that should be '?', not 'Shift+/'). For any key
where the two options are equally-valid "main" key (e.g. '6' and '^'
or '?' and '/' or ':' amd ';'), this is correct, as it allows
the hotkey to be simply the character in question.

Letters don't require this treatment - using this fallback
in the letter case means that a hotkey bound to 'B' will
also fire when 'Shift+B' is pressed, even when Shift+B is
bound to something else. In fact, it could even preempt the
real Shift+B hotkey.

Fixes: https://gitlab.com/kicad/code/kicad/-/issues/19093
---
 common/tool/action_manager.cpp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/common/tool/action_manager.cpp b/common/tool/action_manager.cpp
index 89fe1c0092..fa1bb20f76 100644
--- a/common/tool/action_manager.cpp
+++ b/common/tool/action_manager.cpp
@@ -48,7 +48,7 @@ ACTION_MANAGER::ACTION_MANAGER( TOOL_MANAGER* aToolManager ) :
         std::string groupName = "none";
 
         std::optional<TOOL_ACTION_GROUP> group = action->GetActionGroup();
-        
+
         if( group.has_value() )
         {
             groupID   = group.value().GetGroupID();
@@ -152,19 +152,21 @@ bool ACTION_MANAGER::RunHotKey( int aHotKey ) const
     // If no luck, try without Shift, to handle keys that require it
     // e.g. to get ? you need to press Shift+/ without US keyboard layout
     // Hardcoding ? as Shift+/ is a bad idea, as on another layout you may need to press a
-    // different combination
-    if( it == m_actionHotKeys.end() )
+    // different combination.
+    // This doesn't apply for letters, as we already handled case normalisation.
+    if( it == m_actionHotKeys.end() && !std::isalpha( key ) )
     {
         wxLogTrace( kicadTraceToolStack,
                     wxS( "ACTION_MANAGER::RunHotKey No actions found, searching with key: %s" ),
                     KeyNameFromKeyCode( key | ( mod & ~MD_SHIFT ) ) );
 
         it = m_actionHotKeys.find( key | ( mod & ~MD_SHIFT ) );
-
-        if( it == m_actionHotKeys.end() )
-            return false; // no appropriate action found for the hotkey
     }
 
+    // Still no luck, we're done without a match
+    if( it == m_actionHotKeys.end() )
+        return false; // no appropriate action found for the hotkey
+
     const std::list<TOOL_ACTION*>& actions = it->second;
 
     // Choose the action that has the highest priority on the active tools stack