From e76096a5f9c6afac758789ed4b96be43d99802ea Mon Sep 17 00:00:00 2001 From: "rsadam@chromium.org" Date: Tue, 8 Apr 2014 04:08:03 +0000 Subject: [PATCH] If the user clicks on a key on the accessibility virtual keyboard the cursor disappears, which makes typing the next key difficult. This patch disables the hide cursor whenever the accessibility keyboard is enabled. Added class ash::ash_cursor_manager which extends wm::cursor_manager, and moved the logic of whether or not to hide the cursor on key events to here. Follow up bug to add unittest: crbug.com/359324 BUG=355094 Review URL: https://codereview.chromium.org/213743013 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262299 0039d316-1c4b-4281-b951-d872f2087c98 --- ash/ash.gyp | 2 + ash/shell.h | 8 +++ ...ative_cursor_manager_interactive_uitest.cc | 4 +- ash/wm/ash_native_cursor_manager_unittest.cc | 12 ++-- ash/wm/cursor_manager_chromeos.cc | 55 ++++++++++++++++++ ash/wm/cursor_manager_chromeos.h | 43 ++++++++++++++ ui/aura/client/cursor_client.h | 7 +++ ui/aura/test/test_cursor_client.cc | 5 ++ ui/aura/test/test_cursor_client.h | 6 ++ ui/wm/core/compound_event_filter.cc | 58 ++----------------- ui/wm/core/cursor_manager.cc | 5 ++ ui/wm/core/cursor_manager.h | 8 ++- 12 files changed, 150 insertions(+), 63 deletions(-) create mode 100644 ash/wm/cursor_manager_chromeos.cc create mode 100644 ash/wm/cursor_manager_chromeos.h diff --git a/ash/ash.gyp b/ash/ash.gyp index a1b4038515495a..0d62c8751c5fc2 100644 --- a/ash/ash.gyp +++ b/ash/ash.gyp @@ -518,6 +518,8 @@ 'wm/ash_focus_rules.h', 'wm/boot_splash_screen_chromeos.cc', 'wm/boot_splash_screen_chromeos.h', + 'wm/cursor_manager_chromeos.cc', + 'wm/cursor_manager_chromeos.h', 'wm/coordinate_conversion.cc', 'wm/coordinate_conversion.h', 'wm/default_state.cc', diff --git a/ash/shell.h b/ash/shell.h index 0692bc1ea56c67..e8ce236d794702 100644 --- a/ash/shell.h +++ b/ash/shell.h @@ -12,6 +12,7 @@ #include "ash/metrics/user_metrics_recorder.h" #include "ash/shelf/shelf_types.h" #include "ash/system/user/login_status.h" +#include "ash/wm/cursor_manager_chromeos.h" #include "ash/wm/system_modal_container_event_filter_delegate.h" #include "base/basictypes.h" #include "base/compiler_specific.h" @@ -720,7 +721,14 @@ class ASH_EXPORT Shell : public SystemModalContainerEventFilterDelegate, // |native_cursor_manager_| is owned by |cursor_manager_|, but we keep a // pointer to vend to test code. AshNativeCursorManager* native_cursor_manager_; + +// Cursor may be hidden on certain key events in ChromeOS, whereas we never hide +// the cursor on Windows. +#if defined(OS_CHROMEOS) + CursorManager cursor_manager_; +#else // !defined(OS_CHROMEOS) ::wm::CursorManager cursor_manager_; +#endif // defined(OS_CHROMEOS) ObserverList observers_; diff --git a/ash/wm/ash_native_cursor_manager_interactive_uitest.cc b/ash/wm/ash_native_cursor_manager_interactive_uitest.cc index 48d2af589c5bd5..5084e7c179d898 100644 --- a/ash/wm/ash_native_cursor_manager_interactive_uitest.cc +++ b/ash/wm/ash_native_cursor_manager_interactive_uitest.cc @@ -25,8 +25,6 @@ namespace ash { -using ::wm::CursorManager; - class AshNativeCursorManagerTest : public test::AshTestBase { public: AshNativeCursorManagerTest() {} @@ -90,7 +88,7 @@ void MoveMouseSync(aura::Window* window, int x, int y) { #endif TEST_F(AshNativeCursorManagerTest, MAYBE_CursorChangeOnEnterNotify) { - CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); + ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); test::CursorManagerTestApi test_api(cursor_manager); DisplayManager* display_manager = Shell::GetInstance()->display_manager(); diff --git a/ash/wm/ash_native_cursor_manager_unittest.cc b/ash/wm/ash_native_cursor_manager_unittest.cc index 5d9ab1e49b2de1..141e8a8f1afc85 100644 --- a/ash/wm/ash_native_cursor_manager_unittest.cc +++ b/ash/wm/ash_native_cursor_manager_unittest.cc @@ -19,8 +19,6 @@ #include "ui/base/cursor/cursor_loader_win.h" #endif -using ::wm::CursorManager; - namespace ash { namespace test { @@ -54,7 +52,7 @@ class MouseEventLocationDelegate : public aura::test::TestWindowDelegate { typedef test::AshTestBase AshNativeCursorManagerTest; TEST_F(AshNativeCursorManagerTest, LockCursor) { - CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); + ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); CursorManagerTestApi test_api(cursor_manager); gfx::Display display(0); #if defined(OS_WIN) @@ -113,7 +111,7 @@ TEST_F(AshNativeCursorManagerTest, LockCursor) { } TEST_F(AshNativeCursorManagerTest, SetCursor) { - CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); + ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); CursorManagerTestApi test_api(cursor_manager); #if defined(OS_WIN) ui::CursorLoaderWin::SetCursorResourceModule(L"ash_unittests.exe"); @@ -127,7 +125,7 @@ TEST_F(AshNativeCursorManagerTest, SetCursor) { } TEST_F(AshNativeCursorManagerTest, SetCursorSet) { - CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); + ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); CursorManagerTestApi test_api(cursor_manager); EXPECT_EQ(ui::CURSOR_SET_NORMAL, test_api.GetCurrentCursorSet()); @@ -143,7 +141,7 @@ TEST_F(AshNativeCursorManagerTest, SetCursorSet) { } TEST_F(AshNativeCursorManagerTest, SetScale) { - CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); + ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); CursorManagerTestApi test_api(cursor_manager); EXPECT_EQ(1.f, test_api.GetCurrentScale()); @@ -156,7 +154,7 @@ TEST_F(AshNativeCursorManagerTest, SetScale) { } TEST_F(AshNativeCursorManagerTest, SetDeviceScaleFactorAndRotation) { - CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); + ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager(); CursorManagerTestApi test_api(cursor_manager); gfx::Display display(0); diff --git a/ash/wm/cursor_manager_chromeos.cc b/ash/wm/cursor_manager_chromeos.cc new file mode 100644 index 00000000000000..39a74d57034c62 --- /dev/null +++ b/ash/wm/cursor_manager_chromeos.cc @@ -0,0 +1,55 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ash/wm/cursor_manager_chromeos.h" + +#include "base/logging.h" +#include "ui/keyboard/keyboard_util.h" +#include "ui/wm/core/cursor_manager.h" +#include "ui/wm/core/native_cursor_manager.h" + +namespace ash { + +CursorManager::CursorManager( + scoped_ptr< ::wm::NativeCursorManager> delegate) + : ::wm::CursorManager(delegate.Pass()) { +} + +CursorManager::~CursorManager() { +} + +bool CursorManager::ShouldHideCursorOnKeyEvent( + const ui::KeyEvent& event) const { + // Clicking on a key when the accessibility virtual keyboard is enabled should + // not hide the cursor. + if (keyboard::GetAccessibilityKeyboardEnabled()) + return false; + // All alt and control key commands are ignored. + if (event.IsAltDown() || event.IsControlDown()) + return false; + + ui::KeyboardCode code = event.key_code(); + if (code >= ui::VKEY_F1 && code <= ui::VKEY_F24) + return false; + if (code >= ui::VKEY_BROWSER_BACK && code <= ui::VKEY_MEDIA_LAUNCH_APP2) + return false; + switch (code) { + // Modifiers. + case ui::VKEY_SHIFT: + case ui::VKEY_CONTROL: + case ui::VKEY_MENU: + // Search key == VKEY_LWIN. + case ui::VKEY_LWIN: + case ui::VKEY_WLAN: + case ui::VKEY_POWER: + case ui::VKEY_BRIGHTNESS_DOWN: + case ui::VKEY_BRIGHTNESS_UP: + case ui::VKEY_KBD_BRIGHTNESS_UP: + case ui::VKEY_KBD_BRIGHTNESS_DOWN: + return false; + default: + return true; + } +} +} // namespace ash diff --git a/ash/wm/cursor_manager_chromeos.h b/ash/wm/cursor_manager_chromeos.h new file mode 100644 index 00000000000000..70cfa1e0ddb08b --- /dev/null +++ b/ash/wm/cursor_manager_chromeos.h @@ -0,0 +1,43 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef ASH_WM_CURSOR_MANAGER_CHROMEOS_H_ +#define ASH_WM_CURSOR_MANAGER_CHROMEOS_H_ + +#include "ash/ash_export.h" +#include "base/basictypes.h" +#include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" +#include "ui/wm/core/cursor_manager.h" +#include "ui/wm/core/native_cursor_manager_delegate.h" + +namespace ui { +class KeyEvent; +} + +namespace wm { +class NativeCursorManager; +} + +namespace ash { + +// This class overrides the cursor hiding behaviour on ChromeOS. The cursor is +// hidden on certain key events only if the accessibility keyboard is not +// enabled. +class ASH_EXPORT CursorManager : public ::wm::CursorManager { + public: + explicit CursorManager( + scoped_ptr< ::wm::NativeCursorManager> delegate); + virtual ~CursorManager(); + + // aura::client::CursorClient: + virtual bool ShouldHideCursorOnKeyEvent( + const ui::KeyEvent& event) const OVERRIDE; + private: + DISALLOW_COPY_AND_ASSIGN(CursorManager); +}; + +} // namespace ash + +#endif // ASH_WM_CURSOR_MANAGER_CHROMEOS_H_ diff --git a/ui/aura/client/cursor_client.h b/ui/aura/client/cursor_client.h index 99d9b6a1a1c3be..8f2872e04169ab 100644 --- a/ui/aura/client/cursor_client.h +++ b/ui/aura/client/cursor_client.h @@ -14,6 +14,10 @@ namespace gfx { class Display; } +namespace ui { +class KeyEvent; +} + namespace aura { class Window; namespace client { @@ -80,6 +84,9 @@ class AURA_EXPORT CursorClient { virtual void AddObserver(CursorClientObserver* observer) = 0; virtual void RemoveObserver(CursorClientObserver* observer) = 0; + // Returns true if the mouse cursor should be hidden on |event|. + virtual bool ShouldHideCursorOnKeyEvent(const ui::KeyEvent& event) const = 0; + protected: virtual ~CursorClient() {} }; diff --git a/ui/aura/test/test_cursor_client.cc b/ui/aura/test/test_cursor_client.cc index f0411ef04b86cd..56625556b7cb5b 100644 --- a/ui/aura/test/test_cursor_client.cc +++ b/ui/aura/test/test_cursor_client.cc @@ -99,5 +99,10 @@ void TestCursorClient::RemoveObserver( observers_.RemoveObserver(observer); } +bool TestCursorClient::ShouldHideCursorOnKeyEvent( + const ui::KeyEvent& event) const { + return true; +} + } // namespace test } // namespace aura diff --git a/ui/aura/test/test_cursor_client.h b/ui/aura/test/test_cursor_client.h index 2d152de3e4ddde..10452f1c3df891 100644 --- a/ui/aura/test/test_cursor_client.h +++ b/ui/aura/test/test_cursor_client.h @@ -10,6 +10,10 @@ #include "base/observer_list.h" #include "ui/aura/client/cursor_client.h" +namespace ui { +class KeyEvent; +} + namespace aura { namespace test { @@ -43,6 +47,8 @@ class TestCursorClient : public aura::client::CursorClient { aura::client::CursorClientObserver* observer) OVERRIDE; virtual void RemoveObserver( aura::client::CursorClientObserver* observer) OVERRIDE; + virtual bool ShouldHideCursorOnKeyEvent( + const ui::KeyEvent& event) const OVERRIDE; private: bool visible_; diff --git a/ui/wm/core/compound_event_filter.cc b/ui/wm/core/compound_event_filter.cc index 6664f722011c40..a49e5d56e521d3 100644 --- a/ui/wm/core/compound_event_filter.cc +++ b/ui/wm/core/compound_event_filter.cc @@ -25,55 +25,8 @@ namespace wm { namespace { -bool ShouldHideCursorOnKeyEvent(const ui::KeyEvent& event) { -#if defined(OS_CHROMEOS) - // All alt and control key commands are ignored. - if (event.IsAltDown() || event.IsControlDown()) - return false; - - static bool inited = false; - static base::hash_set ignored_keys; - if (!inited) { - // Modifiers. - ignored_keys.insert(ui::VKEY_SHIFT); - ignored_keys.insert(ui::VKEY_CONTROL); - ignored_keys.insert(ui::VKEY_MENU); - - // Search key == VKEY_LWIN. - ignored_keys.insert(ui::VKEY_LWIN); - - // Function keys. - for (int key = ui::VKEY_F1; key <= ui::VKEY_F24; ++key) - ignored_keys.insert(key); - - // Media keys. - for (int key = ui::VKEY_BROWSER_BACK; key <= ui::VKEY_MEDIA_LAUNCH_APP2; - ++key) { - ignored_keys.insert(key); - } - -#if defined(OS_POSIX) - ignored_keys.insert(ui::VKEY_WLAN); - ignored_keys.insert(ui::VKEY_POWER); - ignored_keys.insert(ui::VKEY_BRIGHTNESS_DOWN); - ignored_keys.insert(ui::VKEY_BRIGHTNESS_UP); - ignored_keys.insert(ui::VKEY_KBD_BRIGHTNESS_DOWN); - ignored_keys.insert(ui::VKEY_KBD_BRIGHTNESS_UP); -#endif - - inited = true; - } - - if (ignored_keys.count(event.key_code()) > 0) - return false; - - return true; -#else // !defined(OS_CHROMEOS) - return false; -#endif // defined(OS_CHROMEOS) -} - // Returns true if the cursor should be hidden on touch events. +// TODO(tdanderson|rsadam): Move this function into CursorClient. bool ShouldHideCursorOnTouch(const ui::TouchEvent& event) { #if defined(OS_WIN) return true; @@ -240,10 +193,11 @@ void CompoundEventFilter::SetMouseEventsEnableStateOnEvent(aura::Window* target, // CompoundEventFilter, ui::EventHandler implementation: void CompoundEventFilter::OnKeyEvent(ui::KeyEvent* event) { - if (ShouldHideCursorOnKeyEvent(*event)) { - SetCursorVisibilityOnEvent( - static_cast(event->target()), event, false); - } + aura::Window* target = static_cast(event->target()); + aura::client::CursorClient* client = + aura::client::GetCursorClient(target->GetRootWindow()); + if (client && client->ShouldHideCursorOnKeyEvent(*event)) + SetCursorVisibilityOnEvent(target, event, false); FilterKeyEvent(event); } diff --git a/ui/wm/core/cursor_manager.cc b/ui/wm/core/cursor_manager.cc index 1a95c9fc7aaebb..736ef6fa4a9de2 100644 --- a/ui/wm/core/cursor_manager.cc +++ b/ui/wm/core/cursor_manager.cc @@ -206,6 +206,11 @@ void CursorManager::RemoveObserver( observers_.RemoveObserver(observer); } +bool CursorManager::ShouldHideCursorOnKeyEvent( + const ui::KeyEvent& event) const { + return false; +} + void CursorManager::CommitCursor(gfx::NativeCursor cursor) { current_state_->set_cursor(cursor); } diff --git a/ui/wm/core/cursor_manager.h b/ui/wm/core/cursor_manager.h index 3640c7eba3a483..a6b226ea1ddbdb 100644 --- a/ui/wm/core/cursor_manager.h +++ b/ui/wm/core/cursor_manager.h @@ -20,6 +20,10 @@ namespace gfx { class Display; } +namespace ui { +class KeyEvent; +} + namespace wm { namespace internal { @@ -35,7 +39,7 @@ class NativeCursorManager; class WM_CORE_EXPORT CursorManager : public aura::client::CursorClient, public NativeCursorManagerDelegate { public: - CursorManager(scoped_ptr delegate); + explicit CursorManager(scoped_ptr delegate); virtual ~CursorManager(); // Overridden from aura::client::CursorClient: @@ -59,6 +63,8 @@ class WM_CORE_EXPORT CursorManager : public aura::client::CursorClient, aura::client::CursorClientObserver* observer) OVERRIDE; virtual void RemoveObserver( aura::client::CursorClientObserver* observer) OVERRIDE; + virtual bool ShouldHideCursorOnKeyEvent( + const ui::KeyEvent& event) const OVERRIDE; private: // Overridden from NativeCursorManagerDelegate: