Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Makes Search+? accelerator work for colemak keyboard.
Browse files Browse the repository at this point in the history
For certain XKB key mapping, e.g. colemak, the modifier key is mapped to
non-modifier, e.g. Search (identical to WIN key) maps to backspace.
In such case, event rewriter can rewrite the key back to modifier key
successfully but fails to rewrite the modifier flag to the later keys,
because the later keys has no modifier flag, so nothing to rewrite.

This cl leverages the "latches" flag state in event rewriter to make the
remapped modifier flag can be remembered and apply to the later keys.

BUG=215923
TEST=Activates colemak keyboard, press Search+U (which is Search+L in
colemak key mapping), the screen can be locked.

Review URL: https://codereview.chromium.org/1473773004

Cr-Commit-Position: refs/heads/master@{#362088}
(cherry picked from commit 27aa4f7)

Review URL: https://codereview.chromium.org/1486423004 .

Cr-Commit-Position: refs/branch-heads/2564@{#200}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
  • Loading branch information
shuchen-google committed Dec 2, 2015
1 parent a691be7 commit 8761add
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 22 deletions.
30 changes: 16 additions & 14 deletions chrome/browser/chromeos/events/event_rewriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ EventRewriter::EventRewriter(ash::StickyKeysController* sticky_keys_controller)
ime_keyboard_for_testing_(NULL),
pref_service_for_testing_(NULL),
sticky_keys_controller_(sticky_keys_controller),
current_diamond_key_modifier_flags_(ui::EF_NONE),
pressed_modifier_latches_(ui::EF_NONE),
latched_modifier_latches_(ui::EF_NONE),
used_modifier_latches_(ui::EF_NONE) {}
Expand Down Expand Up @@ -430,8 +429,7 @@ int EventRewriter::GetRemappedModifierMasks(const PrefService& pref_service,
const ui::Event& event,
int original_flags) const {
int unmodified_flags = original_flags;
int rewritten_flags = current_diamond_key_modifier_flags_ |
pressed_modifier_latches_ | latched_modifier_latches_;
int rewritten_flags = pressed_modifier_latches_ | latched_modifier_latches_;
for (size_t i = 0; unmodified_flags && (i < arraysize(kModifierRemappings));
++i) {
const ModifierRemapping* remapped_key = NULL;
Expand Down Expand Up @@ -737,15 +735,6 @@ bool EventRewriter::RewriteModifierKeys(const ui::KeyEvent& key_event,
DCHECK_EQ(ui::VKEY_CONTROL, kModifierRemappingCtrl->result.key_code);
remapped_key = kModifierRemappingCtrl;
}
// F15 is not a modifier key, so we need to track its state directly.
if (key_event.type() == ui::ET_KEY_PRESSED) {
int remapped_flag = remapped_key->flag;
if (remapped_key->remap_to == input_method::kCapsLockKey)
remapped_flag |= ui::EF_CAPS_LOCK_DOWN;
current_diamond_key_modifier_flags_ = remapped_flag;
} else {
current_diamond_key_modifier_flags_ = ui::EF_NONE;
}
break;
// On Chrome OS, XF86XK_Launch7 (F16) with Mod3Mask is sent when Caps Lock
// is pressed (with one exception: when
Expand Down Expand Up @@ -793,17 +782,30 @@ bool EventRewriter::RewriteModifierKeys(const ui::KeyEvent& key_event,
state->key = remapped_key->result.key;
incoming.flags |= characteristic_flag;
characteristic_flag = remapped_key->flag;
if (remapped_key->remap_to == input_method::kCapsLockKey)
characteristic_flag |= ui::EF_CAPS_LOCK_DOWN;
state->code = RelocateModifier(
state->code, ui::KeycodeConverter::DomCodeToLocation(incoming.code));
}

// Next, remap modifier bits.
state->flags |=
GetRemappedModifierMasks(*pref_service, key_event, incoming.flags);
if (key_event.type() == ui::ET_KEY_PRESSED)

// If the DomKey is not a modifier before remapping but is after, set the
// modifier latches for the later non-modifier key's modifier states.
bool non_modifier_to_modifier =
!ui::KeycodeConverter::IsDomKeyForModifier(incoming.key) &&
ui::KeycodeConverter::IsDomKeyForModifier(state->key);
if (key_event.type() == ui::ET_KEY_PRESSED) {
state->flags |= characteristic_flag;
else
if (non_modifier_to_modifier)
pressed_modifier_latches_ |= characteristic_flag;
} else {
state->flags &= ~characteristic_flag;
if (non_modifier_to_modifier)
pressed_modifier_latches_ &= ~characteristic_flag;
}

if (key_event.type() == ui::ET_KEY_PRESSED) {
if (!ui::KeycodeConverter::IsDomKeyForModifier(state->key)) {
Expand Down
9 changes: 2 additions & 7 deletions chrome/browser/chromeos/events/event_rewriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,6 @@ class EventRewriter : public ui::EventRewriter {
// at time of writing it is a singleton in ash::Shell.
ash::StickyKeysController* sticky_keys_controller_;

// The ChromeOS Diamond key arrives as F15. Since F15 is not a modifier,
// we need to track its pressed state explicitly, and apply the selected
// modifier flag to key and mouse presses that arrive while F15 is down.
// While the Diamond key is down, this holds the corresponding modifier
// ui::EventFlags; otherwise it is EF_NONE.
int current_diamond_key_modifier_flags_;

// Some keyboard layouts have 'latching' keys, which either apply
// a modifier while held down (like normal modifiers), or, if no
// non-modifier is pressed while the latching key is down, apply the
Expand All @@ -196,6 +189,8 @@ class EventRewriter : public ui::EventRewriter {
// here, sticky keys, and the system layer (X11 or Ozone), and could
// do with refactoring.
// - |pressed_modifier_latches_| records the latching keys currently pressed.
// It also records the active modifier flags for non-modifier keys that are
// remapped to modifiers, e.g. Diamond/F15.
// - |latched_modifier_latches_| records the latching keys just released,
// to be applied to the next non-modifier key.
// - |used_modifier_latches_| records the latching keys applied to a non-
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/chromeos/events/event_rewriter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,8 @@ TEST_F(EventRewriterTest, TestRewriteModifiersRemapToCapsLock) {

// Press Search.
EXPECT_EQ(GetExpectedResultAsString(ui::ET_KEY_PRESSED, ui::VKEY_CAPITAL,
ui::DomCode::CAPS_LOCK, ui::EF_MOD3_DOWN,
ui::DomCode::CAPS_LOCK,
ui::EF_MOD3_DOWN | ui::EF_CAPS_LOCK_DOWN,
ui::DomKey::CAPS_LOCK),
GetRewrittenEventAsString(&rewriter, ui::ET_KEY_PRESSED,
ui::VKEY_LWIN, ui::DomCode::OS_LEFT,
Expand Down

0 comments on commit 8761add

Please sign in to comment.