Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getEventKey implementation inconsistent with DOM3 spec / Firefox implementation #2193

Open
Daniel15 opened this issue Sep 15, 2014 · 6 comments

Comments

@Daniel15
Copy link
Member

There are some inconsistencies between getEventKey and the DOM3 keyboard event spec (as well as what Firefox has implemented):

  • key property is not correctly set for printable characters on keydown and keyup events. This works properly in Firefox, and my interpretation of the standard is that Firefox's behaviour is correct here. From the spec: If the key generates a printable character, and there exists an appropriate Unicode code point, then the KeyboardEvent.key attribute must be a string consisting of the char value of that character.
  • Enter key only fires keydown in Firefox, but fires both keydown and keypress in Chrome. This should be consistent across browsers
  • CapsLock key only fires keydown when it is toggled on. When caps lock is toggled from on to off, no keydown event is fired (this may be a browser limitation in Chrome)

Repro: Test this page in Firefox and compare the result to Chrome: http://jsfiddle.net/63ycmLhe/1/

@syranide
Copy link
Contributor

  1. https://github.com/facebook/react/blob/master/src/browser/ui/dom/getEventKey.js#L103
  2. This should already be fixed by Improve normalization of charCode, fix FF not reporting Enter #1956
  3. There is no way to determine caps lock state, so unless there's something to suggest otherwise, I'm sure this is definitely an unavoidable browser limitation.

@Daniel15
Copy link
Member Author

I'm not quite sure what https://github.com/facebook/react/blob/master/src/browser/ui/dom/getEventKey.js#L103 is meant to mean as the comment is not really clear to me. Does it mean keyCode varies depending on keyboard layout?

In any case, if we don't want to include printable characters in the key property in keydown and keyup, we should be consistently doing that across all browsers, normalising the event where appropriate. At the moment, Firefox and Chrome have differing behaviour here.

@syranide
Copy link
Contributor

@Daniel15 keyCode refers to the "virtual key code", i.e. the ID of the key on the keyboard. It is technically useless unless you know the keyboard layout, but thankfully all keyboards we're likely to encounter agree on (most) function keys. There is no way to determine the keyboard layout (including caps lock state, etc) so there's literally nothing we can do to polyfill printable keys.

I don't agree with holding back key-support according to the browsers we target, we gain nothing by reporting Unidentified for browsers which has native support and does report character values or a more complete set of keys. That said, the current native key support is a mess (and somewhat broken) so explicitly ignoring native key for broken browsers has some merit, but I don't think it's a game we really want to play (and I don't think there's any demand to justify the dev-cost).

@Daniel15
Copy link
Member Author

we gain nothing by reporting Unidentified for browsers which has native support and does report character values or a more complete set of keys.

I disagree, I think we gain a baseline for expected behaviour and what can safely be relied upon. At the moment, someone could be using key for printable keys and not even realise it only works in Firefox.

The whole reason people use JavaScript libraries like jQuery and MooTools is that they smooth over browser incompatibilities and provide good baseline functionality. If you write some jQuery event handling code, you can be mostly sure that it will work the same way cross-browser. React's event system should be doing the same thing and providing equal cross-browser support.

@Daniel15
Copy link
Member Author

Or if we can't do that, at least have a large warning in the help explaining that key is flaky and should not be relied upon except for function keys.

@aweary
Copy link
Contributor

aweary commented Jun 25, 2018

I tested this with latest 16 release:

  • key appears to be correct for keydown/keyup events in both Chrome and Firefox.
  • Both Chrome and Firefox only fire keypress and keydown events for Enter. This W3C keyboard event fixture indicates that both browsers are firing keyup events.
  • According to the W3C fixture: Chrome fires a keydown event when CapsLock is enabled, and a keyup event when its disabled. Firefox only fires a keydown event in both cases. React is consistent with this, except it does not fire the keyup event in Chrome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants