Commit 375755b3 authored by yusukes@google.com's avatar yusukes@google.com

Process all global shortcuts for Ash after InputMethodEventFilter.

This CL is for crbug.com/123856 (Aura shell applies accelerators without giving apps the option of handling key events). Currently, shortcuts for IME/layout switching (Shift+Alt, Ctrl+space, and so on) are processed before InputMethodEventFilter, and other Ash global shortcuts are processed after the filter. This is for ensuring that the IME shortcuts always work even if an IME (or an IME extension - http://dev.chromium.org/developers/design-documents/extensions/proposed-changes/apis-under-development/input-method-editor) tries to consume the shortcuts. However, this strategy is not compatible with crbug.com/123856. In order to fix crbug.com/123856, we sometimes have to process an Ash global shortcut key (including IME ones) after the key is passed to a web page.

This CL modifies accelerator_table.cc and shell.cc so that they process IME shortcuts after InputMethodEventFilter, and also simplifies ash/accelerators/ and ui/base/accelerators/accelerator_manager.cc by removing code for processing pre-IME shortcuts.

BUG=123856
TEST=ran aura_shell_unittests and ui_unittests

Review URL: https://chromiumcodereview.appspot.com/10155015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133642 0039d316-1c4b-4281-b951-d872f2087c98
parent f3c06962
...@@ -251,7 +251,8 @@ void AcceleratorController::Init() { ...@@ -251,7 +251,8 @@ void AcceleratorController::Init() {
kAcceleratorData[i].shift, kAcceleratorData[i].shift,
kAcceleratorData[i].ctrl, kAcceleratorData[i].ctrl,
kAcceleratorData[i].alt); kAcceleratorData[i].alt);
accelerator.set_type(kAcceleratorData[i].type); accelerator.set_type(kAcceleratorData[i].trigger_on_press ?
ui::ET_KEY_PRESSED : ui::ET_KEY_RELEASED);
Register(accelerator, this); Register(accelerator, this);
CHECK(accelerators_.insert( CHECK(accelerators_.insert(
std::make_pair(accelerator, kAcceleratorData[i].action)).second); std::make_pair(accelerator, kAcceleratorData[i].action)).second);
......
...@@ -81,9 +81,6 @@ bool AcceleratorDispatcher::Dispatch(const base::NativeEvent& event) { ...@@ -81,9 +81,6 @@ bool AcceleratorDispatcher::Dispatch(const base::NativeEvent& event) {
accelerator.set_type(ui::ET_KEY_RELEASED); accelerator.set_type(ui::ET_KEY_RELEASED);
if (accelerator_controller->Process(accelerator)) if (accelerator_controller->Process(accelerator))
return true; return true;
accelerator.set_type(aura::TranslatedKeyEvent(event, false).type());
if (accelerator_controller->Process(accelerator))
return true;
} }
} }
......
...@@ -35,10 +35,8 @@ AcceleratorFilter::~AcceleratorFilter() { ...@@ -35,10 +35,8 @@ AcceleratorFilter::~AcceleratorFilter() {
bool AcceleratorFilter::PreHandleKeyEvent(aura::Window* target, bool AcceleratorFilter::PreHandleKeyEvent(aura::Window* target,
aura::KeyEvent* event) { aura::KeyEvent* event) {
const ui::EventType type = event->type(); const ui::EventType type = event->type();
if (type != ui::ET_KEY_PRESSED && type != ui::ET_TRANSLATED_KEY_PRESS && if (type != ui::ET_KEY_PRESSED && type != ui::ET_KEY_RELEASED)
type != ui::ET_KEY_RELEASED && type != ui::ET_TRANSLATED_KEY_RELEASE) {
return false; return false;
}
if (event->is_char()) if (event->is_char())
return false; return false;
......
This diff is collapsed.
...@@ -62,7 +62,7 @@ enum AcceleratorAction { ...@@ -62,7 +62,7 @@ enum AcceleratorAction {
}; };
struct AcceleratorData { struct AcceleratorData {
ui::EventType type; bool trigger_on_press;
ui::KeyboardCode keycode; ui::KeyboardCode keycode;
bool shift; bool shift;
bool ctrl; bool ctrl;
......
...@@ -152,7 +152,6 @@ TEST_F(NestedDispatcherTest, AcceleratorsHandled) { ...@@ -152,7 +152,6 @@ TEST_F(NestedDispatcherTest, AcceleratorsHandled) {
ui::Accelerator accelerator(ui::VKEY_A, false, false, false); ui::Accelerator accelerator(ui::VKEY_A, false, false, false);
accelerator.set_type(ui::ET_KEY_RELEASED); accelerator.set_type(ui::ET_KEY_RELEASED);
// TODO(yusukes): Add a test for a ui::ET_TRANSLATED_KEY_RELEASE accelerator.
TestTarget target; TestTarget target;
Shell::GetInstance()->accelerator_controller()->Register(accelerator, Shell::GetInstance()->accelerator_controller()->Register(accelerator,
&target); &target);
......
...@@ -651,16 +651,15 @@ void Shell::Init() { ...@@ -651,16 +651,15 @@ void Shell::Init() {
AddRootWindowEventFilter(partial_screenshot_filter_.get()); AddRootWindowEventFilter(partial_screenshot_filter_.get());
AddShellObserver(partial_screenshot_filter_.get()); AddShellObserver(partial_screenshot_filter_.get());
// Then AcceleratorFilter and InputMethodEventFilter must be added (in this // InputMethodEventFilter must be the third one. It has to be added before
// order) since they have the second highest priority. // AcceleratorFilter.
DCHECK_EQ(2U, GetRootWindowEventFilterCount()); DCHECK_EQ(2U, GetRootWindowEventFilterCount());
input_method_filter_.reset(new internal::InputMethodEventFilter);
AddRootWindowEventFilter(input_method_filter_.get());
#if !defined(OS_MACOSX) #if !defined(OS_MACOSX)
accelerator_filter_.reset(new internal::AcceleratorFilter); accelerator_filter_.reset(new internal::AcceleratorFilter);
AddRootWindowEventFilter(accelerator_filter_.get()); AddRootWindowEventFilter(accelerator_filter_.get());
DCHECK_EQ(3U, GetRootWindowEventFilterCount());
#endif #endif
input_method_filter_.reset(new internal::InputMethodEventFilter);
AddRootWindowEventFilter(input_method_filter_.get());
system_gesture_filter_.reset(new internal::SystemGestureEventFilter); system_gesture_filter_.reset(new internal::SystemGestureEventFilter);
AddRootWindowEventFilter(system_gesture_filter_.get()); AddRootWindowEventFilter(system_gesture_filter_.get());
......
...@@ -119,18 +119,13 @@ bool AcceleratorManager::HasPriorityHandler( ...@@ -119,18 +119,13 @@ bool AcceleratorManager::HasPriorityHandler(
} }
bool AcceleratorManager::ShouldHandle(const Accelerator& accelerator) const { bool AcceleratorManager::ShouldHandle(const Accelerator& accelerator) const {
if (accelerator.type() != ET_KEY_RELEASED && if (accelerator.type() != ET_KEY_RELEASED)
accelerator.type() != ET_TRANSLATED_KEY_RELEASE) {
return true; return true;
}
// This check is necessary e.g. not to process the Shift+Alt+ET_KEY_RELEASED // This check is necessary e.g. not to process the Shift+Alt+ET_KEY_RELEASED
// Accelerator for Chrome OS (see ash/accelerators/accelerator_controller.cc) // Accelerator for Chrome OS (see ash/accelerators/accelerator_controller.cc)
// when Shift+Alt+Tab is pressed and then Tab is released. // when Shift+Alt+Tab is pressed and then Tab is released.
if (last_event_type_ == ET_KEY_PRESSED || return last_event_type_ == ET_KEY_PRESSED;
last_event_type_ == ET_TRANSLATED_KEY_PRESS) {
return true;
}
return false;
} }
} // namespace ui } // namespace ui
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