Commit a49a9d8c authored by Chong Zhang's avatar Chong Zhang Committed by Commit Bot

[Mac] Change DomKey filter from 'non-printable' to 'non-control' characters

According to the latest spec we should filter non-glyph modifiers if the
produced characters are part of Unicode 'Other, Control' General Category.
e.g. http://www.fileformat.info/info/unicode/category/Cc/list.htm

This patch changes implementation to match spec and fixes issues on
'Dvorak - QWERTY Command' layout.

Spec: https://w3c.github.io/uievents-key/#selecting-key-attribute-values

Bug: 747358
Change-Id: I792b472132ea6deb4213206593136343a2050ccd
Reviewed-on: https://chromium-review.googlesource.com/585638Reviewed-by: default avatarGary Kacmarcik <garykac@chromium.org>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Commit-Queue: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491490}
parent 359350cd
...@@ -379,6 +379,35 @@ TEST(WebInputEventBuilderMacTest, USDvorakAlnumNSEventToKeyCode) { ...@@ -379,6 +379,35 @@ TEST(WebInputEventBuilderMacTest, USDvorakAlnumNSEventToKeyCode) {
} }
} }
// 'Dvorak - QWERTY Command' layout will map the key back to QWERTY when Command
// is pressed.
// e.g. Key 'b' maps to 'x' but 'Command-b' remains 'Command-b'.
TEST(WebInputEventBuilderMacTest, USDvorakQWERTYCommand) {
struct DomKeyTestCase {
int mac_key_code;
unichar cmd_character;
} table[] = {{kVK_ANSI_0, '0'}, {kVK_ANSI_1, '1'}, {kVK_ANSI_2, '2'},
{kVK_ANSI_3, '3'}, {kVK_ANSI_4, '4'}, {kVK_ANSI_5, '5'},
{kVK_ANSI_6, '6'}, {kVK_ANSI_7, '7'}, {kVK_ANSI_8, '8'},
{kVK_ANSI_9, '9'}, {kVK_ANSI_A, 'a'}, {kVK_ANSI_B, 'b'},
{kVK_ANSI_C, 'c'}, {kVK_ANSI_D, 'd'}, {kVK_ANSI_E, 'e'},
{kVK_ANSI_F, 'f'}, {kVK_ANSI_G, 'g'}, {kVK_ANSI_H, 'h'},
{kVK_ANSI_I, 'i'}, {kVK_ANSI_J, 'j'}, {kVK_ANSI_K, 'k'},
{kVK_ANSI_L, 'l'}, {kVK_ANSI_M, 'm'}, {kVK_ANSI_N, 'n'},
{kVK_ANSI_O, 'o'}, {kVK_ANSI_P, 'p'}, {kVK_ANSI_Q, 'q'},
{kVK_ANSI_R, 'r'}, {kVK_ANSI_S, 's'}, {kVK_ANSI_T, 't'},
{kVK_ANSI_U, 'u'}, {kVK_ANSI_V, 'v'}, {kVK_ANSI_W, 'w'},
{kVK_ANSI_X, 'x'}, {kVK_ANSI_Y, 'y'}, {kVK_ANSI_Z, 'z'}};
for (const DomKeyTestCase& entry : table) {
NSEvent* mac_event = BuildFakeKeyEvent(
entry.mac_key_code, entry.cmd_character, NSCommandKeyMask, NSKeyDown);
WebKeyboardEvent web_event = WebKeyboardEventBuilder::Build(mac_event);
EXPECT_EQ(ui::DomKey::FromCharacter(entry.cmd_character),
web_event.dom_key);
}
}
// Test conversion from key combination with Control to DomKey. // Test conversion from key combination with Control to DomKey.
// TODO(chongz): Move DomKey tests for all platforms into one place. // TODO(chongz): Move DomKey tests for all platforms into one place.
// http://crbug.com/587589 // http://crbug.com/587589
......
...@@ -18,6 +18,12 @@ namespace ui { ...@@ -18,6 +18,12 @@ namespace ui {
namespace { namespace {
bool IsUnicodeControl(unichar c) {
// C0 control characters: http://unicode.org/charts/PDF/U0000.pdf
// C1 control characters: http://unicode.org/charts/PDF/U0080.pdf
return c <= 0x1F || (c >= 0x7F && c <= 0x9F);
}
// This value is not defined but shows up as 0x36. // This value is not defined but shows up as 0x36.
const int kVK_RightCommand = 0x36; const int kVK_RightCommand = 0x36;
// Context menu is not defined but shows up as 0x6E. // Context menu is not defined but shows up as 0x6E.
...@@ -850,18 +856,10 @@ DomKey DomKeyFromNSEvent(NSEvent* event) { ...@@ -850,18 +856,10 @@ DomKey DomKeyFromNSEvent(NSEvent* event) {
// e.g. On French keyboard [+a will produce "^q", DomKey should be 'q'. // e.g. On French keyboard [+a will produce "^q", DomKey should be 'q'.
unichar dom_key_char = unichar dom_key_char =
[characters characterAtIndex:[characters length] - 1]; [characters characterAtIndex:[characters length] - 1];
const bool is_ctrl_down = ([event modifierFlags] & NSControlKeyMask) && if (IsUnicodeControl(dom_key_char)) {
!([event modifierFlags] & NSAlternateKeyMask); // Filter non-glyph modifiers if the generated characters are part of
const bool is_command_down = [event modifierFlags] & NSCommandKeyMask; // Unicode 'Other, Control' General Category.
// On Mac Blink won't insert ASCII character if either Ctrl or Command, or // https://w3c.github.io/uievents-key/#selecting-key-attribute-values
// both, are down.
// See EditingBehavior::shouldInsertCharacter()
if (std::iscntrl(dom_key_char) ||
(dom_key_char < 0x80 && (is_ctrl_down || is_command_down))) {
// 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 unused_is_dead_key; bool unused_is_dead_key;
const int kAllowedModifiersMask = const int kAllowedModifiersMask =
NSShiftKeyMask | NSAlphaShiftKeyMask | NSAlternateKeyMask; NSShiftKeyMask | NSAlphaShiftKeyMask | NSAlternateKeyMask;
...@@ -870,7 +868,10 @@ DomKey DomKeyFromNSEvent(NSEvent* event) { ...@@ -870,7 +868,10 @@ DomKey DomKeyFromNSEvent(NSEvent* event) {
[event keyCode], [event modifierFlags] & kAllowedModifiersMask, [event keyCode], [event modifierFlags] & kAllowedModifiersMask,
&unused_is_dead_key); &unused_is_dead_key);
} }
if (!std::iscntrl(dom_key_char))
// We need to check again because keys like ESC will produce control
// characters even without any modifiers.
if (!IsUnicodeControl(dom_key_char))
return DomKeyFromCharCode(dom_key_char); return DomKeyFromCharCode(dom_key_char);
} }
} }
......
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