Keys get "stuck" in Crostini |
||||||
Issue description
Chrome version: 68.0.3437.0 (Official Build) dev (64-bit)
OS: Chrome
Repro steps:
1. Boot Pixelbook
2. Open crosh
3. Run `vmc start termina`
4. Run `run_container.sh --container_name=penguin --user npmeyer --shell`
5. Run `xev`
6. Press and hold "Alt+Up"
7. While still holding "Alt+Up", press and hold "Tab"
8. Release "Alt+Up"
9. Release "Tab"
10. Alt+Tab back to xev window.
Expected: xev window is back in foreground, no repeated key events are generated.
Actual: xev shows many repeated KeyPress and KeyRelease events for "Prior" (Alt+Up) as shown below. If the issue does not immediately reproduce, repeat steps 6-10 a few times until it reproduces.
KeyPress event, serial 35, synthetic NO, window 0x400001,
root 0x2d8, subw 0x0, time 761573, (-617,-382), root:(705,470),
state 0x0, keycode 112 (keysym 0xff55, Prior), same_screen YES,
XLookupString gives 0 bytes:
XmbLookupString gives 0 bytes:
XFilterEvent returns: False
KeyRelease event, serial 35, synthetic NO, window 0x400001,
root 0x2d8, subw 0x0, time 761599, (-617,-382), root:(705,470),
state 0x0, keycode 112 (keysym 0xff55, Prior), same_screen YES,
XLookupString gives 0 bytes:
XFilterEvent returns: False
KeyPress event, serial 35, synthetic NO, window 0x400001,
root 0x2d8, subw 0x0, time 761599, (-617,-382), root:(705,470),
state 0x0, keycode 112 (keysym 0xff55, Prior), same_screen YES,
XLookupString gives 0 bytes:
XmbLookupString gives 0 bytes:
XFilterEvent returns: False
KeyRelease event, serial 35, synthetic NO, window 0x400001,
root 0x2d8, subw 0x0, time 761625, (-617,-382), root:(705,470),
state 0x0, keycode 112 (keysym 0xff55, Prior), same_screen YES,
XLookupString gives 0 bytes:
XFilterEvent returns: False
,
May 29 2018
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1addcee1a18a2793020618e3fc2ca532135820a5 commit 1addcee1a18a2793020618e3fc2ca532135820a5 Author: David Reveman <reveman@chromium.org> Date: Thu May 31 00:54:53 2018 exo: Ignore synthetic key repeat events. Exo clients should not see these events and instead handle key repeat on the client side. Bug: 847500 Test: exo_unittests Change-Id: I4d4c755dbe0a2fffe24f3ce85fd70a3608cfed7c Reviewed-on: https://chromium-review.googlesource.com/1079768 Reviewed-by: Daniele Castagna <dcastagna@chromium.org> Commit-Queue: David Reveman <reveman@chromium.org> Cr-Commit-Position: refs/heads/master@{#563095} [modify] https://crrev.com/1addcee1a18a2793020618e3fc2ca532135820a5/components/exo/keyboard.cc [modify] https://crrev.com/1addcee1a18a2793020618e3fc2ca532135820a5/components/exo/seat.cc
,
Jun 4 2018
,
Jun 21 2018
,
Jun 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94a6f1a75f7fd8247443ba66fb6958e786626181 commit 94a6f1a75f7fd8247443ba66fb6958e786626181 Author: David Reveman <reveman@chromium.org> Date: Mon Jun 25 17:04:55 2018 exo: Fix key event rewrite support. Use a map to track pressed keys. The key for the map is the physical code that was pressed and the value is a potentially rewritten code. This allows us to generate appropriate release events when physical keys are released, while still allowing the event rewriting logic to work as expected. Note: Exo clients will always see a release for the same code as the code that was generated when the physical key was pressed. This is necessary in order to not break client side keyboard state tracking and might not match the events that are generated internally. That is because event rewrite logic is usually not rewriting both the press and the release events, often just the press event. Bug: 847500 Test: exo_unittests --gtest_filter=KeyboardTest.OnKeyboardKey Tbr: hidehiko@chromium.org Change-Id: I23240304bd377e3855971edd079ade1b537a74f9 Reviewed-on: https://chromium-review.googlesource.com/1112967 Reviewed-by: Daniele Castagna <dcastagna@chromium.org> Commit-Queue: David Reveman <reveman@chromium.org> Cr-Commit-Position: refs/heads/master@{#570074} [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/ash/system/message_center/arc/arc_notification_content_view_unittest.cc [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/keyboard.cc [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/keyboard.h [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/keyboard_delegate.h [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/keyboard_unittest.cc [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/seat.cc [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/seat.h [modify] https://crrev.com/94a6f1a75f7fd8247443ba66fb6958e786626181/components/exo/wayland/server.cc
,
Jun 25 2018
,
Jul 3
I'm going to revert the CL in comment 6 tomorrow because it breaks all keyboard operations on ARC++ apps with ChromeVox and navigation keys on virtual keyboard. Please see crbug.com/859071 and crbug.com/859797 .
,
Jul 10
yhanada@, are you still going to revert the CL in #6? (It's causing crbug.com/862140 )
,
Jul 10
I think we have two options for how to fix this: 1. Use a different wayland protocol that arc++ understands for synthetic events. 2. Use the standard protocol but force all synthetic events to always generate both a press and a release event so they don't break client side keyboard state tracking.
,
Jul 11
Hmm, option 2 sounds impossible because if we have a rewriting rule like (Search+1 -> F1) and keys are pressed like (press Search -> press 1 -> release Search -> release 1), it seems very difficult to know press event was rewritten when 1 is released. by the way, why do we need to keep consistency between key press events and key release events? As far as I know, there are some cases in X Window apps that no key release event is sent to an app which receives the key press event. For example, If focus is switched by Alt+Tab, previously focused window sees only press events and new focused window sees only release event. Other cases is connecting 2 keyboards and pressing some key at the same time. This will generate event sequence like (press X -> press X -> release X -> release X).
,
Jul 11
There's no way to track the keyboard state in a wayland application unless you have input focus. Which is good, as being able to do so is terrible from a security perspective. As a result, wayland protocol sends the currently pressed keys to the client as part of the "enter" event for input focus. We need to support that properly. That's what the physical code logic is about. 2 keyboards can be assigned to 2 different seats in order to allow separate tracking. If you map 2 keyboards to the same seat then the best we can do is ignore press events when a key is already pressed and ignore release events if a key is not pressed. The protocol is not designed to support multiple keyboards per seat well. Multiple seats is the way to support that in an ideal way. Search+1 rewrite is handled by exo in a reasonable way I think. We store the rewritten code for the physical key when pressed, and when the physical key is released, we generate the same rewritten code independent of if the release event is rewritten the same way or not.
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/421412e7cce9db9512c8e4b816323aa0eed0def5 commit 421412e7cce9db9512c8e4b816323aa0eed0def5 Author: Yuichiro Hanada <yhanada@chromium.org> Date: Fri Jul 13 09:07:50 2018 Send synthetic key events to ARC++ apps. crrev.com/c/1112967 adds a check that sending key events is native or not and synthetic key events were not sent to exo clients after the CL. The check was introduced to make sure key press event and key release event always have the same key code. It's needed for Crostini apps tracking pressed key sets by observing key press/release events. However, after the CL, all synthetic events (for example, the back button on the shelf or keyboard navigation when ChromeVox is enabled) were not sent to ARC++ apps anymore. This CL is a quick hack for fixing this problem on ARC++ apps. We have to find a cleaner way to fix all problems later. Bug: 859071 , 859797 , 847500 , 859797 , 862140 Test: Confirmed the back button on the shelf works on ARC++ apps. Change-Id: Ia1da441cca8bc43687327b69cccf16fb8692046b Reviewed-on: https://chromium-review.googlesource.com/1133102 Commit-Queue: Yuichiro Hanada <yhanada@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: David Reveman <reveman@chromium.org> Cr-Commit-Position: refs/heads/master@{#574866} [modify] https://crrev.com/421412e7cce9db9512c8e4b816323aa0eed0def5/components/exo/keyboard.cc [modify] https://crrev.com/421412e7cce9db9512c8e4b816323aa0eed0def5/components/exo/keyboard.h [modify] https://crrev.com/421412e7cce9db9512c8e4b816323aa0eed0def5/components/exo/notification_surface.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by npmeyer@google.com
, May 29 2018