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

Issue 915277 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Events from keyboard layouts with dead keys interpreted incorrectly on Linux

Reported by rica...@basto.com, Dec 14

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce the problem:
1. Visit http://w3c.github.io/uievents/tools/key-event-viewer.html on Linux (I'm using Ubuntu 18.10)
2. Switch your input keyboard to an international layout with dead keys (I'm using English (US, intl., with dead keys))
3. Press and release any key (the letter a, the ENTER key)

What is the expected behavior?
The repeat flag for both the keydown and keypress events should be off

What went wrong?
The repeat flag is on for these events.
If you try that with the English (US) input layout the flags will correctly show as off.

Did this work before? Yes 70.0.3538.110-1

Chrome version: 71.0.3578.98  Channel: stable
OS Version: Ubuntu 18.10
Flash Version: 

The above described is just one example. The actual use of dead keys appears to be broken as well: the dead key will appear in the event as "Unidentified", the keydown event for the dead key is not triggered, and the keydown event for the letter being composed is not triggered either.
 
Labels: Needs-Bisect Needs-Triage-M71
Cc: santhoshkumar@chromium.org
Labels: Triaged-ET Needs-Feedback
Tried testing the issue on reported chrome version #71.0.3578.98 using Linux 14.04 by following below steps as per comment #0.

Steps to reproduce the problem:
-----------------------------------------------
1. opened chrome and navigated to http://w3c.github.io/uievents/tools/key-event-viewer.html 
2. Switched to ENG (US) keyboard and pressed key A and Key ENTER and observed that repeat flag is off.
3. Changed keyboard layout to Japanese and pressed key A and Key ENTER and observed that repeat flag is off

Attached screencast for reference.
@reporter: Could you please review attached screencast and let us know if anything is being missed here and let us know if this issue is specific to 18.10
Thanks.!
915277.ogv
3.2 MB View Download
The test was performed correctly. Since the problem was not reproduced I suspect it must be related to 18.10/cosmic.
Thank you!
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 17

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Components: -UI UI>Input>Text
Labels: -Pri-2 -Needs-Bisect RegressedIn-71 ReleaseBlock-Stable Target-71 Target-72 Target-73 M-71 FoundIn-71 FoundIn-73 FoundIn-72 hasbisect Pri-1
Owner: erikc...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on reported chrome version #71.0.3578.98  and on latest chrome# 73.0.3643.0 by following steps as per comment#0 using Ubuntu 17.10 and The following is the bisect information.

Bisect information:
==============
Good Build: 71.0.3567.0
Bad Build: 71.0.3568.0

Note:  On running per-revision bisect getting error, hence providing below changelog from chromium bisect
Change Log:  https://chromium.googlesource.com/chromium/src/+log/30e11c9e9b9f5ab322a5c5fdb107e62b44d6215b..2c72cd9089bc3278bf3d2852c971325a9c07f097
Suspecting: https://chromium.googlesource.com/chromium/src/+/2c72cd9089bc3278bf3d2852c971325a9c07f097
Reviewed-on: https://chromium-review.googlesource.com/1252685

@erikchen: Please confirm the issue and help in re-assigning if it is not related to your change
Adding Release-Block-Stable as this is recent regression. Please feel free to remove if not applicable.

Thanks.!

Cc: thomasanderson@chromium.org aelias@chromium.org
That's odd. +thomasanderson, aelias.
Owner: thomasanderson@chromium.org
Over to thomasanderson, who knows a lot more about X. We can always revert the timestamp change if we think that's easier.
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 18

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

commit faa579def265753007f87c3ea85a907f0ed469b0
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Tue Dec 18 19:44:53 2018

Revert "[Reland 1] Use accurate X11 event timestamp computation."""

This reverts commit 2c72cd9089bc3278bf3d2852c971325a9c07f097.

Reason for revert: bugs  915277  and  899881 

Original change's description:
> [Reland 1] Use accurate X11 event timestamp computation.""
> 
> The original CL didn't allow negative time skew from the time server. While this
> is likely correct, this caused unit tests to fail as they relied on negative time
> skew. This CL allows negative time skew, and also removes another source of
> non-determinism from the tests.
> 
> Original change's description:
> > Use accurate X11 event timestamp computation.
> >
> > X events have a timestamp which is only well defined relative to the X11 Server
> > time. The previous computation for timestamp for X11 events was making the
> > assumption that Server time and Chrome time were the same. This assumption is
> > not always true -- this is likely the root cause of "bad" timestamps observed in
> > https://bugs.chromium.org/p/chromium/issues/detail?id=650338#c1
> >
> > This CL changes event timestamp computation to make a roundtrip to the X11
> > Server to get an accurate base::TimeTicks. This logic was lifted out of the
> > responsiveness calculator, which was already doing this computation. The latter
> > will subsequently be changed to use the computation in this CL.
> >
> > Change-Id: I963019cd8bfb8ce14e06b3743a159c9c85f2cb82
> > Bug:  859155 
> > Reviewed-on: https://chromium-review.googlesource.com/1249383
> > Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Commit-Queue: Erik Chen <erikchen@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#594844}
> 
> Change-Id: I6595f6aa163a18c9724c50546ac8c84bc5c84a03
> Bug:  859155 
> Reviewed-on: https://chromium-review.googlesource.com/1252685
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595454}

TBR=avi@chromium.org,erikchen@chromium.org,thomasanderson@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  859155 , 899881 , 915277 
Change-Id: Ib85a5f5d7bf9469076a334d77ef92771199d6a2d
Reviewed-on: https://chromium-review.googlesource.com/c/1382868
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617593}
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/event_unittest.cc
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/platform/x11/x11_event_source.cc
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/platform/x11/x11_event_source.h
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/x/events_x.cc
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/x/events_x_unittest.cc
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/x/events_x_utils.cc
[modify] https://crrev.com/faa579def265753007f87c3ea85a907f0ed469b0/ui/events/x/events_x_utils.h

Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-71 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD
Merge requested on  bug 899881  
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 20

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ad9f6333b2a7d81bbbf926529e560269e05f439

commit 6ad9f6333b2a7d81bbbf926529e560269e05f439
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Thu Dec 20 19:15:13 2018

[Merge to M72] Revert "[Reland 1] Use accurate X11 event timestamp computation."""

> This reverts commit 2c72cd9089bc3278bf3d2852c971325a9c07f097.
>
> Reason for revert: bugs  915277  and  899881 
>
> Original change's description:
> > [Reland 1] Use accurate X11 event timestamp computation.""
> >
> > The original CL didn't allow negative time skew from the time server. While this
> > is likely correct, this caused unit tests to fail as they relied on negative time
> > skew. This CL allows negative time skew, and also removes another source of
> > non-determinism from the tests.
> >
> > Original change's description:
> > > Use accurate X11 event timestamp computation.
> > >
> > > X events have a timestamp which is only well defined relative to the X11 Server
> > > time. The previous computation for timestamp for X11 events was making the
> > > assumption that Server time and Chrome time were the same. This assumption is
> > > not always true -- this is likely the root cause of "bad" timestamps observed in
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=650338#c1
> > >
> > > This CL changes event timestamp computation to make a roundtrip to the X11
> > > Server to get an accurate base::TimeTicks. This logic was lifted out of the
> > > responsiveness calculator, which was already doing this computation. The latter
> > > will subsequently be changed to use the computation in this CL.
> > >
> > > Change-Id: I963019cd8bfb8ce14e06b3743a159c9c85f2cb82
> > > Bug:  859155 
> > > Reviewed-on: https://chromium-review.googlesource.com/1249383
> > > Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> > > Reviewed-by: Avi Drissman <avi@chromium.org>
> > > Commit-Queue: Erik Chen <erikchen@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#594844}
> >
> > Change-Id: I6595f6aa163a18c9724c50546ac8c84bc5c84a03
> > Bug:  859155 
> > Reviewed-on: https://chromium-review.googlesource.com/1252685
> > Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Commit-Queue: Erik Chen <erikchen@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#595454}
>
> TBR=avi@chromium.org,erikchen@chromium.org,thomasanderson@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug:  859155 , 899881 , 915277 
> Change-Id: Ib85a5f5d7bf9469076a334d77ef92771199d6a2d
> Reviewed-on: https://chromium-review.googlesource.com/c/1382868
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#617593}

TBR=avi@chromium.org,erikchen@chromium.org,thomasanderson@chromium.org
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Bug:  859155 , 899881 , 915277 
Change-Id: I325a7b1587a8dace272d2b2ac8a2dc0ea0082218
Reviewed-on: https://chromium-review.googlesource.com/c/1387545
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#487}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/event_unittest.cc
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/platform/x11/x11_event_source.cc
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/platform/x11/x11_event_source.h
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/x/events_x.cc
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/x/events_x_unittest.cc
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/x/events_x_utils.cc
[modify] https://crrev.com/6ad9f6333b2a7d81bbbf926529e560269e05f439/ui/events/x/events_x_utils.h

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/6ad9f6333b2a7d81bbbf926529e560269e05f439

Commit: 6ad9f6333b2a7d81bbbf926529e560269e05f439
Author: thomasanderson@chromium.org
Commiter: thomasanderson@chromium.org
Date: 2018-12-20 19:15:13 +0000 UTC

[Merge to M72] Revert "[Reland 1] Use accurate X11 event timestamp computation."""

> This reverts commit 2c72cd9089bc3278bf3d2852c971325a9c07f097.
>
> Reason for revert: bugs  915277  and  899881 
>
> Original change's description:
> > [Reland 1] Use accurate X11 event timestamp computation.""
> >
> > The original CL didn't allow negative time skew from the time server. While this
> > is likely correct, this caused unit tests to fail as they relied on negative time
> > skew. This CL allows negative time skew, and also removes another source of
> > non-determinism from the tests.
> >
> > Original change's description:
> > > Use accurate X11 event timestamp computation.
> > >
> > > X events have a timestamp which is only well defined relative to the X11 Server
> > > time. The previous computation for timestamp for X11 events was making the
> > > assumption that Server time and Chrome time were the same. This assumption is
> > > not always true -- this is likely the root cause of "bad" timestamps observed in
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=650338#c1
> > >
> > > This CL changes event timestamp computation to make a roundtrip to the X11
> > > Server to get an accurate base::TimeTicks. This logic was lifted out of the
> > > responsiveness calculator, which was already doing this computation. The latter
> > > will subsequently be changed to use the computation in this CL.
> > >
> > > Change-Id: I963019cd8bfb8ce14e06b3743a159c9c85f2cb82
> > > Bug:  859155 
> > > Reviewed-on: https://chromium-review.googlesource.com/1249383
> > > Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> > > Reviewed-by: Avi Drissman <avi@chromium.org>
> > > Commit-Queue: Erik Chen <erikchen@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#594844}
> >
> > Change-Id: I6595f6aa163a18c9724c50546ac8c84bc5c84a03
> > Bug:  859155 
> > Reviewed-on: https://chromium-review.googlesource.com/1252685
> > Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Commit-Queue: Erik Chen <erikchen@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#595454}
>
> TBR=avi@chromium.org,erikchen@chromium.org,thomasanderson@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug:  859155 , 899881 , 915277 
> Change-Id: Ib85a5f5d7bf9469076a334d77ef92771199d6a2d
> Reviewed-on: https://chromium-review.googlesource.com/c/1382868
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#617593}

TBR=avi@chromium.org,erikchen@chromium.org,thomasanderson@chromium.org
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Bug:  859155 , 899881 , 915277 
Change-Id: I325a7b1587a8dace272d2b2ac8a2dc0ea0082218
Reviewed-on: https://chromium-review.googlesource.com/c/1387545
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#487}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment