Subject: bug#21551: Fix Mac OS X key bindings bug

From: Alan Third <[email-masked]>
Date: Wed, 14 Feb 2018 21:44:44 +0000

On Tue, Feb 13, 2018 at 06:56:20PM +0000, Alan Third wrote:
> On Sun, Feb 11, 2018 at 05:57:32PM +0000, Philipp Stephani wrote:
> > It seems like in the macOS input model, we can't: either we strip away all
> > modifiers, or none.
> 
> It appears you can using some Carbon stuff. Half‐assed proof of
> concept patch attached.

Better version attached.

Is there any reason to avoid Carbon code? I’m not aware of any, and if
it’s OK then I think this finally solves this problem.

(It might be best to get someone with a different keyboard layout than
me to test it. The only reason I have for using alt as a shift key is
to type ‘#’ or ‘€’.)
-- 
Alan Third
>From 398c954448cf27f9ae13a358aa381e8cdcb16e10 Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Wed, 14 Feb 2018 20:28:46 +0000
Subject: [PATCH] Fix modifier key handling on macOS

* configure.ac: Use the Carbon framework on macOS.
* src/nsterm.m (ns_get_shifted_character): New function.
(EmacsView::keyDown): Use ns_get_shifted_character when we have
shift style modifiers.
---
 configure.ac |   2 +-
 src/nsterm.m | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 91 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index cb452e053b..cf0347a61e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5269,7 +5269,7 @@ AC_DEFUN
    if test "$HAVE_NS" = "yes"; then
      libs_nsgui="-framework AppKit"
      if test "$NS_IMPL_COCOA" = "yes"; then
-        libs_nsgui="$libs_nsgui -framework IOKit"
+        libs_nsgui="$libs_nsgui -framework IOKit -framework Carbon"
      fi
    else
      libs_nsgui=
diff --git a/src/nsterm.m b/src/nsterm.m
index 627a61cac6..3f74f4131f 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -69,6 +69,8 @@ Updated by Christian Limpach ([email-masked])
 #include "macfont.h"
 #endif
 
+#include <Carbon/Carbon.h>
+
 static EmacsMenu *dockMenu;
 #ifdef NS_IMPL_COCOA
 static EmacsMenu *mainMenu;
@@ -2679,7 +2681,78 @@ so some key presses (TAB) are swallowed by the system. */
   return value;
 }
 
+#ifdef NS_IMPL_COCOA
+static UniChar
+ns_get_shifted_character (NSEvent *event)
+/* Look up the character corresponding to the key pressed on the
+   current keyboard layout and the currently configured shift-like
+   modifiers.  This ignores the control-like modifiers that cause
+   [event characters] to give us the wrong result.
+
+   Although UCKeyTranslate doesn't require the Carbon framework, some
+   of the surrounding paraphernalia does, so this function makes
+   Carbon a requirement.  */
+{
+  static UInt32 dead_key_state;
+
+  /* UCKeyTranslate may return up to 255 characters.  If the buffer
+     isn't large enough then it produces an error.  What kind of
+     keyboard inputs 255 characters in a single keypress?  */
+  UniChar buf[255];
+  UniCharCount max_string_length = 255;
+  UniCharCount actual_string_length = 0;
+  OSStatus result;
+
+  CFDataRef layout_ref = (CFDataRef) TISGetInputSourceProperty
+    (TISCopyCurrentKeyboardLayoutInputSource (), kTISPropertyUnicodeKeyLayoutData);
+  UCKeyboardLayout* layout = (UCKeyboardLayout*) CFDataGetBytePtr (layout_ref);
+
+  UInt32 flags = [event modifierFlags];
+  UInt32 modifiers = (flags & NSEventModifierFlagShift) ? shiftKey : 0;
+
+  NSTRACE ("ns_get_shifted_character");
+
+  if ((flags & NSRightAlternateKeyMask) == NSRightAlternateKeyMask
+      && (EQ (ns_right_alternate_modifier, Qnone)
+          || (EQ (ns_right_alternate_modifier, Qleft)
+              && EQ (ns_alternate_modifier, Qnone))))
+    modifiers |= rightOptionKey;
+
+  if ((flags & NSLeftAlternateKeyMask) == NSLeftAlternateKeyMask
+      && EQ (ns_alternate_modifier, Qnone))
+    modifiers |= optionKey;
+
+  if ((flags & NSRightCommandKeyMask) == NSRightCommandKeyMask
+      && (EQ (ns_right_command_modifier, Qnone)
+          || (EQ (ns_right_command_modifier, Qleft)
+              && EQ (ns_command_modifier, Qnone))))
+    /* Carbon doesn't differentiate between left and right command
+       keys.  */
+    modifiers |= cmdKey;
+
+  if ((flags & NSLeftCommandKeyMask) == NSLeftCommandKeyMask
+      && EQ (ns_command_modifier, Qnone))
+    modifiers |= cmdKey;
+
+  result = UCKeyTranslate (layout, [event keyCode], kUCKeyActionDown,
+                           (modifiers >> 8) & 0xFF, LMGetKbdType (),
+                           kUCKeyTranslateNoDeadKeysBit, &dead_key_state,
+                           max_string_length, &actual_string_length, buf);
+
+  if (result != 0)
+    {
+      NSLog(@"Failed to translate character '%@' with modifiers %x",
+            [event characters], modifiers);
+      return 0;
+    }
 
+  /* FIXME: What do we do if more than one code unit is returned?  */
+  if (actual_string_length > 0)
+    return buf[0];
+
+  return 0;
+}
+#endif /* NS_IMPL_COCOA */
 
 /* ==========================================================================
 
@@ -6148,8 +6221,6 @@ most recently updated (I guess), which is not the correct one. */
       code = ([[theEvent charactersIgnoringModifiers] length] == 0) ?
         0 : [[theEvent charactersIgnoringModifiers] characterAtIndex: 0];
 
-      /* (Carbon way: [theEvent keyCode]) */
-
       /* is it a "function key"? */
       /* Note: Sometimes a plain key will have the NSEventModifierFlagNumericPad
          flag set (this is probably a bug in the OS).
@@ -6191,8 +6262,8 @@ untranslated characters (returned by the
          charactersIgnoringModifiers method).  An annoyance happens if
          we have both shift-like and control-like modifiers because
          the NSEvent API doesn’t let us ignore only some modifiers.
-         Therefore we ignore all shift-like modifiers in that
-         case.  */
+         In that case we use UCKeyTranslate (ns_get_shifted_character)
+         to look up the correct character.  */
 
       /* EV_MODIFIERS2 uses parse_solitary_modifier on all known
          modifier keys, which returns 0 for shift-like modifiers.
@@ -6218,7 +6289,6 @@ untranslated characters (returned by the
       if (fnKeysym || (emacs_event->modifiers
                        && (emacs_event->modifiers != shift_modifier)
                        && [[theEvent charactersIgnoringModifiers] length] > 0))
-/*[[theEvent characters] length] */
         {
           emacs_event->kind = NON_ASCII_KEYSTROKE_EVENT;
           /* FIXME: What are the next four lines supposed to do?  */
@@ -6227,12 +6297,21 @@ untranslated characters (returned by the
           else if (code == 0x7f)
             code |= (1<<28)|(3<<16);
           else if (!fnKeysym)
-            /* FIXME: This seems wrong, characters in the range
-               [0x80, 0xFF] are not ASCII characters.  Can’t we just
-               use MULTIBYTE_CHAR_KEYSTROKE_EVENT here for all kinds
-               of characters?  */
-            emacs_event->kind = code > 0xFF
-              ? MULTIBYTE_CHAR_KEYSTROKE_EVENT : ASCII_KEYSTROKE_EVENT;
+            {
+#ifdef NS_IMPL_COCOA
+              /* We potentially have both shift- and control-like
+                 modifiers in use, so find the correct character
+                 ignoring any control-like ones.  */
+              code = ns_get_shifted_character (theEvent);
+#endif
+
+              /* FIXME: This seems wrong, characters in the range
+                 [0x80, 0xFF] are not ASCII characters.  Can’t we just
+                 use MULTIBYTE_CHAR_KEYSTROKE_EVENT here for all kinds
+                 of characters?  */
+              emacs_event->kind = code > 0xFF
+                ? MULTIBYTE_CHAR_KEYSTROKE_EVENT : ASCII_KEYSTROKE_EVENT;
+            }
 
           emacs_event->code = code;
           EV_TRAILER (theEvent);
-- 
2.16.1