Commit 3f4cf0ae authored by chongz's avatar chongz Committed by Commit bot

Original issue's description:

> [DomKey] Expose Korean special keys on Korean keyboard layout only
>
> Korean keyboard has special keys "HangulMode" and "HanjaMode", we should
> only expose them when the system layout is Korean. Otherwise we should
> return "Unidentified".
>
> This matches Firefox and windows virtual key.
>
> Note: This also works with JIS keyboard + Korean layout, because we are
> mapping DomKey based on KeyboardCode instead of DomCode.
>
> BUG=612736
>
> Committed: https://crrev.com/bdcfb5a1816f1bad537d7611c7ccb972b550339c
> Cr-Commit-Position: refs/heads/master@{#399783}
> Revert: https://codereview.chromium.org/2061123005

Original issue:
Windows API |LoadKeyboardLayout()| won't pass DrMemory
tests for Korean and Japanese locale.
This should be an issue of DrMemory test or Windows itself.
See https://crbug.com/612736#c11

Solution:
Just skip |LoadKeyboardLayout()| and hardcode locale for Korean,
it works because we are only testing non-printable keys, which
doesn't care about |ToUnicodeEx()|.

Note:
|ToUnicodeEx()| returns empty string if the specified locale is invalid. (e.g. Korean HKL without calling 'LoadKeyboardLayout()')

Also Note:
"mock_keyboard.h" has the similar issue, e.g.
Call "MockKeyboard::GetCharacters(MockKeyboard::LAYOUT_KOREAN, ..)" somewhere and run DrMemory full test, which will fail as well.

BUG=612736

Review-Url: https://codereview.chromium.org/2097643002
Cr-Commit-Position: refs/heads/master@{#402611}
parent 6ca90e6b
......@@ -183,9 +183,6 @@ const struct NonPrintableKeyEntry {
{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},
......@@ -197,7 +194,30 @@ const struct NonPrintableKeyEntry {
{VKEY_OEM_CLEAR, DomKey::CLEAR},
};
DomKey NonPrintableKeyboardCodeToDomKey(KeyboardCode key_code) {
// Disambiguates the meaning of certain non-printable keys which have different
// meanings under different languages, but use the same VKEY code.
DomKey LanguageSpecificOemKeyboardCodeToDomKey(KeyboardCode key_code,
HKL layout) {
WORD language = LOWORD(layout);
WORD primary_language = PRIMARYLANGID(language);
if (primary_language == LANG_KOREAN) {
switch (key_code) {
case VKEY_HANGUL:
return DomKey::HANGUL_MODE;
case VKEY_HANJA:
return DomKey::HANJA_MODE;
default:
return DomKey::NONE;
}
}
// TODO(chongz): Handle Japanese keyboard layout keys 0xF0..0xF5, VKEY_KANA,
// VKEY_KANJI.
// https://crbug.com/612694
return DomKey::NONE;
}
DomKey NonPrintableKeyboardCodeToDomKey(KeyboardCode key_code, HKL layout) {
// 1. Most |key_codes| have the same meaning regardless of |layout|.
const NonPrintableKeyEntry* result = std::lower_bound(
std::begin(kNonPrintableKeyMap), std::end(kNonPrintableKeyMap), key_code,
[](const NonPrintableKeyEntry& entry, KeyboardCode needle) {
......@@ -205,7 +225,8 @@ DomKey NonPrintableKeyboardCodeToDomKey(KeyboardCode key_code) {
});
if (result != std::end(kNonPrintableKeyMap) && result->key_code == key_code)
return result->dom_key;
return DomKey::NONE;
// 2. Look up a |layout|-specific meaning for |key_code|.
return LanguageSpecificOemKeyboardCodeToDomKey(key_code, layout);
}
void CleanupKeyMapTls(void* data) {
......@@ -238,14 +259,10 @@ PlatformKeyMap::~PlatformKeyMap() {}
DomKey PlatformKeyMap::DomKeyFromKeyboardCodeImpl(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
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
......
......@@ -17,8 +17,37 @@ namespace ui {
namespace {
const wchar_t* LAYOUT_US = L"00000409";
const wchar_t* LAYOUT_FR = L"0000040c";
enum Layout {
LAYOUT_US,
LAYOUT_FR,
LAYOUT_KR,
};
// |LoadKeyboardLayout()| ensures the locale to be loaded into the system
// (Similar to temporarily adding a locale in Control Panel), otherwise
// |ToUnicodeEx()| will fall-back to the default locale.
// See MSDN LoadKeyboardLayout():
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms646305(v=vs.85).aspx
// And language constants and strings:
// https://msdn.microsoft.com/en-us/library/windows/desktop/dd318693(v=vs.85).aspx
HKL GetInputLocale(Layout layout) {
switch (layout) {
case LAYOUT_US:
return ::LoadKeyboardLayout(L"00000409", KLF_ACTIVATE);
case LAYOUT_FR:
return ::LoadKeyboardLayout(L"0000040c", KLF_ACTIVATE);
case LAYOUT_KR:
// |LoadKeyboardLayout(L"00000412", KLF_ACTIVATE)| returns the correct
// Korean locale, but it will fail on DrMemory tests.
// See https://crbug.com/612736#c6
// However we could bypass it since we are only testing non-printable keys
// on Korean locale.
// (This issue only happens on Korean and Japanese).
return reinterpret_cast<HKL>(0x04120412);
default:
return 0;
}
}
struct TestKey {
KeyboardCode key_code;
......@@ -94,7 +123,7 @@ class PlatformKeyMapTest : public testing::Test {
};
TEST_F(PlatformKeyMapTest, USLayout) {
HKL layout = ::LoadKeyboardLayout(LAYOUT_US, 0);
HKL layout = GetInputLocale(LAYOUT_US);
PlatformKeyMap keymap(layout);
const TestKey kUSLayoutTestCases[] = {
......@@ -143,7 +172,7 @@ TEST_F(PlatformKeyMapTest, USLayout) {
}
TEST_F(PlatformKeyMapTest, FRLayout) {
HKL layout = ::LoadKeyboardLayout(LAYOUT_FR, 0);
HKL layout = GetInputLocale(LAYOUT_FR);
PlatformKeyMap keymap(layout);
const TestKey kFRLayoutTestCases[] = {
......@@ -192,7 +221,7 @@ TEST_F(PlatformKeyMapTest, FRLayout) {
}
TEST_F(PlatformKeyMapTest, NumPad) {
HKL layout = ::LoadKeyboardLayout(LAYOUT_US, 0);
HKL layout = GetInputLocale(LAYOUT_US);
PlatformKeyMap keymap(layout);
const struct TestCase {
......@@ -242,7 +271,7 @@ TEST_F(PlatformKeyMapTest, NumPad) {
}
TEST_F(PlatformKeyMapTest, NonPrintableKey) {
HKL layout = ::LoadKeyboardLayout(LAYOUT_US, 0);
HKL layout = GetInputLocale(LAYOUT_US);
PlatformKeyMap keymap(layout);
for (const auto& test_case : kNonPrintableCodeMap) {
......@@ -278,4 +307,31 @@ TEST_F(PlatformKeyMapTest, NonPrintableKey) {
}
}
TEST_F(PlatformKeyMapTest, KoreanSpecificKeys) {
const struct TestCase {
KeyboardCode key_code;
DomKey key;
} kKoreanTestCases[] = {
{VKEY_HANGUL, DomKey::HANGUL_MODE}, {VKEY_HANJA, DomKey::HANJA_MODE},
};
// US English should not return values for these keys.
HKL us_layout = GetInputLocale(LAYOUT_US);
PlatformKeyMap us_keymap(us_layout);
for (const auto& test_case : kKoreanTestCases) {
EXPECT_EQ(DomKey::NONE, DomKeyFromKeyboardCodeImpl(
us_keymap, test_case.key_code, EF_NONE))
<< test_case.key_code;
}
// Korean layout should return specific DomKey.
HKL ko_layout = GetInputLocale(LAYOUT_KR);
PlatformKeyMap ko_keymap(ko_layout);
for (const auto& test_case : kKoreanTestCases) {
EXPECT_EQ(test_case.key, DomKeyFromKeyboardCodeImpl(
ko_keymap, test_case.key_code, EF_NONE))
<< test_case.key_code;
}
}
} // namespace ui
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment