[Mac] Produce correct DomKey when Ctrl/Shift/Command is down

ui/events/keycodes/keyboard_code_conversion_mac.mm

Index: ui/events/keycodes/keyboard_code_conversion_mac.mm
diff --git a/ui/events/keycodes/keyboard_code_conversion_mac.mm b/ui/events/keycodes/keyboard_code_conversion_mac.mm
index 8eb5749c12bbb959e942df289da4a4ec58d772bd..0fc7706127ce0f9f6f1b7a65e86033850e2a4a02 100644
--- a/ui/events/keycodes/keyboard_code_conversion_mac.mm
+++ b/ui/events/keycodes/keyboard_code_conversion_mac.mm
@@ -9,6 +9,7 @@
 #import <Carbon/Carbon.h>
 
 #include "base/logging.h"
+#include "base/mac/scoped_cftyperef.h"
 #include "base/macros.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
 
@@ -439,6 +440,7 @@ KeyboardCode KeyboardCodeFromKeyCode(unsigned short keyCode) {
 
 DomKey DomKeyFromKeyCode(unsigned short keyCode) {
   switch (keyCode) {
+    case kVK_ANSI_KeypadEnter:
     case kVK_Return:
       return DomKey::ENTER;
     case kVK_Tab:
@@ -535,11 +537,6 @@ DomKey DomKeyFromKeyCode(unsigned short keyCode) {
 
 DomKey DomKeyFromCharCode(unichar char_code) {
   switch (char_code) {
-    case 0x03:
-      return DomKey::ENTER;  // Numpad Enter
-    // Mac maps backspace to forward delete unicode.
-    case 0x7f:
-      return DomKey::BACKSPACE;
     case NSUpArrowFunctionKey:
       return DomKey::ARROW_UP;
     case NSDownArrowFunctionKey:
@@ -635,6 +632,57 @@ DomKey DomKeyFromCharCode(unichar char_code) {
   }
 }
 
+UniChar KeycodeAndModifiersToCharacter(unsigned short native_key_code,
+                                       int modifiers,
+                                       bool& is_deadkey) {

[tapted, 2016/02/23 23:55:52]

bool& -> bool* (non-const refs aren't allowed in Chromium). But also I don't
think you need the is_deadkey argument at all (see below)

+  // Convert EventRecord modifiers to format UCKeyTranslate accepts. See docs
+  // on UCKeyTranslate for more info.
+  int native_modifiersers = 0;

[tapted, 2016/02/23 23:55:52]

modifiersers ? "native" is a bit confusing too since `NSShiftKeyMask` is native
too. Perhaps unicode_modifiers?

+  if (modifiers & NSShiftKeyMask)
+    native_modifiersers |= shiftKey;
+  if (modifiers & NSAlphaShiftKeyMask)
+    native_modifiersers |= alphaLock;
+  if (modifiers & NSControlKeyMask)

[tapted, 2016/02/23 23:55:52]

aren't some of these effectively deadcode since you pass ` modifiers &
(NSShiftKeyMask | NSAlphaShiftKeyMask | NSAlternateKeyMask)`.

+    native_modifiersers |= controlKey;
+  if (modifiers & NSAlternateKeyMask)
+    native_modifiersers |= optionKey;
+  if (modifiers & NSCommandKeyMask)
+    native_modifiersers |= cmdKey;
+  UInt32 modifier_key_state = (native_modifiersers >> 8) & 0xFF;
+
+  base::ScopedCFTypeRef<TISInputSourceRef> input_source_copy(
+      TISCopyCurrentKeyboardLayoutInputSource());
+  CFDataRef layout_data = static_cast<CFDataRef>(TISGetInputSourceProperty(

[tapted, 2016/02/23 23:55:52]

Since it's used a couple of times (and layout_data isn't otherwise needed)

const UCKeyboardLayout* layout = reinterpret_cast<..>(... )

+      input_source_copy, kTISPropertyUnicodeKeyLayoutData));
+
+  UInt32 dead_key_state = 0;
+  UniCharCount char_count = 0;
+  UniChar character = 0;
+  OSStatus status = UCKeyTranslate(
+      reinterpret_cast<const UCKeyboardLayout*>(CFDataGetBytePtr(layout_data)),
+      static_cast<UInt16>(native_key_code), kUCKeyActionDown,
+      modifier_key_state, LMGetKbdLast(), kUCKeyTranslateNoDeadKeysBit,
+      &dead_key_state, 1, &char_count, &character);
+
+  // According to Apple's doc an empty string means dead keys.
+  // https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSEvent_Class/#//apple_ref/occ/instp/NSEvent/characters
+  if (status == noErr && char_count == 0)

[tapted, 2016/02/23 23:55:52]

*is_deadkey = status == noErr && char_count == 0;

But also is there any time the result should be other that noErr? The
UCKeyTranslate documentation suggests to me it would only give an error if the
input is garbage (e.g. layout_data is null)

So perhaps 

OSSTATUS_DCHECK(status == noErr, status);
*is_deadkey = char_count == 0;


[edit] But, then you don't really seem to need is_deadkey as an out param, so
you can just do

if (char_count == 0) {
  // Inject space... 

+    is_deadkey = true;
+  else
+    is_deadkey = false;
+
+  if (is_deadkey) {
+    // Inject space to get character value of dead key.

[tapted, 2016/02/23 23:55:52]

I don't really understand what this means

+    UCKeyTranslate(reinterpret_cast<const UCKeyboardLayout*>(

[tapted, 2016/02/23 23:55:52]

Assign result and do

OSSTATUS_DCHECK(status == noErr, status);

+                       CFDataGetBytePtr(layout_data)),
+                   static_cast<UInt16>(kVK_Space), kUCKeyActionDown, 0,
+                   LMGetKbdLast(), kUCKeyTranslateNoDeadKeysBit,
+                   &dead_key_state, 1, &char_count, &character);
+  }
+
+  return character;
+}
+
 }  // namespace
 
 int MacKeyCodeForWindowsKeyCode(KeyboardCode keycode,
@@ -742,12 +790,36 @@ DomCode DomCodeFromNSEvent(NSEvent* event) {
 
 DomKey DomKeyFromNSEvent(NSEvent* event) {
   // Apply the lookup based on the character first since that has the
-  // Keyboard layout and modifers already applied; whereas the keyCode
+  // Keyboard layout and modifiers already applied; whereas the keyCode
   // doesn't.
   if ([event type] == NSKeyDown || [event type] == NSKeyUp) {
+    // Have to use [event characters] to handle dead key state.
     NSString* characters = [event characters];
-    if ([characters length] > 0)
-      return DomKeyFromCharCode([characters characterAtIndex:0]);
+    if ([characters length] > 0) {
+      // An invalid dead key combination will produce two characters, according
+      // to spec DomKey should be the last character.
+      // e.g. On French keyboard [+a will produce "^q", DomKey should be 'q'.
+      unichar dom_key_char =
+          [characters characterAtIndex:[characters length] - 1];
+      const int kCtrlCmdKeyMask = NSControlKeyMask | NSCommandKeyMask;
+      // On Mac Blink won't insert ASCII character if Ctrl/Command is down.

[tapted, 2016/02/23 23:55:52]

`Ctrl/Command is` -> `either Ctrl or Command, or both, are down`

+      // See EditingBehavior::shouldInsertCharacter()

[tapted, 2016/02/23 23:55:52]

full stop after comment

+      if (dom_key_char < 0x80 && ([event modifierFlags] & kCtrlCmdKeyMask)) {

[chongz, 2016/02/22 20:17:18]

I'm trying to match the logic here
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

So basically I want to know if this keydown will produce a character in the
input field.

[tapted, 2016/02/24 00:05:51]

Also I'm not sure what you're asking here. If you asking whether native
NSTextFields insert a character if either Cmd or Ctrl are down (or both), then
the answer is "no". (But they do if Alt is pressed).

However, Ctrl on Mac triggers a lot of text-editing commands that can still
change the input field. E.g. Ctlr+T will swap characters either side of the
cursor.

[chongz, 2016/02/24 15:12:30]

Yes that's exactly what I need, thanks!

+        // According to spec if the key combination produces a non-printable
+        // character, the key value should be the character without modifiers
+        // except Shift and AltGr.
+        // See https://w3c.github.io/uievents/#keys-guidelines
+        bool is_deadkey;
+        const int fallback_modifiers =

[tapted, 2016/02/23 23:55:52]

kFallbackModifiers or perhaps kAllowedModifiersMask?

+            NSShiftKeyMask | NSAlphaShiftKeyMask | NSAlternateKeyMask;
+        // KeycodeAndModifiersToCharacter() is efficient (around 1.2E-3 ms).
+        dom_key_char = KeycodeAndModifiersToCharacter(
+            [event keyCode], [event modifierFlags] & fallback_modifiers,
+            is_deadkey);

[tapted, 2016/02/23 23:55:52]

is_deadkey isn't used after this - remove?

+      }
+      if (!std::iscntrl(dom_key_char))
+        return DomKeyFromCharCode(dom_key_char);
+    }
   }
   return DomKeyFromKeyCode([event keyCode]);
 }