unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#21551: [PATCH] Fix Mac OS X key bindings bug
@ 2015-09-24 13:19 Kai Yu Zhang
  2015-12-30  8:50 ` bug#21551: " Anders Lindgren
  0 siblings, 1 reply; 10+ messages in thread
From: Kai Yu Zhang @ 2015-09-24 13:19 UTC (permalink / raw)
  To: 21551


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



[-- Attachment #1.2: Type: text/html, Size: 26 bytes --]

[-- Attachment #2: 0001-Fix-Mac-OS-X-key-bindings-bug.patch --]
[-- Type: application/octet-stream, Size: 3358 bytes --]

From c080da382e3d9e60f396eed4364558611e150e8d Mon Sep 17 00:00:00 2001
From: Zhang Kai Yu <yeannylam@gmail.com>
Date: Thu, 24 Sep 2015 21:10:40 +0800
Subject: [PATCH] Fix Mac OS X key bindings bug

---
 src/nsterm.m | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 7c6b9dc..d1ccc8a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -5383,16 +5383,18 @@ not_in_argv (NSString *arg)
           emacs_event->modifiers |= parse_solitary_modifier
             (ns_command_modifier);
 
-          /* if super (default), take input manager's word so things like
-             dvorak / qwerty layout work */
+          /* we should use [theEvent charactersIgnoringModifiers] rather than
+           * [theEvent characters], otherwise the key code will become
+           * extremely big and cause bug when doing key bindings.
+           */
           if (EQ (ns_command_modifier, Qsuper)
               && !fnKeysym
-              && [[theEvent characters] length] != 0)
+              && [[theEvent charactersIgnoringModifiers] length] != 0)
             {
               /* XXX: the code we get will be unshifted, so if we have
                  a shift modifier, must convert ourselves */
               if (!(flags & NSShiftKeyMask))
-                code = [[theEvent characters] characterAtIndex: 0];
+                code = [[theEvent charactersIgnoringModifiers] characterAtIndex: 0];
 #if 0
               /* this is ugly and also requires linking w/Carbon framework
                  (for LMGetKbdType) so for now leave this rare (?) case
@@ -5449,8 +5451,9 @@ not_in_argv (NSString *arg)
                || (EQ (ns_right_alternate_modifier, Qleft) && left_is_none))
               && !fnKeysym)
             {   /* accept pre-interp alt comb */
-              if ([[theEvent characters] length] > 0)
-                code = [[theEvent characters] characterAtIndex: 0];
+              if ([[theEvent charactersIgnoringModifiers] length] > 0)
+                code = [[theEvent charactersIgnoringModifiers]
+                  characterAtIndex: 0];
               /*HACK: clear lone shift modifier to stop next if from firing */
               if (emacs_event->modifiers == shift_modifier)
                 emacs_event->modifiers = 0;
@@ -5466,8 +5469,9 @@ not_in_argv (NSString *arg)
         {
           if (left_is_none && !fnKeysym)
             {   /* accept pre-interp alt comb */
-              if ([[theEvent characters] length] > 0)
-                code = [[theEvent characters] characterAtIndex: 0];
+              if ([[theEvent charactersIgnoringModifiers] length] > 0)
+                code = [[theEvent charactersIgnoringModifiers]
+                  characterAtIndex: 0];
               /*HACK: clear lone shift modifier to stop next if from firing */
               if (emacs_event->modifiers == shift_modifier)
                 emacs_event->modifiers = 0;
@@ -5485,7 +5489,6 @@ not_in_argv (NSString *arg)
       if (fnKeysym || (emacs_event->modifiers
                        && (emacs_event->modifiers != shift_modifier)
                        && [[theEvent charactersIgnoringModifiers] length] > 0))
-/*[[theEvent characters] length] */
         {
           emacs_event->kind = NON_ASCII_KEYSTROKE_EVENT;
           if (code < 0x20)
-- 
2.3.8 (Apple Git-58)


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2015-09-24 13:19 bug#21551: [PATCH] Fix Mac OS X key bindings bug Kai Yu Zhang
@ 2015-12-30  8:50 ` Anders Lindgren
  2016-01-02  8:08   ` Anders Lindgren
  0 siblings, 1 reply; 10+ messages in thread
From: Anders Lindgren @ 2015-12-30  8:50 UTC (permalink / raw)
  To: Kai Yu Zhang, 21551, Philipp Stephani, Mikhail Gusarov

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

Hi,

I'm looking into a key binding bug on OS X reported multiple times (19977,
21330, 21551). Two different patches have been submitted.

The original code looks like:

      if (is_left_key)
        {
          emacs_event->modifiers |= parse_solitary_modifier
            (ns_command_modifier);

          /* if super (default), take input manager's word so things like
             dvorak / qwerty layout work */
          if (EQ (ns_command_modifier, Qsuper)
              && !fnKeysym
              && [[theEvent characters] length] != 0)
            {
              /* XXX: the code we get will be unshifted, so if we have
                 a shift modifier, must convert ourselves */
              if (!(flags & NSShiftKeyMask))
                code = [[theEvent characters] characterAtIndex: 0];

One of the patches simply removes the `if (EQ(...))' statement. The other
modifies the code to strip away modifiers.

First question: What is the code in the `if (EQ(...))' supposed to do? In
other words, what will stop working if it is removed?

Second question: if it is needed for the LEFT command key, should the
corresponding code be added for the RIGHT?

I've tested removing the `if' and Emacs still seems to be working as
intended, both with a normal key layout and when Dvorak is used. If no one
comes up with a reason to keep the code, I will remove it.

Third question: Does anybody know of a good way to automatically test
things like this? What I'm looking for is a way to send keystrokes like
Cmd-Alt-a to Emacs, that way it could be possible to write tests ensuring
that things like this don't break in the future.

    -- Anders Lindgren

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2015-12-30  8:50 ` bug#21551: " Anders Lindgren
@ 2016-01-02  8:08   ` Anders Lindgren
  2016-12-26 20:12     ` bug#19977: " Mikhail Gusarov
  2018-02-11 17:57     ` bug#21551: " Philipp Stephani
  0 siblings, 2 replies; 10+ messages in thread
From: Anders Lindgren @ 2016-01-02  8:08 UTC (permalink / raw)
  To: Kai Yu Zhang, 21551, Philipp Stephani, Mikhail Gusarov

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

I found a case where the code in question is needed, which none of the
suggested patches handle correctly.

Steps to repeat:

    (setq ns-alternate-modifier nil)

    Press left CMD-ALT-9

    An unmodified Emacs replies "s-]" is not bound. (This assumes a Swedish
keyboard layout, other layouts would yield a different character, but the
principle is the same).

    With either of the two patches, Emacs respond with "s-9" is not bound,
which isn't correct.

Unfortunately, I don't know how to distinguish between the cases where we
need to strip away modifiers (C-s-a) and when we shouldn't, so I'm leaving
this open for now.

    -- Anders Lindgren


On Wed, Dec 30, 2015 at 9:50 AM, Anders Lindgren <andlind@gmail.com> wrote:

> Hi,
>
> I'm looking into a key binding bug on OS X reported multiple times (19977,
> 21330, 21551). Two different patches have been submitted.
>
> The original code looks like:
>
>       if (is_left_key)
>         {
>           emacs_event->modifiers |= parse_solitary_modifier
>             (ns_command_modifier);
>
>           /* if super (default), take input manager's word so things like
>              dvorak / qwerty layout work */
>           if (EQ (ns_command_modifier, Qsuper)
>               && !fnKeysym
>               && [[theEvent characters] length] != 0)
>             {
>               /* XXX: the code we get will be unshifted, so if we have
>                  a shift modifier, must convert ourselves */
>               if (!(flags & NSShiftKeyMask))
>                 code = [[theEvent characters] characterAtIndex: 0];
>
> One of the patches simply removes the `if (EQ(...))' statement. The other
> modifies the code to strip away modifiers.
>
> First question: What is the code in the `if (EQ(...))' supposed to do? In
> other words, what will stop working if it is removed?
>
> Second question: if it is needed for the LEFT command key, should the
> corresponding code be added for the RIGHT?
>
> I've tested removing the `if' and Emacs still seems to be working as
> intended, both with a normal key layout and when Dvorak is used. If no one
> comes up with a reason to keep the code, I will remove it.
>
> Third question: Does anybody know of a good way to automatically test
> things like this? What I'm looking for is a way to send keystrokes like
> Cmd-Alt-a to Emacs, that way it could be possible to write tests ensuring
> that things like this don't break in the future.
>
>     -- Anders Lindgren
>
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#19977: Fix Mac OS X key bindings bug
  2016-01-02  8:08   ` Anders Lindgren
@ 2016-12-26 20:12     ` Mikhail Gusarov
  2018-02-11 17:57     ` bug#21551: " Philipp Stephani
  1 sibling, 0 replies; 10+ messages in thread
From: Mikhail Gusarov @ 2016-12-26 20:12 UTC (permalink / raw)
  To: Anders Lindgren, Kai Yu Zhang, 21551, Philipp Stephani, 21330,
	19977

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

Hi Anders,

I have tried reproducing this discrepancy using my patch from #21330
on top of branch emacs-25.

Swedish layout, ns-alternate-modifier set to nil, LCmd-LAlt-9 replies
"s-]".

Maybe you could check again as well?

Best regards,
Mikhail.

On 2 Jan 2016, at 9:08, Anders Lindgren wrote:

> I found a case where the code in question is needed, which none of the
> suggested patches handle correctly.
>
> Steps to repeat:
>
>     (setq ns-alternate-modifier nil)
>
>     Press left CMD-ALT-9
>
>     An unmodified Emacs replies "s-]" is not bound. (This assumes a Swedish
> keyboard layout, other layouts would yield a different character, but the
> principle is the same).
>
>     With either of the two patches, Emacs respond with "s-9" is not bound,
> which isn't correct.
>
> Unfortunately, I don't know how to distinguish between the cases where we
> need to strip away modifiers (C-s-a) and when we shouldn't, so I'm leaving
> this open for now.
>
>     -- Anders Lindgren
>
>
> On Wed, Dec 30, 2015 at 9:50 AM, Anders Lindgren <andlind@gmail.com> wrote:
>
>> Hi,
>>
>> I'm looking into a key binding bug on OS X reported multiple times (19977,
>> 21330, 21551). Two different patches have been submitted.
>>
>> The original code looks like:
>>
>>       if (is_left_key)
>>         {
>>           emacs_event->modifiers |= parse_solitary_modifier
>>             (ns_command_modifier);
>>
>>           /* if super (default), take input manager's word so things like
>>              dvorak / qwerty layout work */
>>           if (EQ (ns_command_modifier, Qsuper)
>>               && !fnKeysym
>>               && [[theEvent characters] length] != 0)
>>             {
>>               /* XXX: the code we get will be unshifted, so if we have
>>                  a shift modifier, must convert ourselves */
>>               if (!(flags & NSShiftKeyMask))
>>                 code = [[theEvent characters] characterAtIndex: 0];
>>
>> One of the patches simply removes the `if (EQ(...))' statement. The other
>> modifies the code to strip away modifiers.
>>
>> First question: What is the code in the `if (EQ(...))' supposed to do? In
>> other words, what will stop working if it is removed?
>>
>> Second question: if it is needed for the LEFT command key, should the
>> corresponding code be added for the RIGHT?
>>
>> I've tested removing the `if' and Emacs still seems to be working as
>> intended, both with a normal key layout and when Dvorak is used. If no one
>> comes up with a reason to keep the code, I will remove it.
>>
>> Third question: Does anybody know of a good way to automatically test
>> things like this? What I'm looking for is a way to send keystrokes like
>> Cmd-Alt-a to Emacs, that way it could be possible to write tests ensuring
>> that things like this don't break in the future.
>>
>>     -- Anders Lindgren
>>
>>



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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2016-01-02  8:08   ` Anders Lindgren
  2016-12-26 20:12     ` bug#19977: " Mikhail Gusarov
@ 2018-02-11 17:57     ` Philipp Stephani
  2018-02-13 18:56       ` Alan Third
  1 sibling, 1 reply; 10+ messages in thread
From: Philipp Stephani @ 2018-02-11 17:57 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: 21551, Kai Yu Zhang, Mikhail Gusarov

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

Anders Lindgren <andlind@gmail.com> schrieb am Sa., 2. Jan. 2016 um
09:08 Uhr:

> I found a case where the code in question is needed, which none of the
> suggested patches handle correctly.
>
> Steps to repeat:
>
>     (setq ns-alternate-modifier nil)
>
>     Press left CMD-ALT-9
>
>     An unmodified Emacs replies "s-]" is not bound. (This assumes a
> Swedish keyboard layout, other layouts would yield a different character,
> but the principle is the same).
>
>     With either of the two patches, Emacs respond with "s-9" is not bound,
> which isn't correct.
>

In the current master this is now interpreted as M-s-9, which is at least
somewhat reasonable.


>
> Unfortunately, I don't know how to distinguish between the cases where we
> need to strip away modifiers (C-s-a) and when we shouldn't, so I'm leaving
> this open for now.
>

It seems like in the macOS input model, we can't: either we strip away all
modifiers, or none.


>
>     -- Anders Lindgren
>
>
> On Wed, Dec 30, 2015 at 9:50 AM, Anders Lindgren <andlind@gmail.com>
> wrote:
>
>> Hi,
>>
>> I'm looking into a key binding bug on OS X reported multiple times
>> (19977, 21330, 21551). Two different patches have been submitted.
>>
>> The original code looks like:
>>
>>       if (is_left_key)
>>         {
>>           emacs_event->modifiers |= parse_solitary_modifier
>>             (ns_command_modifier);
>>
>>           /* if super (default), take input manager's word so things like
>>              dvorak / qwerty layout work */
>>           if (EQ (ns_command_modifier, Qsuper)
>>               && !fnKeysym
>>               && [[theEvent characters] length] != 0)
>>             {
>>               /* XXX: the code we get will be unshifted, so if we have
>>                  a shift modifier, must convert ourselves */
>>               if (!(flags & NSShiftKeyMask))
>>                 code = [[theEvent characters] characterAtIndex: 0];
>>
>> One of the patches simply removes the `if (EQ(...))' statement. The other
>> modifies the code to strip away modifiers.
>>
>> First question: What is the code in the `if (EQ(...))' supposed to do? In
>> other words, what will stop working if it is removed?
>>
>> Second question: if it is needed for the LEFT command key, should the
>> corresponding code be added for the RIGHT?
>>
>> I've tested removing the `if' and Emacs still seems to be working as
>> intended, both with a normal key layout and when Dvorak is used. If no one
>> comes up with a reason to keep the code, I will remove it.
>>
>> Third question: Does anybody know of a good way to automatically test
>> things like this? What I'm looking for is a way to send keystrokes like
>> Cmd-Alt-a to Emacs, that way it could be possible to write tests ensuring
>> that things like this don't break in the future.
>>
>>     -- Anders Lindgren
>>
>>
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2018-02-11 17:57     ` bug#21551: " Philipp Stephani
@ 2018-02-13 18:56       ` Alan Third
  2018-02-14 21:44         ` Alan Third
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Third @ 2018-02-13 18:56 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 21551, Kai Yu Zhang, Mikhail Gusarov, Anders Lindgren

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

On Sun, Feb 11, 2018 at 05:57:32PM +0000, Philipp Stephani wrote:
> Anders Lindgren <andlind@gmail.com> schrieb am Sa., 2. Jan. 2016 um
> 09:08 Uhr:
> 
> > I found a case where the code in question is needed, which none of the
> > suggested patches handle correctly.
> >
> > Steps to repeat:
> >
> >     (setq ns-alternate-modifier nil)
> >
> >     Press left CMD-ALT-9
> >
> >     An unmodified Emacs replies "s-]" is not bound. (This assumes a
> > Swedish keyboard layout, other layouts would yield a different character,
> > but the principle is the same).
> >
> >     With either of the two patches, Emacs respond with "s-9" is not bound,
> > which isn't correct.
> >
> 
> In the current master this is now interpreted as M-s-9, which is at least
> somewhat reasonable.

For me it’s s-9, not M-s-9.

> > Unfortunately, I don't know how to distinguish between the cases where we
> > need to strip away modifiers (C-s-a) and when we shouldn't, so I'm leaving
> > this open for now.
> >
> 
> 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.

(It’s broken the arrow keys, and I don’t know why...)
-- 
Alan Third

[-- Attachment #2: poc.patch --]
[-- Type: text/plain, Size: 2958 bytes --]

diff --git a/configure.ac b/configure.ac
index f2a8332d71..b5db45801d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5260,7 +5260,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 29aef0e9b6..2038c02e60 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -69,6 +69,8 @@ Updated by Christian Limpach (chris@nice.ch)
 #include "macfont.h"
 #endif
 
+#include <Carbon/Carbon.h>
+
 static EmacsMenu *dockMenu;
 #ifdef NS_IMPL_COCOA
 static EmacsMenu *mainMenu;
@@ -2681,6 +2683,45 @@ so some key presses (TAB) are swallowed by the system. */
 }
 
 
+static UniChar
+ns_get_shifted_character (NSEvent *event)
+{
+  static UInt32 deadKeyState;
+
+  CFDataRef layout_ref = (CFDataRef) TISGetInputSourceProperty
+    (TISCopyCurrentKeyboardLayoutInputSource (), kTISPropertyUnicodeKeyLayoutData);
+  UCKeyboardLayout* layout = CFDataGetBytePtr (layout_ref);
+
+  UniChar buf[255];
+  UniCharCount maxStringLength;
+  UniCharCount actualStringLength;
+  OSStatus result;
+
+  unsigned int flags = [event modifierFlags];
+  UInt32 modifiers = (flags & NSEventModifierFlagShift) ? shiftKey : 0;
+  modifiers |= ((flags & NSRightAlternateKeyMask) == NSRightAlternateKeyMask
+                && EQ (ns_right_alternate_modifier, Qnone)) ? rightOptionKey : 0;
+  modifiers |= ((flags & NSLeftAlternateKeyMask) == NSLeftAlternateKeyMask
+                && EQ (ns_alternate_modifier, Qnone)) ? optionKey : 0;
+
+  modifiers |= ((flags & NSRightCommandKeyMask) == NSRightCommandKeyMask
+                && EQ (ns_right_command_modifier, Qnone)) ? cmdKey : 0;
+  modifiers |= ((flags & NSLeftCommandKeyMask) == NSLeftCommandKeyMask
+                && EQ (ns_command_modifier, Qnone)) ? cmdKey : 0;
+
+  result = UCKeyTranslate (layout,
+                           [event keyCode], kUCKeyActionDown,
+                           (modifiers >> 8) & 0xFF,
+                           LMGetKbdType (), kUCKeyTranslateNoDeadKeysBit,
+                           &deadKeyState, 255, &actualStringLength,
+                           buf);
+
+  if (result != 0)
+    NSLog(@"Disaster!");
+
+  return buf[0];
+}
+
 
 /* ==========================================================================
 
@@ -6147,7 +6188,8 @@ most recently updated (I guess), which is not the correct one. */
       /* FIXME: What should happen for key sequences with more than
          one character?  */
       code = ([[theEvent charactersIgnoringModifiers] length] == 0) ?
-        0 : [[theEvent charactersIgnoringModifiers] characterAtIndex: 0];
+        0 : //[[theEvent charactersIgnoringModifiers] characterAtIndex: 0];
+        ns_get_shifted_character (theEvent);
 
       /* (Carbon way: [theEvent keyCode]) */
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2018-02-13 18:56       ` Alan Third
@ 2018-02-14 21:44         ` Alan Third
  2018-02-17 12:36           ` Alan Third
  2018-02-18 21:59           ` Philipp Stephani
  0 siblings, 2 replies; 10+ messages in thread
From: Alan Third @ 2018-02-14 21:44 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 21551, Kai Yu Zhang, Mikhail Gusarov, Anders Lindgren

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

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

[-- Attachment #2: 0001-Fix-modifier-key-handling-on-macOS.patch --]
[-- Type: text/plain, Size: 7043 bytes --]

From 398c954448cf27f9ae13a358aa381e8cdcb16e10 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
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 (chris@nice.ch)
 #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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2018-02-14 21:44         ` Alan Third
@ 2018-02-17 12:36           ` Alan Third
  2018-02-18 21:59           ` Philipp Stephani
  1 sibling, 0 replies; 10+ messages in thread
From: Alan Third @ 2018-02-17 12:36 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 21551, Kai Yu Zhang, Mikhail Gusarov, Anders Lindgren

On Wed, Feb 14, 2018 at 09:44:44PM +0000, Alan Third wrote:
> 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.

Pushed to master.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2018-02-14 21:44         ` Alan Third
  2018-02-17 12:36           ` Alan Third
@ 2018-02-18 21:59           ` Philipp Stephani
  2018-02-19 15:05             ` Richard Stallman
  1 sibling, 1 reply; 10+ messages in thread
From: Philipp Stephani @ 2018-02-18 21:59 UTC (permalink / raw)
  To: Alan Third; +Cc: 21551, Kai Yu Zhang, Mikhail Gusarov, Anders Lindgren

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

Alan Third <alan@idiocy.org> schrieb am Mi., 14. Feb. 2018 um 22:44 Uhr:

> 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.
>
>
Carbon is deprecated, isn't it? That means it's quite likely that patches
relying on Carbon will have to be removed later, should Carbon be ever
removed completely.
For now, it's probably fine though to use Carbon if there's no alternative.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#21551: Fix Mac OS X key bindings bug
  2018-02-18 21:59           ` Philipp Stephani
@ 2018-02-19 15:05             ` Richard Stallman
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Stallman @ 2018-02-19 15:05 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 21551, alan, dottedmag, yeannylam, andlind

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Carbon is deprecated, isn't it?

Does that mean we are in favor of carbon omissions?   ;-).

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-02-19 15:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-24 13:19 bug#21551: [PATCH] Fix Mac OS X key bindings bug Kai Yu Zhang
2015-12-30  8:50 ` bug#21551: " Anders Lindgren
2016-01-02  8:08   ` Anders Lindgren
2016-12-26 20:12     ` bug#19977: " Mikhail Gusarov
2018-02-11 17:57     ` bug#21551: " Philipp Stephani
2018-02-13 18:56       ` Alan Third
2018-02-14 21:44         ` Alan Third
2018-02-17 12:36           ` Alan Third
2018-02-18 21:59           ` Philipp Stephani
2018-02-19 15:05             ` Richard Stallman

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).