PolymerElements / iron-a11y-keys-behavior

A behavior that enables keybindings for greater a11y
24 stars 41 forks source link

'KeyboardEvent.keyIdentifier' is deprecated warning in Chrome #53

Closed web-padawan closed 8 years ago

web-padawan commented 8 years ago

Description

'KeyboardEvent.keyIdentifier' is deprecated and will be removed in M53, around September 2016. See https://www.chromestatus.com/features/5316065118650368 for more details.

This warning is caused by line 182:

transformKeyIdentifier(keyEvent.keyIdentifier) ||

Seems that this should be removed.

web-padawan commented 8 years ago

I'm going to send a PR to fix that warning. Shouldn't we get rid of the keyIdentifier at all? /cc @valdrinkoshi

valdrinkoshi commented 8 years ago

I'm afraid it would be a breaking change as other browsers don't support key yet, and the other options (keyCode, charCode) are no better than keyIdentifier...

samccone commented 8 years ago

@valdrinkoshi thoughts on falling back to key Identifier if key is not supported?

valdrinkoshi commented 8 years ago

@samccone that's already happening, see https://github.com/PolymerElements/iron-a11y-keys-behavior/blob/master/iron-a11y-keys-behavior.html#L178

Fall back from .key, to .keyIdentifier, to .keyCode, and then to .detail.key to support artificial keyboard events

samccone commented 8 years ago

correct, but switching the order of the fallback so that .keyIdentifier is lower on the || fall through. Testing a patch locally now.

valdrinkoshi commented 8 years ago

Unfortunately the warning gets displayed anyways if for example you hit "?" (shift + / key in US keyboard). I think that normalizedKeyForEvent should be converted to something like:

if (keyEvent.key) {
  return transformKey(keyEvent.key, noSpecialChars);
}
if (keyEvent.detail && keyEvent.detail.key) {
  return transformKey(keyEvent.detail.key, noSpecialChars);
}
return transformKeyIdentifier(keyEvent.keyIdentifier) ||
  transformKeyCode(keyEvent.keyCode) || '';

I'll take care of that (there are some tests to be fixed)