Commit c2fffcee authored by Andrew Xu's avatar Andrew Xu Committed by Commit Bot

Fix bug that Capslock is triggered unintendedly

Modify the function CanHandleToggleCapsLock: pressing any
redundant key will prevent triggering the Capslock shortcut.

In original code, pressing Alt + M + Search then releasing Alt
will trigger CapsLock. Because AcceleratorHistory will receive
Alt + Search when Search is pressed. The fact that key M is pr-
-essed is ignored.

R=afakhry@chromium.org

Bug: 789283
Test: ash unittests
Change-Id: Id359d11cda4562a0b90da97af6c3dde3faf33c53
Reviewed-on: https://chromium-review.googlesource.com/1199853
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589702}
parent 8943110d
...@@ -752,8 +752,18 @@ void HandleCycleUser(CycleUserDirection direction) { ...@@ -752,8 +752,18 @@ void HandleCycleUser(CycleUserDirection direction) {
Shell::Get()->session_controller()->CycleActiveUser(direction); Shell::Get()->session_controller()->CycleActiveUser(direction);
} }
bool CanHandleToggleCapsLock(const ui::Accelerator& accelerator, bool CanHandleToggleCapsLock(
const ui::Accelerator& previous_accelerator) { const ui::Accelerator& accelerator,
const ui::Accelerator& previous_accelerator,
const std::set<ui::KeyboardCode>& currently_pressed_keys) {
// Iterate the set of pressed keys. If any redundant key is pressed, CapsLock
// should not be triggered. Otherwise, CapsLock may be triggered accidentally.
// See issue 789283 (https://crbug.com/789283)
for (const auto& pressed_key : currently_pressed_keys) {
if (pressed_key != ui::VKEY_LWIN && pressed_key != ui::VKEY_MENU)
return false;
}
// This shortcust is set to be trigger on release. Either the current // This shortcust is set to be trigger on release. Either the current
// accelerator is a Search release or Alt release. // accelerator is a Search release or Alt release.
if (accelerator.key_code() == ui::VKEY_LWIN && if (accelerator.key_code() == ui::VKEY_LWIN &&
...@@ -1309,7 +1319,9 @@ bool AcceleratorController::CanPerformAction( ...@@ -1309,7 +1319,9 @@ bool AcceleratorController::CanPerformAction(
case TOGGLE_APP_LIST: case TOGGLE_APP_LIST:
return CanHandleToggleAppList(accelerator, previous_accelerator); return CanHandleToggleAppList(accelerator, previous_accelerator);
case TOGGLE_CAPS_LOCK: case TOGGLE_CAPS_LOCK:
return CanHandleToggleCapsLock(accelerator, previous_accelerator); return CanHandleToggleCapsLock(
accelerator, previous_accelerator,
accelerator_history_->currently_pressed_keys());
case TOGGLE_DICTATION: case TOGGLE_DICTATION:
return CanHandleToggleDictation(); return CanHandleToggleDictation();
case TOGGLE_DOCKED_MAGNIFIER: case TOGGLE_DOCKED_MAGNIFIER:
......
...@@ -975,6 +975,17 @@ TEST_F(AcceleratorControllerTest, ToggleCapsLockAccelerators) { ...@@ -975,6 +975,17 @@ TEST_F(AcceleratorControllerTest, ToggleCapsLockAccelerators) {
EXPECT_EQ(4, client.set_caps_lock_count_); EXPECT_EQ(4, client.set_caps_lock_count_);
EXPECT_TRUE(controller->IsCapsLockEnabled()); EXPECT_TRUE(controller->IsCapsLockEnabled());
controller->UpdateCapsLockState(false); controller->UpdateCapsLockState(false);
// 5. Press M, Press Alt, Press Search, Release Alt. After that CapsLock
// should not be triggered. https://crbug.com/789283
ui::test::EventGenerator* generator = GetEventGenerator();
generator->PressKey(ui::VKEY_M, ui::EF_NONE);
generator->PressKey(ui::VKEY_MENU, ui::EF_NONE);
generator->PressKey(ui::VKEY_LWIN, ui::EF_ALT_DOWN);
generator->ReleaseKey(ui::VKEY_MENU, ui::EF_COMMAND_DOWN);
controller->FlushMojoForTesting();
EXPECT_FALSE(controller->IsCapsLockEnabled());
controller->UpdateCapsLockState(false);
} }
class PreferredReservedAcceleratorsTest : public AshTestBase { class PreferredReservedAcceleratorsTest : public AshTestBase {
......
...@@ -31,6 +31,10 @@ class UI_BASE_EXPORT AcceleratorHistory { ...@@ -31,6 +31,10 @@ class UI_BASE_EXPORT AcceleratorHistory {
return previous_accelerator_; return previous_accelerator_;
} }
const std::set<KeyboardCode>& currently_pressed_keys() const {
return currently_pressed_keys_;
}
// Stores the given |accelerator| only if it's different than the currently // Stores the given |accelerator| only if it's different than the currently
// stored one. // stored one.
void StoreCurrentAccelerator(const Accelerator& accelerator); void StoreCurrentAccelerator(const Accelerator& accelerator);
......
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