fix: make sure hotkeys are not triggered outside the player or in form fields within the player (#5969)

Author: misteroneill

src/js/big-play-button.js

@@ -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;
     }
 
@@ -69,10 +68,10 @@ class BigPlayButton extends Button {
     }
   }
 
-  handleKeyPress(event) {
+  handleKeyDown(event) {
     this.mouseused_ = false;
 
-    super.handleKeyPress(event);
+    super.handleKeyDown(event);
   }
 
   handleMouseDown(event) {

src/js/button.js

@@ -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);
   }
 }
 

src/js/clickable-component.js

@@ -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
  */
@@ -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.
@@ -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.
@@ -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.
@@ -146,7 +143,7 @@ class ClickableComponent extends Component {
   }
 
   /**
-   * Enable this `Component`s element.
+   * Enable this `ClickableComponent`
    */
   enable() {
     if (!this.enabled_) {
@@ -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;
@@ -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
@@ -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);

src/js/close-button.js

@@ -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

src/js/component.js

@@ -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

src/js/control-bar/progress-control/seek-bar.js

@@ -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);
     }
   }
 }