New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 707429 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Keyboard layout doesn't propagate to wayland clients

Project Member Reported by chirantan@chromium.org, Mar 31 2017

Issue description

Applications that talk to the wayland server appear to interpret keypresses using a QWERTY layout even if the user is using a different layout for the rest of chrome os.  We should make sure these are consistent.

It should be straightforward enough since the wayland server sends a keymap to the client.  We just need to make sure we send the right keymap.
 
Owner: jclinton@chromium.org
Status: Started (was: Untriaged)
It look be a bit of time to figure out how we brought Wayland into our world as all of the docs on Ozone on chromium.org are 5-6 years out of date. It turns out that Chrome *is* the Wayland server: http://cs/chromeos_public/src/platform2/libchromeos-ui/chromeos/ui/chromium_command_builder.cc?l=239&rcl=15a608ee90bcae472908d63218cb07b9bbf8d10a . That makes a lot of sense when we consider that Chrome Ash is the window manager: this is exactly how Linux desktop environments are implemented (their WM is the session's Wayland server). So, to fix this, it seems that I just need to pass the correct keyboard layout inside of Chrome's Wayland server implementation. Investigating how Chrome keyboard layout is plumbed through, next.
IME is going to get interesting.  keyboard layout works if you're lucky to have a language which fits on the keyboard.  but CJK needs a lot more.
  https://chrome.google.com/webstore/detail/mclkkofklkfljcocdinagocijmpgbhab

but i guess one thing at a time ?
Sharing the keymap used by chrome would be a good start. There's this TODO for this: https://cs.chromium.org/chromium/src/components/exo/wayland/server.cc?l=2745
(Had to work on some other things so just getting back to this.)

This is one massively complex bug with some odd history between Chrome OS, Chrome, and ARC.

There are multiple input API's in Android. One that works in ARC++ today; one that doesn't. This was intentionally implemented this way due to an oversight: https://docs.google.com/document/d/1OoPTUxsHO_OtgEWVKgyAWn-BhKdffCLkNaJTVL-NUC4/edit#heading=h.h218uyrtfgno . I can repro using this feedback report: https://feedback.corp.google.com/product/208/neutron?lView=rd&lReport=53709216836

When I follow the user's feedback example, ConnectBot--which apparently uses KeyCharacterMap--exhibits the bad behavior. When I try the same test with JuiceSSH, the layout is correct.

There's a duplicate bug for this problem internal here: https://buganizer.corp.google.com/u/0/issues/26581953 . However, this bug tracks solving both the Wayland server side and also the ARC side.

Here's the plan (as far as I can tell):

* Add an Observer interface for OnLayoutChanged to: https://cs.chromium.org/chromium/src/ui/base/ime/chromeos/ime_keyboard.h?l=41&rcl=97bec63d0193db10f8e31fdad38ea8886d73e00f
* Fire the callback from here (similar to how we handle caps lock): https://cs.chromium.org/chromium/src/ui/base/ime/chromeos/ime_keyboard_ozone.cc?l=25&rcl=97bec63d0193db10f8e31fdad38ea8886d73e00f
* Write a function to bind to the Observer in WaylandKeyboardDelegate here: https://cs.chromium.org/chromium/src/components/exo/wayland/server.cc?l=2740&rcl=75fe739ccdd005ba0faa8eda191c03a94cc269cb . Will simply call wl_keyboard_send_keymap with new keymap. (I don't see anything else that needs to be done.)
* Bind the function with AddObserver in the constructor here: https://cs.chromium.org/chromium/src/components/exo/wayland/server.cc?l=2762&rcl=75fe739ccdd005ba0faa8eda191c03a94cc269cb

After that, I'll hand the internal bug back over to the ARC++ folks to make the Android-side changes.

I don't see any way to validate that the change works (other than confirm that we don't crash) until the Android changes are in.

Oops, internal bug was a follow-on to the one linked above. It's actually https://b.corp.google.com/issues/27877642 .

Comment 7 by za...@chromium.org, Jun 7 2017

An easy way to validate this change is to run a simple wayland app and see if the keyboard input is correct while changing the layout. That's how Chirantan and I discovered the problem.
Reproduced with a Crouton environment per comment #7. Proceeding with fix and validation.
Fix out for review at https://codereview.chromium.org/2925353002
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2870fad48915fa547f6653e7cdd7afd04cada8fa

commit 2870fad48915fa547f6653e7cdd7afd04cada8fa
Author: jclinton <jclinton@chromium.org>
Date: Thu Jun 15 21:01:11 2017

Implement sync of keyboard layout between Ozone and Wayland clients

BUG= chromium:707429 ,b:27877642
TEST=install Crouton environment, run gtk3 apps with GDK Wayland
backend, confirmed layout is correct at first launch and after toggling
layout, confirmed no new crashes introduced (tried US, US Dvorak and
Latin American layouts). toggling layout after closing Wayland app
doesn't crash Chrome (observer released).

Review-Url: https://codereview.chromium.org/2925353002
Cr-Commit-Position: refs/heads/master@{#479825}

[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ash/system/tray_caps_lock.h
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/components/exo/wayland/BUILD.gn
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/components/exo/wayland/server.cc
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ui/base/BUILD.gn
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ui/base/ime/chromeos/fake_ime_keyboard.cc
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ui/base/ime/chromeos/ime_keyboard.cc
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ui/base/ime/chromeos/ime_keyboard.h
[modify] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ui/base/ime/chromeos/ime_keyboard_mus.cc
[add] https://crrev.com/2870fad48915fa547f6653e7cdd7afd04cada8fa/ui/base/ime/chromeos/ime_keyboard_unittest.cc

Status: Fixed (was: Started)

Comment 12 by sjg@google.com, Jul 5 2017

Labels: Team-BLD
Components: OS>Systems>Containers

Sign in to add a comment