Closed Bug 1036008 Opened 10 years ago Closed 6 years ago

KeyboardEvent.keyCode for OEM key (punctuation key in US layout) is 0 if the key doesn't produce an ASCII character itself nor with Shift

Categories

(Core :: DOM: Events, defect)

51 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
platform-rel --- -
firefox60 --- fixed

People

(Reporter: qnub.ru, Assigned: masayuki)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [webcompat][platform-rel-Google][platform-rel-GoogleSuite][platform-rel-GoogleDocs])

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.153 Safari/537.36

Steps to reproduce:

On page http://unixpapa.com/js/testkey.html switch to russian keyboard layout and press "ё", "х", "ъ", "ж", "э", "б" and "ю" keys.


Actual results:

I've get next output (without keycodes):

keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1105      charCode=1105     
keyup    keyCode=0         which=0         charCode=0        
keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1093      charCode=1093     
keyup    keyCode=0         which=0         charCode=0        
keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1098      charCode=1098     
keyup    keyCode=0         which=0         charCode=0        
keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1078      charCode=1078     
keyup    keyCode=0         which=0         charCode=0        
keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1101      charCode=1101     
keyup    keyCode=0         which=0         charCode=0        
keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1073      charCode=1073     
keyup    keyCode=0         which=0         charCode=0        
keydown  keyCode=0         which=0         charCode=0        
keypress keyCode=0         which=1102      charCode=1102     
keyup    keyCode=0         which=0         charCode=0 


Expected results:

I'll must get that output (with keycodes):

keydown  keyCode=192       which=192       charCode=0        
keypress keyCode=1105      which=1105      charCode=1105     
keyup    keyCode=192       which=192       charCode=0        
keydown  keyCode=219       which=219       charCode=0        
keypress keyCode=1093      which=1093      charCode=1093     
keyup    keyCode=219       which=219       charCode=0        
keydown  keyCode=229       which=229       charCode=0        
keydown  keyCode=221       which=221       charCode=0        
keypress keyCode=1098      which=1098      charCode=1098     
keyup    keyCode=221       which=221       charCode=0        
keydown  keyCode=229       which=229       charCode=0        
keydown  keyCode=186       which=186       charCode=0        
keypress keyCode=1078      which=1078      charCode=1078     
keyup    keyCode=186       which=186       charCode=0        
keydown  keyCode=229       which=229       charCode=0        
keydown  keyCode=222       which=222       charCode=0        
keypress keyCode=1101      which=1101      charCode=1101     
keyup    keyCode=222       which=222       charCode=0        
keydown  keyCode=229       which=229       charCode=0        
keydown  keyCode=188       which=188       charCode=0        
keypress keyCode=1073      which=1073      charCode=1073     
keyup    keyCode=188       which=188       charCode=0        
keydown  keyCode=229       which=229       charCode=0        
keydown  keyCode=190       which=190       charCode=0        
keypress keyCode=1102      which=1102      charCode=1102     
keyup    keyCode=190       which=190       charCode=0
Component: Untriaged → DOM: Events
Keywords: intl
Product: Firefox → Core
Just for the reference, this seems to work fine on Windows.
Whiteboard: [webcompat]
I stumbled upon this issue while debugging a WebCompat issue reported for Google Docs where a spreadsheet did not enable the edit mode when pressing certain keys.

This does not only causing troubles for arrow keys as reported in bug 762836, but also locale specific keys like the German ö, ä and ä keys, Russian keyboards, Norwegian keys (as mentioned in the WebCompat report) and probably some more. For those non-American keys, keyCode/which/charCode are 0 for keydown/keyup, while the keypress event has values.

Example for the "ö" key, generated using the tool linked in comment 0:

In Firefox:
> keydown  keyCode=0         which=0         charCode=0        
> keypress keyCode=0         which=246       charCode=246      
> keyup    keyCode=0         which=0         charCode=0

In Chrome:
> keydown  keyCode=186       which=186       charCode=0        
> keypress keyCode=246       which=246       charCode=246      
> keyup    keyCode=186       which=186       charCode=0        

In Safari:
> keydown  keyCode=186       which=186       charCode=0        
> keypress keyCode=246       which=246       charCode=246      
> keyup    keyCode=186       which=186       charCode=0        

Since Chrome and Safari agree, I assume Firefox is off here.

Masayuki, you were the last one to do some changes to `KeyboardEvent::KeyCode()`, can you provide some insights?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(masayuki)
Keywords: intl
OS: Linux → All
Hardware: x86_64 → All
Summary: Key codes for russian "ё", "х", "ъ", "ж", "э", "б" and "ю" keys not generated for keyup, keypress, keydown events → Key codes for arrow keys and some locale specific keys not generated for keyup and keydown events
Version: 30 Branch → 51 Branch
About keyCode, we won't have plan to change the mapping anymore because changing keyCode mapping will break existing websites which are Firefox-aware. And for solving this historical issues, we've already worked hard for implementing KeyboardEvent.key and KeyboardEvent.code. So, web authors should use the new attributes instead of legacy keyCode attribute.

Note that mapping in specific keyboard layout on specific platform is not so difficult if we can ignore the historical issues. However, it's too hard for i18n. Our keyCode mapping for printable keys depends on the primary input character of the key. See following document for the detail:
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode#Printable_keys_in_standard_position

However, I wonder, the symptom in comment 0 is Linux specific. If we cannot retrieve ASCII capable keyboard layout, we set keyCode to 0 if active keyboard layout inputs a Unicode character (like Cyrillic characters).

You've set OS to All, but keyCode setting issue is really platform (and environment) dependent. So, you should fine a new bug for different platform(s).
Flags: needinfo?(masayuki)
Thank you very much for your reply!

So, do I get you right that keyCode/which/charCode are WONTFIXes? All of them are officially deprecated and should be replaced by key and code according to MDN. If so, I guess we can handle that in some cases by outreach.

> However, I wonder, the symptom in comment 0 is Linux specific.

Unfortunately, it is not. I am getting zeros in Linux, OSX and Windows. But this would be obsolete if keyCode/which/charCode are ignored anyway.
platform-rel: --- → ?
Whiteboard: [webcompat] → [webcompat][platform-rel-Google][platform-rel-GoogleDocs]
Whiteboard: [webcompat][platform-rel-Google][platform-rel-GoogleDocs] → [webcompat][platform-rel-Google][platform-rel-GoogleSuite][platform-rel-GoogleDocs]
So should we close this as WONTFIX?
platform-rel: ? → -
Flags: needinfo?(masayuki)
(In reply to Dennis Schubert [:denschub] from comment #5)
> > However, I wonder, the symptom in comment 0 is Linux specific.
> 
> Unfortunately, it is not. I am getting zeros in Linux, OSX and Windows. But
> this would be obsolete if keyCode/which/charCode are ignored anyway.

When we can retrieve ASCII capable keyboard layouts even when active keyboard layout is non-ASCII capable like Cyrillic. Then, keyCode of keydown and keyup is computed with the secondary ASCII capable keyboard layout. However, computed as non-zero only when the key is [a-z0-9] because non-ASCII capable keyboard layouts may also inputs ASCII punctuation from different key with the secondary ASCII capable keyboard layout.

According to Russian typewriter layout on Win10, "ё", "х", "ъ", "ж", "э", "б" and "ю" are mapped on OEM keys (i.e., inputting a punctuation on ASCII capable keyboard layout). Therefore, they are computed as 0. So, that's intentional behavior at least for now.

(In reply to Andrew Overholt [:overholt] from comment #6)
> So should we close this as WONTFIX?

Currently we don't have any idea to change the behavior without breaking existing Firefox-aware websites, so, the answer is yes.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(masayuki)
Resolution: --- → WONTFIX
Well, whiteboard is "[webcompat][platform-rel-Google][platform-rel-GoogleSuite][platform-rel-GoogleDocs]".

So, does this cause some trouble in Google apps actually?
Flags: needinfo?(dchinniah)
Oops, that's written in comment 3.
Flags: needinfo?(dchinniah)
As I said in comment 7, we use 0 for keyCode of some keys of non-ASCII capable keyboard layout which are positioned at punctuation keys of ASCII capable keyboard layout. The reason is, avoiding to conflict with punctuation keys on non-ASCII capable keyboard layout.

I wonder, the reason *was*, guaranteeing that keyCode is an identifier of each key on active keyboard layout. Now, it can be check with standardized KeyboardEvent.code (or KeyboardEvent.key) strictly. On the other hand, setting 0 to KeyboardEvent.keyCode means that the keys are *dead" on web apps. So, setting KeyboardEvent.keyCode even from punctuation keys of "secondary" ASCII capable keyboard layout is better than current behavior and it must not be risky change.

How do you think this idea, smaug, karlt?
Flags: needinfo?(karlt)
Flags: needinfo?(bugs)
sound reasonable. And does that give better compatibility with Edge and Chrome?
Flags: needinfo?(bugs)
(In reply to Masayuki Nakano [:masayuki] from comment #10)
> I wonder, the reason *was*, guaranteeing that keyCode is an identifier of
> each key on active keyboard layout. Now, it can be check with standardized
> KeyboardEvent.code (or KeyboardEvent.key) strictly. On the other hand,
> setting 0 to KeyboardEvent.keyCode means that the keys are *dead" on web
> apps. So, setting KeyboardEvent.keyCode even from punctuation keys of
> "secondary" ASCII capable keyboard layout is better than current behavior
> and it must not be risky change.

Doing that may mean that more than one key can produce the same keyCode (as you imply IIUC).  Yes, perhaps that is not as much a problem as zero keyCodes are.

I don't have a feel for how these are used.  More specifics about the problem being reported may help.

But I agree that making more keys report non-zero keyCodes sounds good, if it is done in a way that makes Gecko more consistent with other browsers.

If the key generates ASCII punctuation in both the active and Latin/secondary layout, then from which layout would the punctuation be selected?
Flags: needinfo?(karlt)
Thank you for your feedback!

(In reply to Olli Pettay [:smaug] from comment #11)
> sound reasonable. And does that give better compatibility with Edge and
> Chrome?

Not completely but better than current implementation.

Our rules of deciding keyCode of printable keys are basically decided with inputting character, but Edge just exports Windows's native virtual keycode values and Chromium maps them from scan code (Windows), hardware keycode (Linux) and native virtual keycode (Mac).

So, the value could be different from other browsers. However, as far as I know, most non-ASCII capable keyboard layout users use ANSI keyboard layout when they inputs ASCII characters. Therefore, the difference could occur with a few keyboard layouts (I hope).

(If we change the rule now drastically, we break web apps which are Firefox-aware.)

(In reply to Karl Tomlinson (:karlt) from comment #12)
> If the key generates ASCII punctuation in both the active and
> Latin/secondary layout, then from which layout would the punctuation be
> selected?

Good point. In such case, we are setting keyCode from active keyboard layout. I think that we should not change the behavior. So, I'm thinking that only when we ignore secondary ASCII capable keyboard layout's keyCode due to a punctuation key, we should change the behavior.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Flags: needinfo?(miket)
Still affecting Google spreadsheets: https://github.com/webcompat/web-bugs/issues/14931
Flags: needinfo?(miket)
Assignee: nobody → masayuki
Status: REOPENED → ASSIGNED
denshub:

What does it mean "arrow" keys here? This bug report was for keyCode values of OEM keys. But looks like that you added different issue into this bug.
Flags: needinfo?(dschubert)
Summary: Key codes for arrow keys and some locale specific keys not generated for keyup and keydown events → KeyboardEvent.keyCode for OEM key is 0 if the key doesn't produce an ASCII character itself nor with Shift
Smaug:

There are two possible scenarios of the reason why web applications cannot work well by this bug.

1. Web applications doesn't support Firefox completely. They check keyCode values only with Chrome's keyCode table.
2. Web applications support Firefox only when active keyboard layout is ASCII capable keyboard layout like US layout.

If we change keyCode values for punctuation keys of US layout from 0, better solution is different for those cases.

If we need to fix this bug for the case #1, we should use exactly same values as Chromium. We can refer Chromium's keyCode table and this is possible. However, even if we take this approach, some punctuation keys which produce ASCII character by itself or with Shift state won't match with Chromium's result. I think that we should not change non-zero keyCode values anymore for backward compatibility with existing web applications which are aware of Firefox.

If we need to fix this bug for the case #2, we should compute keyCode value with our computing rules with alternative ASCII keyboard layout. Then, if such web applications don't work only when active keyboard layout is not ASCII capable, this approach may work well with existing path of the web applications. Although we shouldn't change non-zero keyCode value in this case too. So, we may not be able to fix this bug even with this approach.

I guess that most cases are #2. So, even if we expose different keyCode value from Chrome/Safari, we should keep consistency with US keyboard layout of Firefox. How do you think?

If we'd assume #1, web applications cannot support both ASCII capable keyboard layout and non-ASCII capable keyboard layout with Firefox since they cannot know active keyboard layout information, e.g., key pressed when whether active keyboard layout is US layout or Russian, Arabic, etc.
Flags: needinfo?(bugs)
Summary: KeyboardEvent.keyCode for OEM key is 0 if the key doesn't produce an ASCII character itself nor with Shift → KeyboardEvent.keyCode for OEM key (punctuation key in US layout) is 0 if the key doesn't produce an ASCII character itself nor with Shift
Masayuki, I added the arrow keys to this bug as it was reported in bug 762836. Back then, we also reported which=0 for arrow keys. However, I see that those are working now, mostly. We have keyCode=which for keydown/keyup, but still report 0 for keypress. But this may be a different issue, yes. I only grouped them together since I didn't know any internals. :)
Flags: needinfo?(dschubert)
(In reply to Dennis Schubert [:denschub] from comment #19)
> we also reported which=0 for arrow keys. However, I see
> that those are working now, mostly. We have keyCode=which for keydown/keyup,
> but still report 0 for keypress. But this may be a different issue, yes.

Thank you for the clarification.

FYI: We'll stop dispatching keypress event for non-printable key combinations like Ctrl+A, Arrow(Left|Right|Up|Down) keys (bug 968056).
FYI: Looks like that we don't need to touch Android because Android widget decides DOM keyCode from native virtual keycode simply:
https://searchfox.org/mozilla-central/rev/5536f71c3833018c4f4e2c73f37eae635aab63ff/widget/android/GeckoEditableSupport.cpp#277-285
Comment on attachment 8952108 [details]
Bug 1036008 - Use alternative ASCII capable keyboard layout information to decide keyCode even if the key produces an ASCII punctuation character

I'll update this patch soon. We can make move common logic to XP part.
Attachment #8952108 - Flags: review?(bugs) → review-
Comment on attachment 8952108 [details]
Bug 1036008 - Use alternative ASCII capable keyboard layout information to decide keyCode even if the key produces an ASCII punctuation character

https://reviewboard.mozilla.org/r/221338/#review227452

::: widget/TextEvents.h:467
(Diff revision 2)
> +  /**
> +   * GetFallbackKeyCodeOfPunctuationKey() returns a DOM keyCode value for
> +   * aCodeNameIndex.  This is keyCode value of the key when active keyboard
> +   * layout is ANSI (US), JIS or ABNT keyboard layout (the latter 2 layouts
> +   * are used only when ANSI doesn't have the key).  The result is useful
> +   * if the key won't produce ASCII character with active keyboard layout

s/won't/doesn't/


I don't quite understand what the
'nor' refers to.
"key won't produce character..."
"nor ...layout"

I guess it should be
"...with active keyboard layout nor with alternative ...."

::: widget/gtk/nsGtkKeyUtils.cpp:798
(Diff revision 2)
>          return WidgetUtils::ComputeKeyCodeFromChar(unmodifiedChar);
>      }
>  
>      // If the unmodified character is not an ASCII character, that means we
>      // couldn't find the hint. We should reset it.
> -    if (unmodifiedChar > 0x7F) {
> +    if (unmodifiedChar < 0x20 || unmodifiedChar > 0x7F) {

It would be nice to have a helper which checks if a char either < 0x20 or > 0x7F.
And have some comment why such range is checked.
Attachment #8952108 - Flags: review?(bugs) → review+
Comment on attachment 8952108 [details]
Bug 1036008 - Use alternative ASCII capable keyboard layout information to decide keyCode even if the key produces an ASCII punctuation character

https://reviewboard.mozilla.org/r/221338/#review227452

> It would be nice to have a helper which checks if a char either < 0x20 or > 0x7F.
> And have some comment why such range is checked.

Okay, I'll add I IsPrintableASCIICharacter() and also it avoids returning true for U+007F. Although, WidgetUtils::ComputeKeyCodeFromChar() returns 0 if given charCode is 0x7F. So, this change must not cause any regression (also I don't know keyboard layouts which produces U+007F by a key itself or only with Shift key).
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/097cade856ec
Use alternative ASCII capable keyboard layout information to decide keyCode even if the key produces an ASCII punctuation character r=smaug
https://hg.mozilla.org/mozilla-central/rev/097cade856ec
Status: ASSIGNED → RESOLVED
Closed: 7 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #31)
> Document updated:
> https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/
> keyCode#keyCode_of_punctuation_keys_on_some_keyboard_layout

Thanks for the docs updates!

I've copy edited the text - could you have a quick look to make sure I've not affected the meaning in a negative way?

One part I didn't understand was "when 7.1 or 7.2 is reached" - can you clarify what this means?
Flags: needinfo?(masayuki)
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #33)
> I've copy edited the text - could you have a quick look to make sure I've
> not affected the meaning in a negative way?

Sure. Looks good but I made "keycode" to "<code>keyCode</code>" for making clearer if it's DOM keyCode or virtual keycode of OS.

> One part I didn't understand was "when 7.1 or 7.2 is reached" - can you
> clarify what this means?

Ah, I meant the list item of above lists. Could you rewrite it with what you can understand it so easily?
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #34)
> (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #33)
> > I've copy edited the text - could you have a quick look to make sure I've
> > not affected the meaning in a negative way?
> 
> Sure. Looks good but I made "keycode" to "<code>keyCode</code>" for making
> clearer if it's DOM keyCode or virtual keycode of OS.
> 
> > One part I didn't understand was "when 7.1 or 7.2 is reached" - can you
> > clarify what this means?
> 
> Ah, I meant the list item of above lists. Could you rewrite it with what you
> can understand it so easily?

OK, cool - I know what you mean now. I have rewritten it.
You need to log in before you can comment on or make changes to this bug.