jplayer / jPlayer

jPlayer : HTML5 Audio & Video for jQuery
http://jplayer.org/
Other
4.6k stars 1.47k forks source link

Add support for KeyboardEvent.event #253

Closed ghost closed 9 years ago

ghost commented 9 years ago

jPlayer allows custom keybindings with "keybindings"-property in the constructor. The key is detected using deprecated KeyboardEvent.which -attribute. Please add support for KeyboardEvent.key which allows defining keys by value instead of non-standard keycodes.

Proposed implementation:

If key-property is of type String and KeyboardEvent.key is supported (Only in newer browser, compatibility table: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.key) bind to KeyboardEvent.key. Otherwise bind to KeyboardEvent.which.

thepag commented 9 years ago

I'll take a look at the key property...

But be aware that I am likely to be disabling key controls by default since they interfere with ARIA features. In the least, the keys will have to be changed to avoid the main ARIA key controls of spacebar, enter and the arrow keys. We currently use all of them and ARIA is not happy... Mobile have no idea they are even there so don't care... And desktop users have a mouse or touch screen.

While I plan to leave in key controls, and will look into this event.key property, the ARIA controls are the most important. After all, isn't that the whole point of keyboard controls in the first place? To improve accessibility? And if the user is using a true ARIA enabled device, then it will work fine through using the controls they know well, rather than using the letter "g" to play since it is the first letter of the word "go" in english.

This is wip, and i seemed to go on a bit above... Thank you for reporting it.

ghost commented 9 years ago

Thank you

I agree, ARIA is great, and preventing interfering keys is the right way to go. According to this guideline, the so called 'mnemonic' key-shortcuts are recommended: http://www.w3.org/TR/wai-aria-practices/#keyboard (See "3.1.4. Keyboard Shortcuts for Widgets").

The problem with the media keys is that they're not captured by (all, if any) browsers. So the user has to install an extension to actually get them working...Nevertheless, I appreciate if you can implement this feature at some point.

thepag commented 9 years ago

Support for this KeyboardEvent.key is pretty poor atm, it appears to be only IE9+ and Firefox, and where there is support, a lot of the key codes being generated (by current Firefox) are already obsolete.

Other refactor factors include:

Thought on implementing:

In this way, we can keep backwards compatibility and when one of these new event.key string codes are used, we can attempt to compare it with the key property of the event - where it is defined.

Ref: MDN KeyboardEvent.key

thepag commented 9 years ago

I have nearly finished this change.

While doing it, I've fixed the key bindings failing in Internet Explorer, which was failing due to IE allowing all sorts of elements to gain focus of the document.activeElement. This was happening in IE8 and IE9 at least, but that was enough to force me to revert to the original way of doing it, which was still in the code as a fallback to activeElement.

I am also changing the default key bindings since they conflict with the ARIA keys and they are now mnemonic. Well, the toggles are mnemonic.

thepag commented 9 years ago

Something strange is going on with the plus and minus keys. The codes for Firefox differ to the other main browsers. - is 189 = is 187 But in firefox: - is 173 = is 61

Think I will switch to the , and . keys since they look like < and >

thepag commented 9 years ago

Implemented in 2.8.2