Commit d4fa1fc7 authored by Joel Hockey's avatar Joel Hockey Committed by Commit Bot

Use KeyboardEvent.keyCode in addition to KeyboardEvent.key for

shortcut handling to work with RU keyboard.

Added tests to verify expected behavior for US, DV, RU keyboards.

Bug: 725869
Change-Id: I9b81f0bca5fed786fa5b152e25b646cd188ca2a4
Reviewed-on: https://chromium-review.googlesource.com/c/1341444Reviewed-by: default avatarNaoki Fukino <fukino@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610303}
parent b67e8dfa
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
<title>cr.ui.Command test</title> <title>cr.ui.Command test</title>
</head> </head>
<body> <body>
<command></command> <command shortcut="n|Ctrl"></command>
<script> <script>
function testCommandDefaultPrevented() { function testCommandDefaultPrevented() {
...@@ -21,6 +21,31 @@ function testCommandDefaultPrevented() { ...@@ -21,6 +21,31 @@ function testCommandDefaultPrevented() {
assertEquals(1, calls); assertEquals(1, calls);
} }
function createEvent(key, code, keyCode) {
return {
key: key,
code: code,
keyCode: keyCode,
altKey: false,
ctrlKey: true,
metaKey: false,
shiftKey: false
};
}
function testShortcuts() {
cr.ui.decorate('command', cr.ui.Command);
const cmd = document.querySelector('command');
// US keyboard - qwerty-N should work.
assertTrue(cmd.matchesEvent(createEvent('n', 'KeyN', 0x4e)));
// DV keyboard - qwerty-L (dvorak-N) should work.
assertTrue(cmd.matchesEvent(createEvent('n', 'KeyL', 0x4e)));
// DV keyboard - qwerty-N (dvorak-B) should not work.
assertFalse(cmd.matchesEvent(createEvent('b', 'KeyN', 0x42)));
// RU keyboard - qwerty-N (Cyrillic Te) should work.
assertTrue(cmd.matchesEvent(createEvent('т', 'KeyN', 0x4e)));
}
</script> </script>
</body> </body>
</html> </html>
...@@ -24,26 +24,30 @@ cr.define('cr.ui', function() { ...@@ -24,26 +24,30 @@ cr.define('cr.ui', function() {
* @constructor * @constructor
*/ */
function KeyboardShortcut(shortcut) { function KeyboardShortcut(shortcut) {
var mods = {}; this.useKeyCode_ = false;
var ident = ''; this.mods_ = {};
shortcut.split('|').forEach(function(part) { shortcut.split('|').forEach((part) => {
var partLc = part.toLowerCase(); var partLc = part.toLowerCase();
switch (partLc) { switch (partLc) {
case 'alt': case 'alt':
case 'ctrl': case 'ctrl':
case 'meta': case 'meta':
case 'shift': case 'shift':
mods[partLc + 'Key'] = true; this.mods_[partLc + 'Key'] = true;
break; break;
default: default:
if (ident) if (this.key_)
throw Error('Invalid shortcut'); throw Error('Invalid shortcut');
ident = part; this.key_ = part;
// For single key alpha shortcuts use event.keyCode rather than
// event.key to match how chrome handles shortcuts and allow
// non-english language input to work.
if (part.match(/^[a-z]$/)) {
this.useKeyCode_ = true;
this.keyCode_ = part.toUpperCase().charCodeAt(0);
}
} }
}); });
this.ident_ = ident;
this.mods_ = mods;
} }
KeyboardShortcut.prototype = { KeyboardShortcut.prototype = {
...@@ -53,8 +57,9 @@ cr.define('cr.ui', function() { ...@@ -53,8 +57,9 @@ cr.define('cr.ui', function() {
* @return {boolean} Whether we found a match or not. * @return {boolean} Whether we found a match or not.
*/ */
matchesEvent: function(e) { matchesEvent: function(e) {
if (e.key == this.ident_) { if ((this.useKeyCode_ && e.keyCode == this.keyCode_) ||
// All keyboard modifiers needs to match. e.key == this.key_) {
// All keyboard modifiers need to match.
var mods = this.mods_; var mods = this.mods_;
return ['altKey', 'ctrlKey', 'metaKey', 'shiftKey'].every(function(k) { return ['altKey', 'ctrlKey', 'metaKey', 'shiftKey'].every(function(k) {
return e[k] == !!mods[k]; return e[k] == !!mods[k];
......
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