cocoabits / MASShortcut

Modern framework for managing global keyboard shortcuts compatible with Mac App Store. More details:
http://blog.shpakovski.com/2012/07/global-keyboard-shortcuts-in-cocoa.html
BSD 2-Clause "Simplified" License
1.52k stars 220 forks source link

Allow 'Services' menu items to pass the shortcut validation. #174

Closed GetToSet closed 2 years ago

GetToSet commented 2 years ago

Shift+Cmd+A is a default shortcut for a core functionary of our app.

I got an issue from our user that if they set it to another shortcut, then clear it, they'll not be able to set it back.

Screen Shot 2022-04-13 at 21 05 09

This PR excludes -[NSApp servicesMenu] from -[MASShortcutValidator isShortcut:alreadyTakenInMenu:explanation:] check. Which prevents the user from being confused when they can't find the conflicting menu item. It's generally fine to have a custom shortcut that overrides the 'Services' shortcut.

GetToSet commented 2 years ago

The 'Services' menu seems to be loaded lazily. When the app initially launched, -[NSApp servicesMenu] is returned with no items.

2022-04-14 12:52:36.366651+0800 Demo[31001:344278] <NSMenu: 0x600000a96180>
Title: Services
Supermenu: 0x600000a96140 (Demo), autoenable: YES
Items:     (
)

Therefore MASShortcut lets shortcuts like Shift+Cmd+A go through the validation on a clean launch.

-[_NSServicesMenuUpdater updateMenu:withEvent:withFlags:] is called when AppKit populates the 'Services' menu. A symbol breakpoint can conclude that AppKit updates the 'Services' menu in the following cases:

  1. When the user expands the 'Services' menu item.
  2. On certain keystrokes, which normally happen when the user sets a shortcut with MASShortcutView, the call stack looks like this: 2022-04-14 10 12 59

-[NSApp servicesMenu] returns a whole list of menu items(note some menu item is hidden) when populated:

<NSMenu: 0x600000a96180>
    Title: Services
    Open bounds: [t=1189, l=205, b=948, r=440]
    Supermenu: 0x600000a96140 (Demo), autoenable: YES
    Items:     (
        "<NSMenuItem: 0x6000034fe060 Add to Music as a Spoken Track, hidden>",
        "<NSMenuItem: 0x6000034ff3a0 Add to SnippetsLab, hidden>",
        "<NSMenuItem: 0x6000034ff090 Compare Text in Kaleidoscope, hidden>",
        "<NSMenuItem: 0x6000034fe7d0 Convert Text to Full Width, hidden>",
        "<NSMenuItem: 0x6000034fe1b0 Convert Text to Half Width, hidden>",
        "<NSMenuItem: 0x6000034ff560 Convert Text to Simplified Chinese, ke='Control-Option-Shift-Command-C', hidden>",
        "<NSMenuItem: 0x6000034ff100 Convert Text to Traditional Chinese, ke='Control-Shift-Command-C', hidden>",
        "<NSMenuItem: 0x6000034ff170 Create Snippet in Dash, hidden>",
        "<NSMenuItem: 0x6000034ff480 Make New Sticky Note, ke='Shift-Command-Y', hidden>",
        "<NSMenuItem: 0x6000034ff6b0 Open, hidden>",
        "<NSMenuItem: 0x6000034ff720 Show in Finder, hidden>",
        "<NSMenuItem: 0x6000034ff790 Show Info in Finder, hidden>",
        "<NSMenuItem: 0x6000034ff800 Open man Page in Terminal, ke='Shift-Command-M', hidden>",
        "<NSMenuItem: 0x6000034fee60 Search man Page Index in Terminal, ke='Shift-Command-A', hidden>",
        "<NSMenuItem: 0x6000034ff950 Send to Boop, ke='Control-Option-Command-B', hidden>",
        "<NSMenuItem: 0x6000034ff870 Show Map, hidden>",
        "<NSMenuItem: 0x6000034fe3e0 Compare Images in Kaleidoscope, hidden>",
        "<NSMenuItem: 0x6000034ff4f0 ImageOptimize, hidden>",
        "<NSMenuItem: 0x6000034ff8e0 Set Desktop Picture, hidden>",
        "<NSMenuItem: 0x6000034ff410 Compare Files, hidden>",
        "<NSMenuItem: 0x6000034ffb10 Compare Folders, hidden>",
        "<NSMenuItem: 0x6000034ffaa0 Select Left File for Compare, hidden>",
        "<NSMenuItem: 0x6000034ffa30 Select Left Folder for Compare, hidden>",
        "<NSMenuItem: 0x6000034ffb80 Compress using Keka, ke='Control-Shift-C', hidden>",
        "<NSMenuItem: 0x6000034ffc60 Extract using Keka, ke='Control-Shift-X', hidden>",
        "<NSMenuItem: 0x6000034ffcd0 Send to Keka, ke='Control-Shift-K', hidden>",
        "<NSMenuItem: 0x6000034ffbf0 Encode Selected Audio Files, hidden>",
        "<NSMenuItem: 0x6000034ffd40 Encode Selected Video Files, hidden>",
        "<NSMenuItem: 0x6000034ffdb0 Folder Actions Setup\U2026, hidden>",
        "<NSMenuItem: 0x6000034ffe90 New iTerm2 Tab Here, hidden>",
        "<NSMenuItem: 0x6000034fff70 New iTerm2 Window Here, hidden>",
        "<NSMenuItem: 0x6000034ff1e0 New Terminal at Folder, hidden>",
        "<NSMenuItem: 0x6000034fa290 New Terminal Tab at Folder, hidden>",
        "<NSMenuItem: 0x6000034fbf70 Open File in CotEditor, hidden>",
        "<NSMenuItem: 0x6000034fa220 Open in Kaleidoscope, hidden>",
        "<NSMenuItem: 0x6000034fadf0 Look Up in Dash, hidden>",
        "<NSMenuItem: 0x6000034b9ab0 Look Up in Dictionary, hidden>",
        "<NSMenuItem: 0x6000034b9a40 Search With Google, ke='Shift-Command-L', hidden>",
        "<NSMenuItem: 0x6000034b99d0 New Email To Address, hidden>",
        "<NSMenuItem: 0x6000034b9960 New Email With Selection, hidden>",
        "<NSMenuItem: 0x6000034b98f0 Add to Reading List, hidden>",
        "<NSMenuItem: 0x6000034b9880 Open in Sourcetree, hidden>",
        "<NSMenuItem: 0x6000034b9810 Open URL, hidden>",
        "<NSMenuItem: 0x6000034b9110 >",
        "<NSMenuItem: 0x6000034b9180 Development>",
        "<NSMenuItem: 0x6000034b97a0 Activity Monitor>",
        "<NSMenuItem: 0x6000034b9730 Allocations & Leaks>",
        "<NSMenuItem: 0x6000034b96c0 File Activity>",
        "<NSMenuItem: 0x6000034b9650 System Trace>",
        "<NSMenuItem: 0x6000034b95e0 Open With Apparency, hidden>",
        "<NSMenuItem: 0x6000034b9570 Time Profile Active Application>",
        "<NSMenuItem: 0x6000034b9500 Time Profile App Under Mouse>",
        "<NSMenuItem: 0x6000034b9490 Time Profile Entire System>",
        "<NSMenuItem: 0x6000034b9420 Toggle Instruments Recording>",
        "<NSMenuItem: 0x6000034b93b0 No Services Apply, hidden>",
        "<NSMenuItem: 0x6000034b8f50 >",
        "<NSMenuItem: 0x6000034b9340 Services Preferences\U2026>"
    )

Most menu items with a key equivalent only appear when certain conditions are met (such as the user selects some text). MASShortcut warns of a shortcut conflict when the shortcut matches one of these menu items.

When the user tries to find the conflicting menu item, they have no idea where it resides. The correct behavior should allow a shortcut to pass the validation if the user can't see a corresponding menuItem.

NSMenuItems normally don't respond to their key equivalents when hidden, except for allowsKeyEquivalentWhenHidden set to YES.

Initially my fix code was written like this:

- (BOOL) isShortcut: (MASShortcut*) shortcut alreadyTakenInMenu: (NSMenu*) menu explanation: (NSString**) explanation
{
    NSString *keyEquivalent = [shortcut keyCodeStringForKeyEquivalent];
    NSEventModifierFlags flags = [shortcut modifierFlags];

    for (NSMenuItem *menuItem in menu.itemArray) {
        if (menuItem.hasSubmenu && [self isShortcut:shortcut alreadyTakenInMenu:[menuItem submenu] explanation:explanation]) return YES;

// ---
        if ([menuItem isHidden]) {
            continue;
        }
// ---
...

After some debugging, it won't 100% solve the problem. In some cases, a menuItem of -[NSApp servicesMenu] has its hidden set to NO, even if it doesn't appear on the menu. (Can be reproduced by first selecting some text, then deselecting the text).

shpakovski commented 2 years ago

Many thanks!