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

Issue 707055 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Figure out if key press synthesis is needed on Linux Ozone

Project Member Reported by jamescook@chromium.org, Mar 30 2017

Issue description

services/ui/ws/platform_display_default.cc contains this code in PlatformDisplayDefault::DispatchEvent(). It runs after each event is dispatched to the event sink.

  // We want to emulate the WM_CHAR generation behaviour of Windows.
  //
  // On Linux, we've previously inserted characters by having
  // InputMethodAuraLinux take all key down events and send a character event
  // to the TextInputClient. This causes a mismatch in code that has to be
  // shared between Windows and Linux, including blink code. Now that we're
  // trying to have one way of doing things, we need to standardize on and
  // emulate Windows character events.
  //
  // This is equivalent to what we're doing in the current Linux port, but
  // done once instead of done multiple times in different places.
  if (event->type() == ui::ET_KEY_PRESSED) {
    ui::KeyEvent* key_press_event = event->AsKeyEvent();
    ui::KeyEvent char_event(key_press_event->GetCharacter(),
                            key_press_event->key_code(),
                            key_press_event->flags());
    // We don't check that GetCharacter() is equal because changing a key event
    // with an accelerator to a character event can change the character, for
    // example, from 'M' to '^M'.
    DCHECK_EQ(key_press_event->key_code(), char_event.key_code());
    DCHECK_EQ(key_press_event->flags(), char_event.flags());
    SendEventToSink(&char_event);
  }

This results in two ui::ET_KEYPRESSED events for each letter typed. On Chrome OS it causes  issue 706574 , tab moving focus by 2 items. The code isn't necessary on Chrome OS -- the ordinary dispatch works fine. I'm removing it for Chrome OS.

Code was added in https://codereview.chromium.org/1234623004

It's not clear to me or erg@ if this will be needed on Linux Ozone. If not, the code could be removed entirely.

 

Comment 1 by sadrul@chromium.org, Mar 30 2017

Cc: moshayedi@chromium.org
We shouldn't need this on linux-ozone either, as long as we have ime working on linux.
Sadrul, should I just nuke that code then?

Cc: toniki...@igalia.com
FYI for tonikitoo, I think this code isn't needed anymore, so I'm going to delete it. I tried to manually test Linux Ozone, but it seems to be crashing on startup right now. If you see the keyboard stop working with web content on Linux Ozone, this CL might be why: https://codereview.chromium.org/2795503002

Hi james, thanks for looping me in.

We are working linux/ozone (x11/wayland) out off trunk for the past 2 weeks - see a quick status in https://www.youtube.com/watch?v=w0IQ6N781iQ - rebasing weekly.
As it stabilizes (we still need to get popups / menus properly working), we will proceed with upstreaming it.

For the record, feel free to nuke this code out, if it causes ChromeOS problems.
Cc: afakhry@chromium.org
Owner: weidongg@chromium.org
Status: Assigned (was: Untriaged)
Status: WontFix (was: Assigned)

Sign in to add a comment