[DomKey] Expose Korean special keys on Korean keyboard layout only

ui/events/keycodes/platform_key_map_win.cc

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/events/keycodes/platform_key_map_win.h"
 
 #include <utility>
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ skipping 225 matching lines @@
     {VKEY_MEDIA_STOP, DomKey::MEDIA_STOP},
     {VKEY_MEDIA_PLAY_PAUSE, DomKey::MEDIA_PLAY_PAUSE},
     {VKEY_MEDIA_LAUNCH_MAIL, DomKey::LAUNCH_MAIL},
     {VKEY_MEDIA_LAUNCH_MEDIA_SELECT, DomKey::LAUNCH_MEDIA_PLAYER},
     {VKEY_MEDIA_LAUNCH_APP1, DomKey::LAUNCH_MY_COMPUTER},
     {VKEY_MEDIA_LAUNCH_APP2, DomKey::LAUNCH_CALCULATOR},
     // VKEY_OEM_1..8, 102, PLUS, COMMA, MINUS, PERIOD
     {VKEY_ALTGR, DomKey::ALT_GRAPH},
     {VKEY_PROCESSKEY, DomKey::PROCESS},
     // VKEY_PACKET - Used to pass Unicode char, considered as printable key.
-
-    // TODO(chongz): Handle Japanese keyboard layout keys 0xF0..0xF5.
-    // https://crbug.com/612694
     {VKEY_ATTN, DomKey::ATTN},
     {VKEY_CRSEL, DomKey::CR_SEL},
     {VKEY_EXSEL, DomKey::EX_SEL},
     {VKEY_EREOF, DomKey::ERASE_EOF},
     {VKEY_PLAY, DomKey::PLAY},
     {VKEY_ZOOM, DomKey::ZOOM_TOGGLE},
     // TODO(chongz): Handle VKEY_NONAME, VKEY_PA1.
     // https://crbug.com/616910
     {VKEY_OEM_CLEAR, DomKey::CLEAR},
 };
 
-DomKey NonPrintableKeyboardCodeToDomKey(KeyboardCode key_code) {
+DomKey LangSpecificNonPrintableKeyboardCodeToDomKey(KeyboardCode key_code,
+                                                    HKL layout) {

[Wez, 2016/06/10 22:55:09]

nit: I'd suggest calling this LanguageSpecificOemKeyboardCodeToDomKey().

It'd also help to have a comment e.g. "Disambiguates the meaning of certain
non-printable keys which have different meanings under different languages, but
use the same VKEY code."

[chongz, 2016/06/14 20:41:47]

Done.

+  WORD langID = LOWORD(layout);

[Wez, 2016/06/10 22:55:09]

nit: langID->language_id, or just |language|

[chongz, 2016/06/14 20:41:47]

Done.

+  WORD primaryLangID = PRIMARYLANGID(langID);

[Wez, 2016/06/10 22:55:09]

nit: primary_language_id or just |primary_language|

[chongz, 2016/06/14 20:41:47]

Done.

+  // TODO(chongz): Handle Japanese keyboard layout keys 0xF0..0xF5.
+  // https://crbug.com/612694

[Wez, 2016/06/10 22:55:09]

nit: I'd suggest putting this comment after the Korean "if" block, otherwise it
looks like the comment refers to this block.

[chongz, 2016/06/14 20:41:47]

Done.

+  if (primaryLangID == LANG_KOREAN) {
+    switch (key_code) {
+      case VKEY_HANGUL:  // Same as VKEY_KANA.
+        return DomKey::HANGUL_MODE;
+      case VKEY_HANJA:  // Same as VKEY_KANJI.

[Wez, 2016/06/10 22:55:09]

Do these comments make sense in the context of Korean - VKEY_KANA and KANJI are
Japanese-layout names for those OEM values, right?

[chongz, 2016/06/14 20:41:47]

Done.

+        return DomKey::HANJA_MODE;
+      default:
+        return DomKey::NONE;
+    }
+  }
+  return DomKey::NONE;
+}
+
+DomKey NonPrintableKeyboardCodeToDomKey(KeyboardCode key_code, HKL layout) {
+  // 1. Look up general key map.

[Wez, 2016/06/10 22:55:09]

nit: Suggest "Most |key_codes| have the same meaning regardless of |layout|."
and then "Look up a |layout|-specific meaning for |key_code|." below.

[chongz, 2016/06/14 20:41:47]

Done.

   const NonPrintableKeyEntry* result = std::lower_bound(
       std::begin(kNonPrintableKeyMap), std::end(kNonPrintableKeyMap), key_code,
       [](const NonPrintableKeyEntry& entry, KeyboardCode needle) {
         return entry.key_code < needle;
       });
   if (result != std::end(kNonPrintableKeyMap) && result->key_code == key_code)
     return result->dom_key;
-  return DomKey::NONE;
+  // 2. Look up language specific key map.
+  return LangSpecificNonPrintableKeyboardCodeToDomKey(key_code, layout);
 }
 
 void CleanupKeyMapTls(void* data) {
   PlatformKeyMap* key_map = reinterpret_cast<PlatformKeyMap*>(data);
   delete key_map;
 }
 
 struct PlatformKeyMapInstanceTlsTraits
     : public base::DefaultLazyInstanceTraits<base::ThreadLocalStorage::Slot> {
   static base::ThreadLocalStorage::Slot* New(void* instance) {
@@ skipping 13 matching lines @@
 
 PlatformKeyMap::PlatformKeyMap(HKL layout) {
   UpdateLayout(layout);
 }
 
 PlatformKeyMap::~PlatformKeyMap() {}
 
 DomKey PlatformKeyMap::DomKeyFromNativeImpl(DomCode code,
                                             KeyboardCode key_code,
                                             int flags) const {
-  DomKey key = NonPrintableKeyboardCodeToDomKey(key_code);
+  DomKey key = NonPrintableKeyboardCodeToDomKey(key_code, keyboard_layout_);
   if (key != DomKey::NONE)
     return key;
 
-  // TODO(chongz): Handle VKEY_KANA/VKEY_HANGUL, VKEY_HANJA/VKEY_KANJI based on
-  // layout.
-  // https://crbug.com/612736
-
   if (KeycodeConverter::DomCodeToLocation(code) == DomKeyLocation::NUMPAD) {
     // Derived the DOM Key value from |key_code| instead of |code|, to address
     // Windows Numlock/Shift interaction - see crbug.com/594552.
     return NumPadKeyCodeToDomKey(key_code);
   }
 
   const int flags_to_try[] = {
       // Trying to match Firefox's behavior and UIEvents DomKey guidelines.
       // If the combination doesn't produce a printable character, the key value
       // should be the key with no modifiers except for Shift and AltGr.
@@ skipping 88 matching lines @@
         }
       } else {
         // TODO(chongz): Handle rv <= -2 and rv >= 2.
       }
     }
   }
   ::SetKeyboardState(keyboard_state_to_restore);
 }
 
 }  // namespace ui