QCocoaNSMenuDelegate: Improve key-equivalent logic

By using NSEvent.characters instead of NSEvent.charactersIgnoringModifiers,
we may miss sending ShortcutOverride events.

For example, when the user presses Cmd-Opt-o, characters will be "ø"
(on a US keyboard layout) and therefore we'll be looking for the wrong
key-equivalent among the menu items. We only fall back on the modified
string when the search on the unmodified string fails.

As and addendum, we also skip any submenu when doing the key search.
This is not necessary since each menu delegate will get called eventually.

Change-Id: Id793315293a02c99e99d793ad812cff7b4a47821
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@qt.io>
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>

src/plugins/platforms/cocoa/qcocoansmenu.h

@@ -64,7 +64,9 @@ typedef QPointer<QCocoaMenu> QCocoaMenuPointer;
 
 + (instancetype)sharedMenuDelegate;
 
-- (NSMenuItem *)findItem:(NSMenu *)menu forKey:(NSString *)key forModifiers:(NSUInteger)modifier;
+- (NSMenuItem *)findItemInMenu:(NSMenu *)menu
+                        forKey:(NSString *)key
+                     modifiers:(NSUInteger)modifiers;
 
 - (BOOL)validateMenuItem:(NSMenuItem *)item; // NSMenuValidation
 

src/plugins/platforms/cocoa/qcocoansmenu.mm

@@ -197,12 +197,25 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
 
     CHECK_MENU_CLASS(menu);
 
-    // Change the private unicode keys to the ones used in setting the "Key Equivalents"
-    NSString *characters = qt_mac_removePrivateUnicode([event characters]);
     // Interested only in Shift, Cmd, Ctrl & Alt Keys, so ignoring masks like, Caps lock, Num Lock ...
-    const NSUInteger mask = NSShiftKeyMask | NSControlKeyMask | NSCommandKeyMask | NSAlternateKeyMask;
+    static const NSUInteger mask = NSShiftKeyMask | NSControlKeyMask | NSCommandKeyMask | NSAlternateKeyMask;
-    if (NSMenuItem *menuItem = [self findItem:menu forKey:characters forModifiers:([event modifierFlags] & mask)]) {
+
-        if (!menuItem.target) {
+    // Change the private unicode keys to the ones used in setting the "Key Equivalents"
+    NSString *characters = qt_mac_removePrivateUnicode(event.charactersIgnoringModifiers);
+    const auto modifiers = event.modifierFlags & mask;
+    NSMenuItem *keyEquivalentItem = [self findItemInMenu:menu
+                                                  forKey:characters
+                                               modifiers:modifiers];
+    if (!keyEquivalentItem) {
+        // Maybe the modified character is what we're looking for after all
+        characters = qt_mac_removePrivateUnicode(event.characters);
+        keyEquivalentItem = [self findItemInMenu:menu
+                                          forKey:characters
+                                       modifiers:modifiers];
+    }
+
+    if (keyEquivalentItem) {
+        if (!keyEquivalentItem.target) {
             // This item was modified by QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder
             // and it looks like we're running a modal session for NSOpenPanel/NSSavePanel.
             // QCocoaFileDialogHelper is actually the only place we use this and we run NSOpenPanel modal
@@ -211,7 +224,7 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
             // and do not touch the Qt's focusObject (which is different from some native view
             // having a focus inside NSSave/OpenPanel.
             *target = nil;
-            *action = menuItem.action;
+            *action = keyEquivalentItem.action;
             return YES;
         }
 
@@ -249,25 +262,32 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
             }
         }
     }
+
     return NO;
 }
 
-- (NSMenuItem *)findItem:(NSMenu *)menu forKey:(NSString *)key forModifiers:(NSUInteger)modifier
+- (NSMenuItem *)findItemInMenu:(NSMenu *)menu
+                        forKey:(NSString *)key
+                     modifiers:(NSUInteger)modifiers
 {
-    for (NSMenuItem *item in [menu itemArray]) {
+    // Find an item in 'menu' that has the same key equivalent as specified by
-        if (![item isEnabled] || [item isHidden] || [item isSeparatorItem])
+    // 'key' and 'modifiers'. We ignore disabled, hidden and separator items.
-            continue;
+    // In a similar fashion, we don't need to recurse into submenus because their
-        if ([item hasSubmenu]) {
+    // delegate will have [menuHasKeyEquivalent:...] invoked at some point.
-            if (NSMenuItem *nested = [self findItem:[item submenu] forKey:key forModifiers:modifier])
-                return nested;
-        }
 
-        NSString *menuKey = [item keyEquivalent];
+    for (NSMenuItem *item in menu.itemArray) {
-        if (menuKey
+        if (!item.enabled || item.hidden || item.separatorItem)
-            && NSOrderedSame == [menuKey compare:key]
+            continue;
-            && modifier == [item keyEquivalentModifierMask])
+
+        if (item.hasSubmenu)
+            continue;
+
+        NSString *menuKey = item.keyEquivalent;
+        if (menuKey && NSOrderedSame == [menuKey compare:key]
+            && modifiers == item.keyEquivalentModifierMask)
                 return item;
     }
+
     return nil;
 }