Commit c2946be0 authored by Hidehiko Abe's avatar Hidehiko Abe Committed by Commit Bot

Lacros: Fix cursor key behavior.

It turned out current physical key tracking does not work
if IME is enabled. It is not only for ARC, but also Lacros (as we
start using IME soon), and crostini in the future when IME is
fully enabled.

Currently, we have workaround for ARC, so Lacros follows the
approach for short term.
We need a long term fix in the future, though.

      if there is no composing text.

Bug: 1133651
Test: Typed cursor key on Lacros window. It works as expected
Change-Id: Ib709ea852ddbec75d4790cf3aabdf353cade089d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2452273
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: default avatarJun Mukai <mukai@chromium.org>
Reviewed-by: default avatarYuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815475}
parent 63d89d5a
...@@ -141,15 +141,21 @@ bool ProcessAcceleratorIfReserved(Surface* surface, ui::KeyEvent* event) { ...@@ -141,15 +141,21 @@ bool ProcessAcceleratorIfReserved(Surface* surface, ui::KeyEvent* event) {
return IsReservedAccelerator(event) && ProcessAccelerator(surface, event); return IsReservedAccelerator(event) && ProcessAccelerator(surface, event);
} }
// Returns true if surface belongs to an ARC application. // Returns true if the surface needs to support IME.
// TODO(yhanada, https://crbug.com/847500): Remove this when we find a way // TODO(yhanada, https://crbug.com/847500): Remove this when we find a way
// to fix https://crbug.com/847500 without breaking ARC++ apps. // to fix https://crbug.com/847500 without breaking ARC apps/Lacros browser.
bool IsArcSurface(Surface* surface) { bool IsImeSupportedSurface(Surface* surface) {
aura::Window* window = surface->window(); aura::Window* window = surface->window();
for (; window; window = window->parent()) { for (; window; window = window->parent()) {
if (window->GetProperty(aura::client::kAppType) == const auto app_type =
static_cast<int>(ash::AppType::ARC_APP)) { static_cast<ash::AppType>(window->GetProperty(aura::client::kAppType));
return true; switch (app_type) {
case ash::AppType::ARC_APP:
case ash::AppType::LACROS:
return true;
default:
// Do nothing.
break;
} }
} }
return false; return false;
...@@ -284,12 +290,18 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) { ...@@ -284,12 +290,18 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
// Ensured by the observer registration order. // Ensured by the observer registration order.
delegate_->OnKeyboardModifiers(seat_->xkb_tracker()->GetModifiers()); delegate_->OnKeyboardModifiers(seat_->xkb_tracker()->GetModifiers());
// Currently, physical keycode is tracked in Seat, assuming that the
// Keyboard::OnKeyEvent is called between Seat::WillProcessEvent and
// Seat::DidProcessEvent. However, if IME is enabled, it is no longer true,
// because IME work in async approach, and on its dispatching, call stack
// is split so actually Keyboard::OnKeyEvent is called after
// Seat::DidProcessEvent.
// TODO(yhanada): This is a quick fix for https://crbug.com/859071. Remove // TODO(yhanada): This is a quick fix for https://crbug.com/859071. Remove
// ARC-specific code path once we can find a way to manage press/release // ARC-/Lacros-specific code path once we can find a way to manage
// events pair for synthetic events. // press/release events pair for synthetic events.
ui::DomCode physical_code = ui::DomCode physical_code =
seat_->physical_code_for_currently_processing_event(); seat_->physical_code_for_currently_processing_event();
if (physical_code == ui::DomCode::NONE && focus_belongs_to_arc_app_) { if (physical_code == ui::DomCode::NONE && focused_on_ime_supported_surface_) {
// This key event is a synthetic event. // This key event is a synthetic event.
// Consider DomCode field of the event as a physical code // Consider DomCode field of the event as a physical code
// for synthetic events when focus surface belongs to an ARC application. // for synthetic events when focus surface belongs to an ARC application.
...@@ -426,7 +438,7 @@ void Keyboard::SetFocus(Surface* surface) { ...@@ -426,7 +438,7 @@ void Keyboard::SetFocus(Surface* surface) {
delegate_->OnKeyboardEnter(surface, pressed_keys_); delegate_->OnKeyboardEnter(surface, pressed_keys_);
focus_ = surface; focus_ = surface;
focus_->AddSurfaceObserver(this); focus_->AddSurfaceObserver(this);
focus_belongs_to_arc_app_ = IsArcSurface(surface); focused_on_ime_supported_surface_ = IsImeSupportedSurface(surface);
} }
} }
......
...@@ -130,7 +130,7 @@ class Keyboard : public ui::EventHandler, ...@@ -130,7 +130,7 @@ class Keyboard : public ui::EventHandler,
// True when the ARC app window is focused. // True when the ARC app window is focused.
// TODO(yhanada, https://crbug.com/847500): Remove this when we find a way to // TODO(yhanada, https://crbug.com/847500): Remove this when we find a way to
// fix https://crbug.com/847500 without breaking ARC++ apps. // fix https://crbug.com/847500 without breaking ARC++ apps.
bool focus_belongs_to_arc_app_ = false; bool focused_on_ime_supported_surface_ = false;
base::ObserverList<KeyboardObserver>::Unchecked observer_list_; base::ObserverList<KeyboardObserver>::Unchecked observer_list_;
......
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