1293505 - Remapped function keys by Keyboard Layout Manager do not work in FF v. 48.0

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[softcom]

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

Steps to reproduce:

I am using the excellent program Keyboard Layout Manager (KLM), Windows 7, and Firefox 48.0.

Sometimes, and I do not know how this has happened, the KLM-remapped F keys I have dedicated to special characters (like å ä ö) stopped working in Firefox. It has happened so seldom that I have not been able to isolate any previous reason for it.

 In these situations, Alt+132 (ä), for instance, works. And the F keys work in other programs, including IE. Non-mapped F keys wok. Rebooting the computer have fixed the problem in the past.

With v 48.0, rebooting does not help. Neither does opening FF in Safe Mode. Alt+ 132 (ä), for instance, works. I tried the F keys with "old 'portable' build of v 47, and they work in that version! 

Here is what one of the developers of KLM says:

---------------------
I guess that Firefox works on a lower level of processing keys than other applications, which is not a good feature for that program. What I mean is that when you press the key, your keyboard sends so called "scan key" code, which is translated into the "virtual key" code. All "normal" applications work with virtual key codes, not with scan codes, since scan codes can differ from keyboard to keyboard. The fact that Firefox ignores your layout makes me conclude that it works on the scan key code level.
---------------------

This is obviously an old problem (intermittent and seldom happening before 48.0).


Actual results:

Remapped F keys do not work.


Expected results:

Remapped F keys should have worked.

Comment 1

[softcom]

Not only do I (and all translators in hte world) need easy access to keys for non-common letters, but alsoo non-common characters such as n dash (which is ued instead of m dash in most of o the world). This problems should be fixed once and for all. Thanks!

Comment 2

[Tooru Fujisawa [:arai]]

Can you provide the URL of the software?
Also, what's the actual configuration do you apply, and what's the actual key stroke do you type?

Comment 3

[softcom]

The Keyboard Layout Manager (KLM) URL is http://www.klm32.com/.

I will have to ask the developer of KLM about the configuration of the F keys (and I will also ask about the configuration of other remapped keys, which actually work in FF v 48.0).

I get back to you as soon as I get a reply. Thanks for your efforts.

Hans L

Comment 4

[Tooru Fujisawa [:arai]]

thanks :)

widget/windows/KeyboardLayout.cpp would be related
https://dxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.cpp

Comment 5

[softcom]

Here is the response from one of the two developers of KLM:

-------------------

Thank you for your email. KLM creates a keyboard layout file just as Microsoft does. The keyboard layout file is a dll file (dynamic library link file) which holds mapping tables needed by the operating system.

There are several tables in the dll file and among those two are important: scan code to virtual key code, and virtual key code to character. The former is used to translate low level keyboard scan codes (which can vary among keyboard manufacturers) into the virtual key code wich is the same on the OS level. The latter is used to map virtual key codes into the unicode characters, and is used for all those keys and key combinations which produce characters.

So, whenever you press some character-producing key ('a' key for example), your keyboard will send the scan code, it will be translated into the virtual key code (the first table), and if that virtual key code exists in the character map tble, it would be translated into the unicode 'a' character (using thesecond table). All this stands for key combinations (for example, you press Ctrl+Alt+A). That second table for the Ctrl+Alt+A looks like this (very simpified view):

Ctrl | Alt |  Shift | Virtual Key Code | Unicode character

x    |  x  |        | VK_A             | 'a'

If v47 worked with KLM-created layout, and v48 does not, it is really interesting to see what they have changed.

If you need more information, please contact me.

-----------------
Hans L

Comment 6

[Tooru Fujisawa [:arai]]

Can you provide the following information to reproduce the issue?
  1. where does the issue happen?
     is it webpage, or Firefox UI?
  2. what's the actual key stroke do you type?
  3. what is "KLM-remapped F keys" and what does "Alt+132" mean?

Comment 7

[softcom]

Tooru, before v48, the problem occurred intermittently and seldom. As I mentioned, I was never able to ascertain what the reason was.

In v48, I cannot get my F-key  letters (åäö) to work at all in websites when I am using FF (they work in IE). Also, for instance, when I am using Edit > Find, and I type characters in the Find field, the F keys do not work.

My actual keystrokes for some characters are, for instance, AltGr+e for é, and AltGr+3 for ≠. As for the F keys, they have been remapped from their usual function to "my functions", e.g., F8/F9/F10 = å/ä/ö (shift+F key gives the upper case version of these letters). Let me reiterate that my AltGr+character (such as é) works in FF. Only the remapped F keys do not work.

So the KLM-remapped F keys are remapped as any other key on the keyboard, and this is explained in the message from the KLM developer above. 

As for Alt+132, and sorry if I was not clear, I press Alt and then 132 on the Num keyboard, and I get ä. Alt+134 = å, Alt+148 = ö (all these letters in this message were created this way, since my F keys do not work :-)

I hope that these explanations are satisfactory. If not, do not hesitate to ask for clarifications.

Hans L

Comment 8

[Tooru Fujisawa [:arai]]

Okay, thank you for the information, I confirmed the issue :)

Comment 9

[Tooru Fujisawa [:arai]]

Here's regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=290223936152&tochange=eb7c36e2ef5d

There are 4 bugs, but bug 1137561 touches KeyboardLayout on windows, so it would be related.


To be clear, here's reproduction steps on Windows 7 32bit on virtualbox.

1. Install
  1-1. Download Keyboard Layout Manager 2000 demo (32-bit version) from:
       http://www.klm32.com/Download.html
       (klm2000Demo32.zip)
  1-2. Extract klm2000Demo32.zip
  1-3. Run klm2000Demo32.exe and install it

2. Setup Keyboard Layout Manager
  2-1. Run Keyboard Layout Manager with "Run as administrator" from Start Menu
  2-2. In "Keyboard layout manager 32 bit" window click "New" button in "Keyboards" tab
  2-3. Change to following configuration if needed, and click "Create"
       Layout Name: New Layout
       Language:    English (United States)
       Template:    US
  2-4. Choose "New Layout" from list in "Keyboards" tab
  2-5. Click "Edit" button
  2-6. Right-click on "F9" key, and choose "Character"
  2-7. Click on "F9" key
  2-8. Click "CharMap" button
  2-9. In "Character map" window, click "ä" button
 2-10. Close "Character map" window
 2-11. Click "OK" button
 2-12. Click "Yes" to "Confirm" dialog
 2-13. Click "Close" button

3. Setup Keyboard
  3-1. In Control Panel, click "Change keyboards or other input methods" under "Clock, Language, and Region"
  3-2. In "Region and Language" window, click "Change keyboards..." button
  3-3. Choose "English (United States) - New Layout" in Default input language
  3-4. Click "OK" button
  3-5. Restart Windows

4. Test on Explorer
  4-1. Open Explorer
  4-2. Focus on Search field
  4-3. Hit "F9" key

5. Test on Firefox
  5-1. Run Firefox 48
  5-2. Focus on Search field
  5-3. Hit "F9" key

Expected Result:
  "ä" is inserted on steps 4-3 and 5-3.

Actual Result:
  "ä" is inserted on step 4-3, but nothing happens on step 5-3.

Same behavior happens on Nightly (2016-08-11).

Comment 10

[softcom]

Notice you tested on 32-bit KLM (and got the same result I got). However (although it might not matter, but I want to mention it), I use 64-bit KLM (and 64-bit Windows 7 Ultimate).

Regards/Hans L

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Tooru, is it a big deal? Thanks

Probably not critical enough to be a ride along in 48.0.1,right?

Comment 12

[Tooru Fujisawa [:arai]]

I'm afraid I have no idea how much this case would happen in wild or how critical this is for affected users.
I'd forward the question to the developer of Keyboard Layout Manager.

FWIW, according to the following page, the feature to override function keys is available only in single edition (2000 edition).
http://www.klm32.com/Registration.html

Comment 13

[softcom]

As you all well know, worldwide geographical mobility is tremendous, and that is a fact not only because of refugees of late. I dare say that every single person with a computer living in a country with letters different from those of their mother tongue has a great need for getting 'their' letters on the keyboard in an efficient way. I worked for 20 years as a translator, and if I had not been able to use KLM, and thus, having the Swedish letters at one-press keys, on FF, I would not had used this otherwise excellent browser. And if I now, even after retirement, have to press two or more keys to get the Swedish letters, I will, unfortunately, have to say goodby to FF. This should not be take as a threat (pretty unimpressive as such :-), but as an indication that millions of FF users are corresponding or visiting websites in their mother tongue, and they need to use, or at least having the protential of using, 'their' letters easily. (In addition, I use the F keys for characters that I use frequently, such as a bullet of my choice and the n dash. And I would like to add more.) I believe that this issue is, while not critical, of utmost importance.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

[Tracking Requested - why for this release]:
OK, we can try to fix it for 49 (see comment #13)

Comment 15

[softcom]

 Sylvestre Ledru, 49 sounds great to me./Hans L

Comment 16

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

As far as Gecko can retrieve WM_CHAR message at handling WM_KEYDOWN from the queue (i.e., ::TranslateMessage() generates WM_CHAR message(s), normally), NativeKey can accept any keyboard utilities.

So, if it worked well with 47, bug 1137561 could break something.

Comment 17

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

> 2. Setup Keyboard Layout Manager
>   2-1. Run Keyboard Layout Manager with "Run as administrator" from Start Menu
>   2-2. In "Keyboard layout manager 32 bit" window click "New" button in "Keyboards" tab
>   2-3. Change to following configuration if needed, and click "Create"
>        Layout Name: New Layout
>        Language:    English (United States)
>        Template:    US
>   2-4. Choose "New Layout" from list in "Keyboards" tab
>   2-5. Click "Edit" button
>   2-6. Right-click on "F9" key, and choose "Character"
>   2-7. Click on "F9" key
>   2-8. Click "CharMap" button
>   2-9. In "Character map" window, click "ä" button
>  2-10. Close "Character map" window
>  2-11. Click "OK" button
>  2-12. Click "Yes" to "Confirm" dialog
>  2-13. Click "Close" button

Hmm, NativeKey does NOT assume that user to customize non-printable key as printable key. See here...:
https://dxr.mozilla.org/mozilla-central/rev/054d4856cea6150a6638e5daf7913713281af97d/widget/windows/KeyboardLayout.cpp#1816,1860

Comment 18

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

Ah, but if Function keys are pressed with Ctrl, Alt nor Win, it should hit here:
https://dxr.mozilla.org/mozilla-central/rev/054d4856cea6150a6638e5daf7913713281af97d/widget/windows/KeyboardLayout.cpp#1816,1847-1850

Then, following WM_*CHAR message should be respected.

Comment 19

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

One of the important thing is the message order.

There are 3 possible scenarios:

Case #1, normal case:

1. WM_KEYDOWN
2. WM_CHAR (be in the queue at WM_KEYDOWN is received)
3. WM_KEYUP (or WM_KEYDOWN)

Case #2, unexpected case:

1. WM_KEYDOWN
2. WM_CHAR (be not in the queue at WM_KEYDOWN is received)
3. WM_KEYUP (or WM_KEYDOWN)

Case #3, WM_CHAR comes later:

1. WM_KEYDOWN
2. WM_KEYUP (or WM_KEYDOWN)
3. WM_CHAR

Each scenario runs different code path. So, Which message order is used by KLM? I assumed Case #1, but I don't still see any problems in current code...

Comment 20

[Tooru Fujisawa [:arai]]

Checked message with Window Detective (I don't install VisualStudio on VM).
I see the following messages when hitting F9 key

  WM_KEYDOWN (0x100)
    virtuel key = 136
    state = 0x00430001
  WM_CHAR (0x102)
    character code = 229
  WM_KEYUP (0x101)
    virtuel key = 136
    state = 0xC0430001

so it seems to be #1 or #2, but I'm not sure how to distinguish them.
maybe there's some logging feature that can distinguish them?

Comment 21

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

> maybe there's some logging feature that can distinguish them?

Yeah. Unfortunately, KeyboardLayout.cpp doesn't log its detail. But it has log module. So, if you build a trysever build with adding a simple log which puts the result of GetFollowingCharMessage().

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: debug log for remapped keys

tested with the build with debug log added (sorry I still couldn't manage MOZ_LOG to work... :/

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

Attached file is the related log, for the cases pressing "a" and pressing "F9" (where "a" key works and "F9" key doesn't work).
Looks like, both are taking same #1 path.
I don't see any behavior difference, in NativeKey::NativeKey and NativeKey::GetFollowingCharMessage code path.

The only difference is "Unknown virtual keycode (0x00000088)" message, that is shown for "F9" key
https://dxr.mozilla.org/mozilla-central/rev/054d4856cea6150a6638e5daf7913713281af97d/widget/windows/KeyboardLayout.cpp#3215

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: testcase to log key events

With this testcase, when I hit F9 key (remapped to "å") in the input field,
the following log is shown in console:

> Array [ "keydown", 0, "F9", "Unidentified", 0 ]
> Array [ "keypress", 0, "F9", "Unidentified", 0 ]
> Array [ "keyup", 0, "F9", "Unidentified", 0 ]

Comment 24

[Tooru Fujisawa [:arai]]

about "Unknown virtual keycode (0x00000088)".

https://msdn.microsoft.com/en-us/library/windows/desktop/dd375731(v=vs.85).aspx

the document says 0x88 is "Unassigned", and F9 is 0x78.
will check other function keys.

Comment 25

[Tooru Fujisawa [:arai]]

Here's the result of remapped keys and virtual-key codes
(only F5-F11 keys can be remapped, in function keys)

       | Original    |          | Actual
       | Virtual-key | Remapped | Virtual-key
       | Code        | to       | Code
-------+-------------+----------+-------------
  F5   | 0x74        | ø        | 0x8C
  F6   | 0x75        | ù        | 0x8D
  F7   | 0x76        | ý        | 0x8E
  F8   | 0x77        | ò        | 0x8B
  F9   | 0x78        | å        | 0x88
  F10  | 0x79        | è        | 0x89
  F11  | 0x7A        | ì        | 0x8A
  Esc  | 0x1B        | ÿ        | 0x8F

So, Keyboard Layout Manager uses those Unassigned Virtual-key Codes (0x88-8F) for remapped special keys.
And all of them don't work on Firefox.

Comment 26

[softcom]

> (only F5-F11 keys can be remapped, in function keys)

I have F3 remapped.

Hans L

Comment 27

[Tooru Fujisawa [:arai]]

(In reply to softcom from comment #26)
> > (only F5-F11 keys can be remapped, in function keys)
> 
> I have F3 remapped.
> 
> Hans L

sorry, I'm using unregistered version.
the website says registered version can edit any function keys.
http://www.klm32.com/Registration.html

Comment 28

[softcom]

Understand./Hans L

Comment 29

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

(In reply to Tooru Fujisawa [:arai] from comment #25)
> Here's the result of remapped keys and virtual-key codes
> (only F5-F11 keys can be remapped, in function keys)
> 
>        | Original    |          | Actual
>        | Virtual-key | Remapped | Virtual-key
>        | Code        | to       | Code
> -------+-------------+----------+-------------
>   F5   | 0x74        | ø        | 0x8C
>   F6   | 0x75        | ù        | 0x8D
>   F7   | 0x76        | ý        | 0x8E
>   F8   | 0x77        | ò        | 0x8B
>   F9   | 0x78        | å        | 0x88
>   F10  | 0x79        | è        | 0x89
>   F11  | 0x7A        | ì        | 0x8A
>   Esc  | 0x1B        | ÿ        | 0x8F
> 
> So, Keyboard Layout Manager uses those Unassigned Virtual-key Codes
> (0x88-8F) for remapped special keys.
> And all of them don't work on Firefox.

Ideally, in such case,

KeyboardEvent.key -> the character
KeyboardEvent.keyCode -> 0
KeyboardEvent.code -> depending on its scancode value
KeyboardEvent.charCode -> 0 for keydown/keyup, the character's code point for keypress

So, anyway, it should work, though.

Comment 30

[David Bolter [:davidb] (NeedInfo me for attention)]

Masayuki, are you taking this bug? Worrisome if there are a lot of KLM users...

Comment 31

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

Hmm, I'm not sure if it's possible. I'm trying to fix bug 1297013 for investigating this bug but I'm going to be in business trip end of this week and I'm going to be on vacation during 9/5-9/7. So, I'm not sure if I can fix this bug in this cycle.

Although, I don't like to keep such bugs in release builds, but this bug hasn't been reported during Nightly-Beta cycle, so, I guess the number of users is not so many (I hope).

But please don't misunderstand this comment, this bug is still in top of the queue of my work. But I get a lot of review/needinfo requests everyday. Therefore, it's hard to make promise to fix in this cycle. (In other words, I'm just busy.)

# I hope I'll request review of bug 1297013 today.

Comment 32

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

Ah, but I don't need to land bug1297013 before this. Somebody who can reproduce this bug tests with tryserver build for logging its behavior.

Comment 33

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

Could you attach a log file of <https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-af6add12aae3941193a78bd9fd1a5e671790048f/>?

You can specify MOZ_LOG=nsTextStoreWidgets:0,NativeKeyWidgets:5,sync to log NativeKey behavior.

Comment 34

[Tooru Fujisawa [:arai]]

Attachment: logs with MOZ_LOG=nsTextStoreWidgets:0,NativeKeyWidgets:5,sync

Here's logs for each case (separated by "# ==== ...." lines)

  Parent process (search field next to location bar)
    * F9 key (remapped to å) hit once
    * F9 key (remapped to å) hit twice
    * a key (remapped to æ) hit once
    * a key (remapped to æ) hit twice
  Content process (https://www.google.co.jp/ search field on e10s)
    * F9 key (remapped to å) hit once
    * F9 key (remapped to å) hit twice
    * a key (remapped to æ) hit once
    * a key (remapped to æ) hit twice

(maybe some of them are redundant.  logged just in case)

Comment 35

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

Thank you very much. I understand what occurs.

In InitKeyEvent(), we don't reset mKeyNameIndex to KEY_NAME_INDEX_USE_STRING even if mCommittedCharsAndModifiers has unicode characters. Okay, this fix must be simple.

Comment 36

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=08976e94d31f

Comment 37

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

arai-san, could you test it with this build?
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-793f89934bd2b2949ebac56ce0db9f20a13c0931/

(not have logging patches)

Comment 38

[Tooru Fujisawa [:arai]]

Here's the result with key combinations from comment #7:

 key               | remapped to | result (parent) | result(content)
-------------------+-------------+-----------------+-----------------
 F8                |      å      |      works      |      works
 Shift+F8          |      Å      |      works      |      works
 F9                |      ä      |      works      |      works
 Shift+F9          |      Ä      |      works      |      works
 F10               |      ö      |      works      |      works
 Shift+F10         |      Ö      |      works      |      works


and other key combinations that KLM supports (might be out of scope of this bug tho):

 key               | remapped to | result (parent) | result(content)
-------------------+-------------+-----------------+-----------------
 Ctrl+F9           |      Ë      |   doesn't work  |  doesn't work
 Ctrl+Alt+F9       |      Ï      |   doesn't work  |  doesn't work
 Shift+Ctrl+Alt+F9 |      Ü      |   doesn't work  |  doesn't work

I'll check the log for those keys shortly

Comment 39

[Tooru Fujisawa [:arai]]

Attachment: logs with MOZ_LOG=nsTextStoreWidgets:0,NativeKeyWidgets:5,sync for other combinations

here's the log for Ctrl+F9, Ctrl+Alt+F9, and Shift+Ctrl+Alt+F9, with previous try build (logging enabled one)
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-af6add12aae3941193a78bd9fd1a5e671790048f/try-win32-debug/

Comment 40

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

Thanks, but did with Ctrl/Alt work on old builds? When Ctrl or Alt key is pressed, WM_CHARs are completely ignored.

Comment 41

[Tooru Fujisawa [:arai]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #40)
> Thanks, but did with Ctrl/Alt work on old builds? When Ctrl or Alt key is
> pressed, WM_CHARs are completely ignored.

no, those are not working on Nightly nor Release.
They works on Explorer's search field.

Comment 42

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

I guess that fixing Ctrl/Alt modified case requires risky patch which changes the design of NativeKey...

Comment 43

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

(In reply to Tooru Fujisawa [:arai] from comment #41)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #40)
> > Thanks, but did with Ctrl/Alt work on old builds? When Ctrl or Alt key is
> > pressed, WM_CHARs are completely ignored.
> 
> no, those are not working on Nightly nor Release.
> They works on Explorer's search field.

Release already has this bug. How about ESR45? (Or Thunderbird)

Comment 44

[Tooru Fujisawa [:arai]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #43)
> (In reply to Tooru Fujisawa [:arai] from comment #41)
> > (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #40)
> > > Thanks, but did with Ctrl/Alt work on old builds? When Ctrl or Alt key is
> > > pressed, WM_CHARs are completely ignored.
> > 
> > no, those are not working on Nightly nor Release.
> > They works on Explorer's search field.
> 
> Release already has this bug. How about ESR45? (Or Thunderbird)

I misunderstood the question :P
on Firefox ESR 45.3.0, Ctrl+F9, Ctrl+Alt+F9, and Shift+Ctrl+Alt+F9 work and remapped characters are inserted.

Comment 45

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

Thank you. But it's bad news to me. Um, I should look for another hack for fixing this bug safely.

Comment 46

[Gerry Chang [:gchang]]

Track for 49+ as this is a regression for key mapping in keyboard layout manager.

Comment 47

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

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

Comment 48

[Tooru Fujisawa [:arai]]

With the binary above, every combination including Ctrl+F9, Ctrl+Alt+F9, and Shift+Ctrl+Alt+F9 works :)

Comment 49

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

(In reply to Tooru Fujisawa [:arai] from comment #48)
> With the binary above, every combination including Ctrl+F9, Ctrl+Alt+F9, and
> Shift+Ctrl+Alt+F9 works :)

Thank you very much! Actually, right now, I'd try to request you to test it :-D

Comment 50

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

Attachment: Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

Some keyboard utilities for Windows can change non-printable keys to printable keys.  Therefore, if a keydown message is followed by one or more char message whose wParam isn't a control character, NativeKey should treat it as a printable key's event.

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

Comment 51

[Makoto Kato [:m_kato]]

Comment on attachment 8785329 [details]
Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

https://reviewboard.mozilla.org/r/74580/#review72562

Comment 52

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

Comment on attachment 8785329 [details]
Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

https://reviewboard.mozilla.org/r/74580/#review72624

Sorry, this causes new permanent orange. I'll investigate it ASAP.

Comment 53

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

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

Comment 54

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

Attachment: Bug 1293505 part.2 KeyboardLayout::SynthesizeNativeKeyEvent() should emulate WM_SYEKEYDOWN, WM_SYSCHAR, WM_SYSDEADCHAR and WM_SYSKEYUP correctly

The new test failure is caused by a bug of the test API, KeyboardLayout::SysnthesizeNativeKeyEvent().  It doesn't generate WM_SYSKEY* messages nor WM_SYS*CHAR messages when Alt key is pressed.  Therefore, the new path in the previous code works unexpectedly with automated tests.

This patch makes KeyboardLayout::SysnthesizeNativeKeyEvent() WM_SYS* message aware.  When Alt key is pressed (but Ctrl key is not pressed) and the Alt key + something doesn't cause text input, the API generates WM_SYS* messages as expected.

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

Comment 55

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

Attachment: Bug 1293505 part.3 Fix wrong key emulation in test_keycodes.xul

Some tests in test_keycodes.xul emulate native key event with printable character even when Ctrl or Alt key is pressed.

With en-US keyboard layout, Ctrl+[A-Z] causes a control character's WM_CHAR message. However, the other OEM keys and numeric keys don't cause WM_CHAR message when Ctrl is pressed.  So, we need to fix some wrong emulations in it now.

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

Comment 56

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

Comment on attachment 8785329 [details]
Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/74580/diff/1-2/

Comment 57

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

Fortunately, my patch is completely correct.

However, there are some bugs both in test API (SynthesizeNativeKey()) and test_keycodes.xul itself. So, each additional patches fix only them. I.e., they don't touch any code which users never run in normal usage.

Comment 58

[Liz Henry (:lizzard) (relman/hg->git project)]

Good to hear.  Can you request uplift to aurora and beta, so that we can be sure to land this for the beta 9 build? I should start the build tomorrow afternoon Pacific Time.

Comment 59

[Makoto Kato [:m_kato]]

Comment on attachment 8786703 [details]
Bug 1293505 part.2 KeyboardLayout::SynthesizeNativeKeyEvent() should emulate WM_SYEKEYDOWN, WM_SYSCHAR, WM_SYSDEADCHAR and WM_SYSKEYUP correctly

https://reviewboard.mozilla.org/r/75618/#review73898

Comment 60

[Makoto Kato [:m_kato]]

Comment on attachment 8786704 [details]
Bug 1293505 part.3 Fix wrong key emulation in test_keycodes.xul

https://reviewboard.mozilla.org/r/75620/#review73926

Comment 61

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa3c6d4622f9
part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message r=m_kato
https://hg.mozilla.org/integration/autoland/rev/31fa184dec89
part.2 KeyboardLayout::SynthesizeNativeKeyEvent() should emulate WM_SYEKEYDOWN, WM_SYSCHAR, WM_SYSDEADCHAR and WM_SYSKEYUP correctly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/0770506cb101
part.3 Fix wrong key emulation in test_keycodes.xul r=m_kato

Comment 62

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

Comment on attachment 8785329 [details]
Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

Approval Request Comment
[Feature/regressing bug #]: Bug 1137561 (and related landings)
[User impact if declined]: Some unusual keyboard layout users (especially if they map non-printable keys to printable keys), they cannot use some some keys as printable keys or they see unexpected characters at typing some keys.
[Describe test coverage new/current, TreeHerder]: Landed m-c right now.
[Risks and why]: Low. Bug 1137561 made NativeKey handles WM_CHAR messages at handling WM_KEYDOWN due to limitation of TextEventDispatcher. However, even if there are following WM_CHAR messages, NativeKey ignores the information in WM_CHAR messages and set text from keyboard layout's raw information. This patch makes NativeKey use WM_CHAR if there is following WM_CHAR message.
[String/UUID change made/needed]: Nothing.

Comment 63

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

Comment on attachment 8786703 [details]
Bug 1293505 part.2 KeyboardLayout::SynthesizeNativeKeyEvent() should emulate WM_SYEKEYDOWN, WM_SYSCHAR, WM_SYSDEADCHAR and WM_SYSKEYUP correctly

Approval Request Comment
[Risks and why]: Nothing, this patch just improves testing API. In the previous patch, difference of WM_CHAR and WM_SYSCHAR is important. When Alt key is pressed, WM_SYSCHAR should be sent instead of WM_CHAR but current SynthesizeNativeKeyEvent() always sends WM_CHAR.  Therefore, that causes new orange with the previous patch.

Comment 64

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

Comment on attachment 8786704 [details]
Bug 1293505 part.3 Fix wrong key emulation in test_keycodes.xul

Approval Request Comment
[Risks and why]: Nothing, this fixes bugs of test_keycodes.xul. At synthesizing native keyboard events, they should specify same characters which are sent by WM_CHAR or WM_SYSCHAR.  When Ctrl key is pressed, [A-Z] keys should cause a control character, other keys should not cause any input. This patch fixes the mistakes.

Comment 65

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aa3c6d4622f9
https://hg.mozilla.org/mozilla-central/rev/31fa184dec89
https://hg.mozilla.org/mozilla-central/rev/0770506cb101

Comment 66

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Masayuki, based on the comments in this bug, it seems a bunch of people use this kind of remapping. Do we have any automated tests in this area that could have caught this regression sooner? If not, would you be able to add an automated test for it?

Comment 67

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello there, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 68

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

I'd like to wait for verification on Nightly before uplifting this to Aurora.

Comment 69

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Ritu Kothari (:ritu) from comment #66)
> Hi Masayuki, based on the comments in this bug, it seems a bunch of people
> use this kind of remapping. Do we have any automated tests in this area that
> could have caught this regression sooner? If not, would you be able to add
> an automated test for it?

Duh, you did add tests in the bug. Sorry for not noticing, perhaps I've done too many uplift reviews today. ;)

Comment 70

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8785329 [details]
Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

Regression in remapping some keys, 50 is still in Aurora, let's take it.

Comment 71

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

(In reply to Ritu Kothari (:ritu) from comment #69)
> (In reply to Ritu Kothari (:ritu) from comment #66)
> > Hi Masayuki, based on the comments in this bug, it seems a bunch of people
> > use this kind of remapping. Do we have any automated tests in this area that
> > could have caught this regression sooner? If not, would you be able to add
> > an automated test for it?
> 
> Duh, you did add tests in the bug. Sorry for not noticing, perhaps I've done
> too many uplift reviews today. ;)

No, I didn't. Unfortunately, when we want to test this completely, we need to install such keyboard layout into the environment. We could emulate such messages with SynthesizeNativeKey(). However, it may run different path because mozilla::widget::KeyboardLayout loads different keyboard layout which don't think F[0-12] keys don't cause text input.

Comment 72

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8786703 [details]
Bug 1293505 part.2 KeyboardLayout::SynthesizeNativeKeyEvent() should emulate WM_SYEKEYDOWN, WM_SYSCHAR, WM_SYSDEADCHAR and WM_SYSKEYUP correctly

Aurora50+

Comment 73

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8786704 [details]
Bug 1293505 part.3 Fix wrong key emulation in test_keycodes.xul

Aurora50+

Comment 74

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8785329 [details]
Bug 1293505 part.1 NativeKey should treat a key message as printable key's when the key message is followed by a printable char message

Too late for beta 49, we are heading into Monday's release candidate build now.

Comment 75

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9731047ebbfa
https://hg.mozilla.org/releases/mozilla-aurora/rev/87c57eedbd7f
https://hg.mozilla.org/releases/mozilla-aurora/rev/5968286c776b

Comment 76

[softcom]

"Too late for beta 49, we are heading into Monday's release candidate build now."

Disappointing!

Hans L

Comment 77

[Liz Henry (:lizzard) (relman/hg->git project)]

softcom@oceanisl:  Sorry to disappoint. I know Masayuki worked hard to get this in. We have a lot of other fluctuation and risk in late beta, sometimes, it just has to wait another 6 weeks for the fix.  Maybe you can use Firefox 50 in the meantime.

Comment 78

[softcom]

Liz, I do understand your difficulties. When would 50 Beta be available with the fix in question?

Hans L

Comment 79

[Liz Henry (:lizzard) (relman/hg->git project)]

50 is available now, currently in the Developer Edition channel here, https://www.mozilla.org/en-US/firefox/channel/#developer   And Sept. 13th it should be available for download on the beta channel. You can get the current beta (still 49 ) from either the above url or from here: https://www.mozilla.org/en-US/firefox/beta/all/ if you want to pick a specific platform and language.

Comment 80

[softcom]

Okay Liz, and thanks. Since I have started to get problems with plugin-container the last 2-3 days, hope 50 will solve that.

Regards,

Hans L

Comment 81

[softcom]

Hello: I post this here for information, since I was shown here where to go to install v49 and v50. I post it just in case there is something fishy about these two versions.

NOTE: I am not blaming anyone here for this. You are doing a remarkable and extremely valuable job!!! And I will not see this as a discussion here, and expect no comments.

I posted this on Mozillazine (FF Build) today:

---------------------------

What a disaster.

FF 48 lost the ability to have a keyboard app (Keybard Layout Manager) remap function keys F1-F12. I filed a bug report, and this will be fixed for v50 (as it now stands).

However, yesterday, I downloaded v50 developer edition, where the remapped F keys work. But I also updated v48.0.2 to v49 beta, and rebooted the computer, That's when the s-t hit the fan.

Windows did not start. Tried a few disks for starting Windows via disk, but the external drives where I had the images of the OS did not show up. So I let the computer be off over night.

At the same time as my problems started, my wife, on her computer, lost her RoboForm toolbar. She had not noticed the message that this toolbar caused FF instability, and she removed the toolbar without really knowing it. She also lost other toolbar buttons. And, I could not get on Mozillazine from her computer.

What a disaster!!!

After virus scans galore, I finally reinstalled the latest version of the Roboform toolbar on my wife's computer, and it has so far not been kicked off.

And, lo and behold, this morning, my computer started up as if nothing had happened. Except, I got the same message as my wife did, namely that Roboform cause instability. So I removed the toolbar. Reinstalled the latest version of Roboform, and has so far not gotten any kick-off message.

I am mentioning this here to show what extraordinary events might follow what is probably a small but in FF (I have not filed a bug report; should I?)

Regards,

Hans L

----------------------

Hans L