Keyboard controls for buttons propagate causing double behavior

[heff]

The spacebar and enter key can be used to trigger a button, based on accessibility guidelines. We do preventDefault on those events, but they're still apparently bubbling up to the browser.

This may relate to #1446.

What did you do? (steps to reproduce)

1. Go to videojs.com.
2. Press tab until the play button is highlighted.
3. Press spacebar.

What happened? (actual results)

The player will start playing and also the page will jump down, like the normal space bar operation.

What should have happened? (expected results)

The event should have been prevented from reaching the browser so the pages doesn't jump down.

What version of Video.js, and any plugins used?

4.7

What browsers and platforms does this occur/not occur on?

Mac, Safari and Chrome

[songpete]

Is there a reason the 'onKeyPress' function binds on the 'keyup' event (line 68 of Button.js)? By then, it's too late to prevent the default behavior of moving the page. If you change it to bind on 'keydown' it will prevent the default behavior in time (in Chrome at least). I don't know if this will work in all browsers.

[heff]

Hmm, that could be the way to fix it. I can't think of any reason why it needs to be on keyup, except that it's more the completion of the action like click. But the browser doesn't wait for keyup to jump the page so I guess that's what we have to do either way. Good find, thanks! Would you want to create a PR for that?

[albell]

+1. keydown has the added UX benefit of feeling more immediate to the user, and less laggy, because it fires earlier. This is the canonical way to write a keyboard handler. If we ever need for the default action to have already occurred, just call the callback function with a timeout of zero. Listening for keyup is generally something of an antipattern, and typically to be avoided.

[mmcc]

#1455 should have taken care of this one.