Skip to content

Commit

Permalink
fix: make sure hotkeys are not triggered outside the player or in for…
Browse files Browse the repository at this point in the history
…m fields within the player (#5969)
  • Loading branch information
misteroneill authored and gkatsev committed Jun 10, 2019
1 parent 5a7fe48 commit 79eadac
Show file tree
Hide file tree
Showing 17 changed files with 674 additions and 275 deletions.
9 changes: 4 additions & 5 deletions src/js/big-play-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,15 @@ class BigPlayButton extends Button {
// exit early if clicked via the mouse
if (this.mouseused_ && event.clientX && event.clientY) {
silencePromise(playPromise);
// call handleFocus manually to get hotkeys working
this.player_.handleFocus({});
this.player_.tech(true).focus();
return;
}

const cb = this.player_.getChild('controlBar');
const playToggle = cb && cb.getChild('playToggle');

if (!playToggle) {
this.player_.focus();
this.player_.tech(true).focus();
return;
}

Expand All @@ -69,10 +68,10 @@ class BigPlayButton extends Button {
}
}

handleKeyPress(event) {
handleKeyDown(event) {
this.mouseused_ = false;

super.handleKeyPress(event);
super.handleKeyDown(event);
}

handleMouseDown(event) {
Expand Down
19 changes: 13 additions & 6 deletions src/js/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,20 @@ class Button extends ClickableComponent {
*
* @listens keydown
*/
handleKeyPress(event) {
// Ignore Space or Enter key operation, which is handled by the browser for a button.
if (!(keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter'))) {

// Pass keypress handling up for unsupported keys
super.handleKeyPress(event);
handleKeyDown(event) {

// Ignore Space or Enter key operation, which is handled by the browser for
// a button - though not for its super class, ClickableComponent. Also,
// prevent the event from propagating through the DOM and triggering Player
// hotkeys. We do not preventDefault here because we _want_ the browser to
// handle it.
if (keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter')) {
event.stopPropagation();
return;
}

// Pass keypress handling up for unsupported keys
super.handleKeyDown(event);
}
}

Expand Down
81 changes: 22 additions & 59 deletions src/js/clickable-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@
*/
import Component from './component';
import * as Dom from './utils/dom.js';
import * as Events from './utils/events.js';
import * as Fn from './utils/fn.js';
import log from './utils/log.js';
import document from 'global/document';
import {assign} from './utils/obj';
import keycode from 'keycode';

/**
* Clickable Component which is clickable or keyboard actionable,
* but is not a native HTML button.
* Component which is clickable or keyboard actionable, but is not a
* native HTML button.
*
* @extends Component
*/
Expand All @@ -36,7 +33,7 @@ class ClickableComponent extends Component {
}

/**
* Create the `Component`s DOM element.
* Create the `ClickableComponent`s DOM element.
*
* @param {string} [tag=div]
* The element's node type.
Expand Down Expand Up @@ -83,7 +80,7 @@ class ClickableComponent extends Component {
}

/**
* Create a control text element on this `Component`
* Create a control text element on this `ClickableComponent`
*
* @param {Element} [el]
* Parent element for the control text.
Expand All @@ -109,7 +106,7 @@ class ClickableComponent extends Component {
}

/**
* Get or set the localize text to use for the controls on the `Component`.
* Get or set the localize text to use for the controls on the `ClickableComponent`.
*
* @param {string} [text]
* Control text for element.
Expand Down Expand Up @@ -146,7 +143,7 @@ class ClickableComponent extends Component {
}

/**
* Enable this `Component`s element.
* Enable this `ClickableComponent`
*/
enable() {
if (!this.enabled_) {
Expand All @@ -157,13 +154,12 @@ class ClickableComponent extends Component {
this.el_.setAttribute('tabIndex', this.tabIndex_);
}
this.on(['tap', 'click'], this.handleClick);
this.on('focus', this.handleFocus);
this.on('blur', this.handleBlur);
this.on('keydown', this.handleKeyDown);
}
}

/**
* Disable this `Component`s element.
* Disable this `ClickableComponent`
*/
disable() {
this.enabled_ = false;
Expand All @@ -173,27 +169,15 @@ class ClickableComponent extends Component {
this.el_.removeAttribute('tabIndex');
}
this.off(['tap', 'click'], this.handleClick);
this.off('focus', this.handleFocus);
this.off('blur', this.handleBlur);
this.off('keydown', this.handleKeyDown);
}

/**
* This gets called when a `ClickableComponent` gets:
* - Clicked (via the `click` event, listening starts in the constructor)
* - Tapped (via the `tap` event, listening starts in the constructor)
* - The following things happen in order:
* 1. {@link ClickableComponent#handleFocus} is called via a `focus` event on the
* `ClickableComponent`.
* 2. {@link ClickableComponent#handleFocus} adds a listener for `keydown` on using
* {@link ClickableComponent#handleKeyPress}.
* 3. `ClickableComponent` has not had a `blur` event (`blur` means that focus was lost). The user presses
* the space or enter key.
* 4. {@link ClickableComponent#handleKeyPress} calls this function with the `keydown`
* event as a parameter.
* Event handler that is called when a `ClickableComponent` receives a
* `click` or `tap` event.
*
* @param {EventTarget~Event} event
* The `keydown`, `tap`, or `click` event that caused this function to be
* called.
* The `tap` or `click` event that caused this function to be called.
*
* @listens tap
* @listens click
Expand All @@ -202,52 +186,31 @@ class ClickableComponent extends Component {
handleClick(event) {}

/**
* This gets called when a `ClickableComponent` gains focus via a `focus` event.
* Turns on listening for `keydown` events. When they happen it
* calls `this.handleKeyPress`.
* Event handler that is called when a `ClickableComponent` receives a
* `keydown` event.
*
* @param {EventTarget~Event} event
* The `focus` event that caused this function to be called.
*
* @listens focus
*/
handleFocus(event) {
Events.on(document, 'keydown', Fn.bind(this, this.handleKeyPress));
}

/**
* Called when this ClickableComponent has focus and a key gets pressed down. By
* default it will call `this.handleClick` when the key is space or enter.
* By default, if the key is Space or Enter, it will trigger a `click` event.
*
* @param {EventTarget~Event} event
* The `keydown` event that caused this function to be called.
*
* @listens keydown
*/
handleKeyPress(event) {
// Support Space or Enter key operation to fire a click event
handleKeyDown(event) {

// Support Space or Enter key operation to fire a click event. Also,
// prevent the event from propagating through the DOM and triggering
// Player hotkeys.
if (keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter')) {
event.preventDefault();
event.stopPropagation();
this.trigger('click');
} else {

// Pass keypress handling up for unsupported keys
super.handleKeyPress(event);
super.handleKeyDown(event);
}
}

/**
* Called when a `ClickableComponent` loses focus. Turns off the listener for
* `keydown` events. Which Stops `this.handleKeyPress` from getting called.
*
* @param {EventTarget~Event} event
* The `blur` event that caused this function to be called.
*
* @listens blur
*/
handleBlur(event) {
Events.off(document, 'keydown', Fn.bind(this, this.handleKeyPress));
}
}

Component.registerComponent('ClickableComponent', ClickableComponent);
Expand Down
19 changes: 2 additions & 17 deletions src/js/close-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,10 @@ class CloseButton extends Button {
return `vjs-close-button ${super.buildCSSClass()}`;
}

/**
* This gets called when a `CloseButton` has focus and `keydown` is triggered via a key
* press.
*
* @param {EventTarget~Event} event
* The event that caused this function to get called.
*
* @listens keydown
*/
handleKeyPress(event) {
// Override the default `Button` behavior, and don't pass the keypress event
// up to the player because this button is part of a `ModalDialog`, which
// doesn't pass keypresses to the player either.
}

/**
* This gets called when a `CloseButton` gets clicked. See
* {@link ClickableComponent#handleClick} for more information on when this will be
* triggered
* {@link ClickableComponent#handleClick} for more information on when
* this will be triggered
*
* @param {EventTarget~Event} event
* The `keydown`, `tap`, or `click` event that caused this function to be
Expand Down
23 changes: 20 additions & 3 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -1078,18 +1078,35 @@ class Component {
}

/**
* When this Component receives a keydown event which it does not process,
* When this Component receives a `keydown` event which it does not process,
* it passes the event to the Player for handling.
*
* @param {EventTarget~Event} event
* The `keydown` event that caused this function to be called.
*/
handleKeyPress(event) {
handleKeyDown(event) {
if (this.player_) {
this.player_.handleKeyPress(event);

// We only stop propagation here because we want unhandled events to fall
// back to the browser.
event.stopPropagation();
this.player_.handleKeyDown(event);
}
}

/**
* Many components used to have a `handleKeyPress` method, which was poorly
* named because it listened to a `keydown` event. This method name now
* delegates to `handleKeyDown`. This means anyone calling `handleKeyPress`
* will not see their method calls stop working.
*
* @param {EventTarget~Event} event
* The event that caused this function to be called.
*/
handleKeyPress(event) {
this.handleKeyDown(event);
}

/**
* Emit a 'tap' events when touch event support gets detected. This gets used to
* support toggling the controls through a tap on the video. They get enabled
Expand Down
12 changes: 9 additions & 3 deletions src/js/control-bar/progress-control/seek-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,30 +407,36 @@ class SeekBar extends Slider {
*
* @listens keydown
*/
handleKeyPress(event) {
handleKeyDown(event) {
if (keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter')) {
event.preventDefault();
event.stopPropagation();
this.handleAction(event);
} else if (keycode.isEventKey(event, 'Home')) {
event.preventDefault();
event.stopPropagation();
this.player_.currentTime(0);
} else if (keycode.isEventKey(event, 'End')) {
event.preventDefault();
event.stopPropagation();
this.player_.currentTime(this.player_.duration());
} else if (/^[0-9]$/.test(keycode(event))) {
event.preventDefault();
event.stopPropagation();
const gotoFraction = (keycode.codes[keycode(event)] - keycode.codes['0']) * 10.0 / 100.0;

this.player_.currentTime(this.player_.duration() * gotoFraction);
} else if (keycode.isEventKey(event, 'PgDn')) {
event.preventDefault();
event.stopPropagation();
this.player_.currentTime(this.player_.currentTime() - (STEP_SECONDS * PAGE_KEY_MULTIPLIER));
} else if (keycode.isEventKey(event, 'PgUp')) {
event.preventDefault();
event.stopPropagation();
this.player_.currentTime(this.player_.currentTime() + (STEP_SECONDS * PAGE_KEY_MULTIPLIER));
} else {
// Pass keypress handling up for unsupported keys
super.handleKeyPress(event);
// Pass keydown handling up for unsupported keys
super.handleKeyDown(event);
}
}
}
Expand Down
Loading

0 comments on commit 79eadac

Please sign in to comment.