1261880 - Special layers of Neo keyboard layout fails to work in recent Nightly versions

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[tobiasdiez]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160322030417

Steps to reproduce:

1. Activate the Neo keyboard layout http://neo-layout.org/
2. Try to type a character in the third layer ("Mod 3") by pressing Caps lock + letter in an arbitrary text field (on a website, or the address bar). For example Caps lock + n normally produces the brace (. The same problem happens with the other special layers of Neo which normally can be accessed with the combo Shift + caps lock. Some functionality of the Mod4 layer works as expected, for example the arrow keys.

Notes: The problem occurs since one or two weeks with the latest nightly versions and is reproducible on different machines.




Actual results:

The pressed letter is inserted. So in the above example 'n'. Apparently the modifiers are completely ignored.


Expected results:

As in other applications, the special letter of the (third) layer should have been inserted. In the above example, '('.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to tobiasdiez from comment #0)
> Notes: The problem occurs since one or two weeks with the latest nightly
> versions and is reproducible on different machines.

Could you look for the regression cause with mozregressions?
https://github.com/mozilla/mozregression/releases

I guess that this is broken around 3/16 due to bug 1137561.

Comment 2

[Paul Pasca[Away. Please needinfo? Paul Oiegas [:pauloiegasSV]]]

Could you please share an opinion on this issue?

Comment 3

[Carsten Book [:Tomcat]]

(In reply to Paul Pasca[:PoollyMcklayn] from comment #2)
> Could you please share an opinion on this issue?

Hi,

no idea what i should do here, seems not a sheriff issue and don't work in QA anymore (since a long time :)

Comment 4

[tobiasdiez]

Sorry that it took me so long to run the mopregression tool. Here is the result:

2016-04-25T09:12:19: INFO : Narrowed nightly regression window from [2016-03-13, 2016-03-16] (3 days) to [2016-03-15, 2016-03-16] (1 days) (~0 steps left)
2016-04-25T10:04:43: INFO : Narrowed inbound regression window from [29022393, 1d96035b] (3 revisions) to [29022393, eb7c36e2] (2 revisions) (~1 steps left)
2016-04-25T10:04:43: DEBUG : Starting merge handling...
2016-04-25T10:04:43: DEBUG : Using url: https://hg.mozilla.org/integration/mozilla-inbound/json-pushes?changeset=eb7c36e2ef5d48262bc8566da9ea37623e7d0883&full=1
2016-04-25T10:04:44: DEBUG : Found commit message:
Bug 1137563 part.5 Set charCode of dead key's keypress event on Mac to the dead char r=m_kato

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you. I'll investigate this bug this week if possible. Unfortunately, 4/29 - 5/8 is holiday week of Japan. So, it might be after the week.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, I cannot use CapsLock is a modifier in any applications on MacOS X 10.11. And I realized that you reported this bug as a bug on Mac (according to the status) but your report was posted from Windows.

(In reply to tobiasdiez from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0)
> Gecko/20100101 Firefox/48.0
> Build ID: 20160322030417

Do you reproduce this bug on which OS (and version)?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oops, the status was set by myself accidentally. I'll retry to test this on Windows.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sigh, the keyboard layout isn't available on Win10 (-Ja?). I tried to install kbdneo driver both with kbdneo64.zip and kbdneo_ahk.exe but the keyboard layout isn't appear in the input language settings in the control panel...

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

tobiasdiez:

Do you have Spy++ of Visual Studio? If so, I'd like you to log the keyboard messages (you can attach text file from the above link, "Add an attachment").  If you don't have it, I'll create a test build to log the behavior. Could you test it?

Comment 10

[tobiasdiez]

It happens on Windows 10. The installation of Neo works with the installer http://wiki.neo-layout.org/export/HEAD/windows/Neo2.0_setup.exe. After this some fiddling in the keyboard/input settings dialog is required (removing some languages, adding Neo and another one), maybe even a restart helps.

I added the spy++ log for Firefox 46. For some reason no messages are logged with the recent Nightly build.

Comment 11

[tobiasdiez]

Attachment: Spy++ for Firefox 46 (works as expected)

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8750785 [details]
Spy++ for Firefox 46 (works as expected)

> <00235> 0006057E P WM_CHAR chCharCode:'40' (40) cRepeat:1 ScanCode:24 fExtended:0 fAltDown:0 fRepeat:0 fUp:0

Okay, WM_CHAR is received as expected. So, we must run in the path which ignores WM_CHAR in the case... This must be a simple mistake.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to tobiasdiez from comment #10)
> It happens on Windows 10. The installation of Neo works with the installer
> http://wiki.neo-layout.org/export/HEAD/windows/Neo2.0_setup.exe.

I tried this one.

> After this
> some fiddling in the keyboard/input settings dialog is required (removing
> some languages, adding Neo and another one), maybe even a restart helps.

Yeah, but "Neo" doesn't appear in Germany in input language settings on my environment even after restart.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, perhaps, I found the bug. I'll try to create a patch and make a test build. Then, I'd like tobiasdiez to test it.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6de01d4e0b92

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Could you check if this test build fixes this bug?
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-f1c5b711a76d74cbf96fe55e43fb99464ff403e4/try-win32/

(I recommend to use firefox-49.0a1.en-US.win32.zip rather than installer.)

Comment 17

[Johannes Linke]

I had this very same problem, now in the developer edition, and the try build fixes it for me.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: MozReview Request: Bug 1261880 NativeKey should decide printable KeyboardEvent.key value of keydown and keypress events with following WM_CHAR message of WM_KEYDOWN r?m_kato

Some special keyboard layout may use a key as a non-lockable modifier key even if the key isn't a non-lockable modifier key (e.g., CapsLock key of 'Neo' for German). In such case, KeyboardLayout class cannot initialize NativeKey::mCommittedCharsAndModifiers with actual input character properly because KeyboardLayout class doesn't support such eccentric keyboard layouts.

For preventing this issue, NativeKey should overwrite mCommittedCharsAndModifiers with following WM_CHAR message when it handles WM_KEYDOWN and should handle with following WM_CHAR message.  However, we should ignore following WM_CHAR message if the character is a control character which shouldn't be inputted into focused text editor.

Review commit: https://reviewboard.mozilla.org/r/52101/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52101/

Comment 19

[tobiasdiez]

I can also confirm that the bug is fixed in the build you provided. Thanks a lot!

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you for your test and this bug report!

Comment 21

[Makoto Kato [:m_kato]]

Comment on attachment 8751566 [details]
MozReview Request: Bug 1261880 NativeKey should decide printable KeyboardEvent.key value of keydown and keypress events with following WM_CHAR message of WM_KEYDOWN r?m_kato

https://reviewboard.mozilla.org/r/52101/#review49049

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3eaad74999f

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b3eaad74999f

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8751566 [details]
MozReview Request: Bug 1261880 NativeKey should decide printable KeyboardEvent.key value of keydown and keypress events with following WM_CHAR message of WM_KEYDOWN r?m_kato

Approval Request Comment
[Feature/regressing bug #]: regression of bug 1137561
[User impact if declined]: Some unusual keyboard layouts which use a non-modifier key as a modifier key don't work as expected. So, this regression may affects a few uses but the impact is serious.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Low because this patch respects WM_CHAR messages rather than current keyboard layout's information which is retrieved with current modifier state. (So, this patch fixes the problem that we cannot retrieve actual input character from keyboard layout information when non-modifier key is treated as a modifier key.)
[String/UUID change made/needed]: Nothing.

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8751566 [details]
MozReview Request: Bug 1261880 NativeKey should decide printable KeyboardEvent.key value of keydown and keypress events with following WM_CHAR message of WM_KEYDOWN r?m_kato

Fix a regression, taking it

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7209d10232de