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

Add fn key to `MASPickCocoaModifiers` #180

Closed Coder-256 closed 1 year ago

Coder-256 commented 1 year ago

I uncovered a very strange bug in an app I'm working on, which allows single-key shortcuts by using a custom validator; however, my validator still calls -[MASShortcutValidator isShortcutAlreadyTakenBySystem:explanation:].

It turns out that if you open the "View" (or "Window"?) menu and close it, from that point on MASShortcut will start recognizing the "Enter Full Screen" menu item, which has a shortcut of fn-F (stylized as globe-F). Then, when calling -[MASShortcutValidator isShortcutAlreadyTakenBySystem:explanation:], MASPickCocoaModifiers drops the NSEventModifierFlagFunction mask, which makes it appear the same as the bare "F" key, and the user is prohibited from selecting "F" as a shortcut.

I imagine this would also be an issue if the user assigned any custom shortcuts to a menu item (from System Preferences > Keyboard) that include the fn key, e.g. fn-cmd-shift-A would conflict with cmd-shift-A.

This PR fixes the issue by including NSEventModifierFlagFunction in the flags returned by MASPickCocoaModifiers.

shpakovski commented 1 year ago

Hey, thanks for the PR. I like the idea but what about handling fn in other places? If we support this modifier, it should be available for selection, rendering, writing and reading into user defaults etc. Adding this workaround in just here is probably not enough. Could you please investigate and fix it in other places as well?

Edit: After checking other hotkey recorders including “System Preferences / Keyboard / Services”, I believe that fn should not become a standard flag like Command or Option. This is a ‘very system’ key. What do you think?

Coder-256 commented 1 year ago

I think a deeper issue is that Carbon just doesn't support the fn key as a modifier at all (there's cmdKeyBit, shiftKeyBit, etc. but there's nothing that supports the function key as a modifier in <HIToolbox/Events.h>). I think it's a Cocoa addition, so there may not actually be a way to register a hotkey involving the fn key using the Carbon API.

Perhaps this change could just be applied to the body of -[MASShortcutValidator isShortcut:alreadyTakenInMenu:explanation:] so that it will properly handle menu shortcuts containing the fn key? That way it would solve the issue I was facing without deciding to fully support fn or causing any unexpected consequences.

Coder-256 commented 1 year ago

I updated the PR with one possible way of doing this.