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

Support forward delete menu item accelerator #6168

Merged
merged 2 commits into from Jun 22, 2016
Merged

Support forward delete menu item accelerator #6168

merged 2 commits into from Jun 22, 2016

Conversation

kevinsawicki
Copy link
Contributor

@kevinsawicki kevinsawicki commented Jun 21, 2016

Not sure if/when/how this might have regressed, but using an accelerator of Delete no longer works as reported in #5419.

This looks to be because ui::MacKeyCodeForWindowsKeyCode from Chrome is returning NSDeleteFunctionKey as the character instead of NSDeleteCharacter.

From the NSMenuItem keyEquivalent docs:

If you want to specify the Backspace key as the key equivalent for a menu item, use a single character string with NSBackspaceCharacter (defined in NSText.h as 0x08) and for the Forward Delete key, use NSDeleteCharacter (defined in NSText.h as 0x7F). Note that these are not the same characters you get from an NSEvent key-down event when pressing those keys.

https://cs.chromium.org/chromium/src/ui/events/keycodes/keyboard_code_conversion_mac.mm?l=75

This pull request adds a check and mapping for this case which gets Delete working again as an accelerator for a menu like:

{
  label: 'Backspace',
  accelerator: 'Backspace'
},
{
  label: 'Delete',
  accelerator: 'Delete'
}

Before

screen shot 2016-06-21 at 12 31 35 pm

### After

screen shot 2016-06-21 at 12 32 13 pm

Closes #5419

@zcbenz
Copy link
Member

zcbenz commented Jun 22, 2016

👍

@zcbenz zcbenz merged commit c0aebc9 into master Jun 22, 2016
@zcbenz zcbenz deleted the forward-delete branch June 22, 2016 01:12
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 this pull request may close these issues.

None yet

2 participants