bug#29595: 26.0.90; menu items show incorrect shortcut keys

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Paul Rankin]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

The GUI build of Emacs 26.x shows incorrect shortcut keys in the menu: either ⇧C (shift-C) or ⇧M (shift-M). See attached screenshot or https://imgur.com/a/fQaly

These are displayed in the macOS native menu style, while the Emacs duplicated shortcuts in parentheses are still correct.

To reproduce:

1. emacs -Q
2. click a menu item

Expected results:

Menu items should show correct shortcut cut keys.

Actual results:

Menu items show combination of incorrect keys with OS native display and correct keys in Emacs's duplicates.

Configuration:

GNU Emacs 26.0.90 (build 1, x86_64-apple-darwin17.2.0, NS appkit-1561.10 Version 10.13.1 (Build 17B48)) of 2017-11-05

macOS 10.13.1 (17B1003)

-- 
https://www.paulwrankin.com

[-- Attachment #2: Screen Shot 2017-12-07 at 11.26.08 am.jpg --]
[-- Type: image/jpeg, Size: 164404 bytes --]

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Alan Third]

On Thu, Dec 07, 2017 at 11:35:06AM +1000, Paul Rankin wrote:
> The GUI build of Emacs 26.x shows incorrect shortcut keys in the
> menu: either ⇧C (shift-C) or ⇧M (shift-M). See attached screenshot
> or https://imgur.com/a/fQaly
> 
> These are displayed in the macOS native menu style, while the Emacs
> duplicated shortcuts in parentheses are still correct.
> 
> To reproduce:
> 
> 1. emacs -Q
> 2. click a menu item
> 
> Expected results:
> 
> Menu items should show correct shortcut cut keys.
> 
> Actual results:
> 
> Menu items show combination of incorrect keys with OS native display
> and correct keys in Emacs's duplicates.
> 
> Configuration:
> 
> GNU Emacs 26.0.90 (build 1, x86_64-apple-darwin17.2.0, NS
> appkit-1561.10 Version 10.13.1 (Build 17B48)) of 2017-11-05
> 
> macOS 10.13.1 (17B1003)

I can’t replicate this. It looks right to me. The only difference I
can see is that I’m using macOS 10.12, so maybe that’s it. Can anyone
else confirm?

-- 
Alan Third

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Paul Rankin]

I’ve now tested this with Emacs versions 24.4, 25.4, 26.0.90 and HEAD
with the same results; this hasn’t happened for me with 24.x in the
past, so it appears that something has changed with macOS High Sierra
that screws up the display of shortcut keys in the menus.

I realise there may be a dearth of macOS systems in Emacs core
development; I’m sorry to say only know Emacs Lisp, not C, so my ability
to troubleshoot this is limited, but I’m happy to apply any provided
patches and rebuild for testing.

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Eli Zaretskii]

> From: Paul Rankin <hello@paulwrankin.com>
> Date: Fri, 22 Dec 2017 15:16:32 +1000
> 
> I’ve now tested this with Emacs versions 24.4, 25.4, 26.0.90 and HEAD
> with the same results; this hasn’t happened for me with 24.x in the
> past, so it appears that something has changed with macOS High Sierra
> that screws up the display of shortcut keys in the menus.

You are probably right.

> I realise there may be a dearth of macOS systems in Emacs core
> development; I’m sorry to say only know Emacs Lisp, not C, so my ability
> to troubleshoot this is limited, but I’m happy to apply any provided
> patches and rebuild for testing.

I hope Alan (CC'ed) will be able to look into this.  I myself don't
have access to any macOS systems.

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On Fri, Dec 22, 2017 at 09:26:18AM +0200, Eli Zaretskii wrote:
> > From: Paul Rankin <hello@paulwrankin.com>
> > Date: Fri, 22 Dec 2017 15:16:32 +1000
> > 
> > I’ve now tested this with Emacs versions 24.4, 25.4, 26.0.90 and HEAD
> > with the same results; this hasn’t happened for me with 24.x in the
> > past, so it appears that something has changed with macOS High Sierra
> > that screws up the display of shortcut keys in the menus.
> 
> You are probably right.

It seems that older macOS versions ignored keyboard shortcuts they
didn’t understand, but High Sierra appears to use a ‘best effort’
attempt which results in it just printing out the first character
(capital C for ctrl, so shift‐C).

I’ve attached a patch that I hope fixes it.

-- 
Alan Third

[-- Attachment #2: 0001-Fix-menu-keyboard-shortcuts-on-macOS-Bug-29595.patch --]
[-- Type: text/plain, Size: 1760 bytes --]

From 3ca7f2e2113eeace6b75f5f4b86bbcc4be609807 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Fri, 22 Dec 2017 12:27:05 +0000
Subject: [PATCH] Fix menu keyboard shortcuts on macOS (Bug#29595)

* src/nsmenu.m (addItemWithWidgetValue)[NS_IMPL_COCOA]: It looks like
macOS 10.13 no longer ignores long modifier strings, so explicitly use
an empty string.
---
 src/nsmenu.m | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index 6ef7b60dc2..d2e102a2a9 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -643,14 +643,23 @@ - (NSMenuItem *)addItemWithWidgetValue: (void *)wvptr
 
       keyEq = [self parseKeyEquiv: wv->key];
 #ifdef NS_IMPL_COCOA
-      /* macOS just ignores modifier strings longer than one character */
+      /* macOS mangles modifier strings longer than one character.  */
       if (keyEquivModMask == 0)
-        title = [title stringByAppendingFormat: @" (%@)", keyEq];
+        {
+          title = [title stringByAppendingFormat: @" (%@)", keyEq];
+          item = [self addItemWithTitle: (NSString *)title
+                                 action: @selector (menuDown:)
+                          keyEquivalent: @""];
+        }
+      else
+        {
+#endif
+          item = [self addItemWithTitle: (NSString *)title
+                                 action: @selector (menuDown:)
+                          keyEquivalent: keyEq];
+#ifdef NS_IMPL_COCOA
+        }
 #endif
-
-      item = [self addItemWithTitle: (NSString *)title
-                             action: @selector (menuDown:)
-                      keyEquivalent: keyEq];
       [item setKeyEquivalentModifierMask: keyEquivModMask];
 
       [item setEnabled: wv->enabled];
-- 
2.14.3

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Eli Zaretskii]

> Date: Fri, 22 Dec 2017 12:34:33 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: Paul Rankin <hello@paulwrankin.com>, 29595@debbugs.gnu.org
> 
> It seems that older macOS versions ignored keyboard shortcuts they
> didn’t understand, but High Sierra appears to use a ‘best effort’
> attempt which results in it just printing out the first character
> (capital C for ctrl, so shift‐C).
> 
> I’ve attached a patch that I hope fixes it.

Thanks.

Will this patch work on both old and new versions of macOS?  I'd like
to install this on the release branch, but I don't want to cause any
regressions as result of that.

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Alan Third]

On Fri, Dec 22, 2017 at 03:09:26PM +0200, Eli Zaretskii wrote:
> > Date: Fri, 22 Dec 2017 12:34:33 +0000
> > From: Alan Third <alan@idiocy.org>
> > Cc: Paul Rankin <hello@paulwrankin.com>, 29595@debbugs.gnu.org
> > 
> > It seems that older macOS versions ignored keyboard shortcuts they
> > didn’t understand, but High Sierra appears to use a ‘best effort’
> > attempt which results in it just printing out the first character
> > (capital C for ctrl, so shift‐C).
> > 
> > I’ve attached a patch that I hope fixes it.
> 
> Thanks.
> 
> Will this patch work on both old and new versions of macOS?  I'd like
> to install this on the release branch, but I don't want to cause any
> regressions as result of that.

Yes. I’m still on the previous macOS version where I can’t see the bug
and it makes no difference here.

The current code relies on macOS ignoring a string it doesn’t
understand, however the documentation explicitly says to use an empty
string and the API hasn’t changed.
-- 
Alan Third

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Paul Rankin]

Does this fix hinge on me building and testing on High Sierra? Sorry I haven't done this yet.

-- 
https://www.paulwrankin.com

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

Yes, if you could test it that would be very helpful.

On 31 Dec 2017 4:03 a.m., "Paul Rankin" <hello@paulwrankin.com> wrote:

> Does this fix hinge on me building and testing on High Sierra? Sorry I
> haven't done this yet.
>
> --
> https://www.paulwrankin.com
>

[-- Attachment #2: Type: text/html, Size: 635 bytes --]

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Paul Rankin]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Confirming that the previous patch fixes the issue on macOS 10.13 High
Sierra. See attached screenshot or https://imgur.com/gBp6ucn

-- 
https://www.paulwrankin.com

On Sun, 31 Dec 2017, at 9:40 PM, Alan Third wrote:
> Yes, if you could test it that would be very helpful.
> 
> On 31 Dec 2017 4:03 a.m., "Paul Rankin" <hello@paulwrankin.com> wrote:
> 
> > Does this fix hinge on me building and testing on High Sierra? Sorry I
> > haven't done this yet.
> >
> > --
> > https://www.paulwrankin.com
> >

[-- Attachment #2: Screen Shot 2018-01-01 at 8.36.04 pm.jpg --]
[-- Type: image/jpeg, Size: 136178 bytes --]

bug#29595: 26.0.90; menu items show incorrect shortcut keys

[Alan Third]

On Mon, Jan 01, 2018 at 08:40:49PM +1000, Paul Rankin wrote:
> Confirming that the previous patch fixes the issue on macOS 10.13 High
> Sierra. See attached screenshot or https://imgur.com/gBp6ucn

Thanks for the confirmation.

I’ve pushed the fix to emacs-26.
-- 
Alan Third