Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MenuItem::CreateFromNative for macOS crashes for items that only have Function modifier #7739

Closed
ecschlegel opened this issue Jun 10, 2021 · 4 comments · Fixed by #7740
Closed

Comments

@ecschlegel
Copy link

NWJS Version : 54
Operating System : macOS 12 WWDC seed

The macOS implementation of MenuItem::CreateFromNative (in nw.js/src/api/menuitem/menuitem_mac.mm) creates a string based on keyEquivalentModifierMask of the native NSMenuItem. This code assumes that if the modifier mask is not zero, then the mask will contain at least one of the command, option, shift, and control modifiers. If none of those modifiers is present, then the resulting string is empty, and the code then crashes.

In macOS 12, the AppKit framework is now creating standard menu items with a keyEquivalentModifierMask that only contains NSEventModifierFlagFunction, with no other modifiers. For example, the "Emoji & Symbols" menu item in the Edit menu now has the keyboard shortcut "fn-E". Therefore, any app that uses nw.js will crash at launch on macOS 12, when the nw.js library attempts to convert the native NSMenuItem into a framework item.

Expected behavior

Shouldn't crash; should handle menu items with modifier mask containing only the Function key modifier.

Actual behavior

Crashes.

How to reproduce

  • download pgAdmin 4, which uses nw.js
  • launch on macOS 12 WWDC seed
  • observe crash at launch

Apple is aware of this crash in pgAdmin, and is providing a workaround for pgAdmin 5.3 and earlier on the next seed of macOS 12 (menu items that would ordinarily use the Function key modifier, will instead use the shortcut that they had in macOS 11 and earlier). However, this won't help any other apps that are linked against nw.js. A fix is needed in nw.js, and any apps that use nw.js will need to update to the new version of the library.

If you need further information about this change in NSMenu in macOS 12, you can contact me at ericsc@apple.com.

@Cubelrti
Copy link
Contributor

Thanks a lot for clarifying technical details.

I submitted a PR and verified that is no more crash in a custom build. (via nwjs 0.49.0).

Hope it helps and get fixed soon!

@dpage-edb
Copy link

pgAdmin dev here - thanks for digging into this folks, I was just starting to do so myself. I'm happy to do any additional testing/builds of pgAdmin if needed. I'd just need a patched nwjs build to do so as I don't have a buildenv for it.

@Cubelrti
Copy link
Contributor

Hi @dpage-edb sorry for late response, please try this unofficial build if needed:
https://www.dropbox.com/sh/r1b74momvnpij3o/AAA1ms2SVQnW_VRq8G04XyZua?dl=0

Below is my test on maOS 12.
image

@dpage-edb
Copy link

Thanks @Cubelrti - I can confirm your build works for me with pgAdmin.

Screenshot 2021-06-21 at 10 47 18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants