fix(fs): make sure there's only one fullscreenchange event (videojs#5686)

gkatsev · web-flow · commit 2f00a68a51c6 · 2019-01-07T16:38:47.000-05:00
Before the fullscreen API was un-prefixed, we listened to triggered a `fullscreenchange` event on the player manually. This worked fine because a prefixed `fullscreenchange` event was triggered on the player element itself. But when it was unprefixed, we ended up with two events now, the unprefixed event and the one we triggered. Instead, we should listen the event the browser supports and re-trigger it as the unprefixed event as necessary. We also make sure that the handler gets calls when the document level fullscreenchange handler gets called so that it is cancelled properly. Fixes videojs#5685.
diff --git a/src/js/fullscreen-api.js b/src/js/fullscreen-api.js
@@ -64,6 +64,7 @@ const apiMap = [
 
 const specApi = apiMap[0];
 let browserApi;
+let prefixedAPI = true;
 
 // determine the supported set of functions
 for (let i = 0; i < apiMap.length; i++) {
@@ -79,6 +80,9 @@ if (browserApi) {
   for (let i = 0; i < browserApi.length; i++) {
     FullscreenApi[specApi[i]] = browserApi[i];
   }
+
+  prefixedAPI = browserApi[0] !== specApi[0];
 }
 
 export default FullscreenApi;
+export { prefixedAPI };
diff --git a/src/js/player.js b/src/js/player.js
@@ -15,12 +15,13 @@ import * as Dom from './utils/dom.js';
 import * as Fn from './utils/fn.js';
 import * as Guid from './utils/guid.js';
 import * as browser from './utils/browser.js';
+import {IE_VERSION} from './utils/browser.js';
 import log, { createLogger } from './utils/log.js';
 import toTitleCase, { titleCaseEquals } from './utils/to-title-case.js';
 import { createTimeRange } from './utils/time-ranges.js';
 import { bufferedPercent } from './utils/buffer.js';
 import * as stylesheet from './utils/stylesheet.js';
-import FullscreenApi from './fullscreen-api.js';
+import FullscreenApi, {prefixedAPI as prefixedFS} from './fullscreen-api.js';
 import MediaError from './media-error.js';
 import safeParseTuple from 'safe-json-parse/tuple';
 import {assign} from './utils/obj';
@@ -33,7 +34,6 @@ import * as middleware from './tech/middleware.js';
 import {ALL as TRACK_TYPES} from './tracks/track-types';
 import filterSource from './utils/filter-source';
 import {getMimetype, findMimetype} from './utils/mimetypes';
-import {IE_VERSION} from './utils/browser';
 
 // The following imports are used only to ensure that the corresponding modules
 // are always included in the video.js package. Importing the modules will
@@ -519,7 +519,15 @@ class Player extends Component {
     this.reportUserActivity();
 
     this.one('play', this.listenForUserActivity_);
-    this.on('fullscreenchange', this.handleFullscreenChange_);
+
+    if (FullscreenApi.fullscreenchange) {
+      this.on(FullscreenApi.fullscreenchange, this.handleFullscreenChange_);
+
+      if (IE_VERSION || browser.IS_FIREFOX && prefixedFS) {
+        this.on(document, FullscreenApi.fullscreenchange, this.handleFullscreenChange_);
+      }
+    }
+
     this.on('stageclick', this.handleStageClick_);
 
     this.breakpoints(this.options_.breakpoints);
@@ -549,6 +557,11 @@ class Player extends Component {
     // prevent dispose from being called twice
     this.off('dispose');
 
+    // make sure to remove fs handler on IE from the document
+    if (IE_VERSION || browser.IS_FIREFOX && prefixedFS) {
+      this.off(document, FullscreenApi.fullscreenchange, this.handleFullscreenChange_);
+    }
+
     if (this.styleEl_ && this.styleEl_.parentNode) {
       this.styleEl_.parentNode.removeChild(this.styleEl_);
       this.styleEl_ = null;
@@ -1908,29 +1921,60 @@ class Player extends Component {
     event.preventDefault();
   }
 
+  /**
+   * native click events on the SWF aren't triggered on IE11, Win8.1RT
+   * use stageclick events triggered from inside the SWF instead
+   *
+   * @private
+   * @listens stageclick
+   */
+  handleStageClick_() {
+    this.reportUserActivity();
+  }
+
   /**
    * Fired when the player switches in or out of fullscreen mode
    *
    * @private
    * @listens Player#fullscreenchange
+   * @listens Player#webkitfullscreenchange
+   * @listens Player#mozfullscreenchange
+   * @listens Player#MSFullscreenChange
+   * @fires Player#fullscreenchange
    */
-  handleFullscreenChange_() {
+  handleFullscreenChange_(event = {}, retriggerEvent = true) {
     if (this.isFullscreen()) {
       this.addClass('vjs-fullscreen');
     } else {
       this.removeClass('vjs-fullscreen');
     }
+
+    if (prefixedFS && retriggerEvent) {
+      /**
+       * @event Player#fullscreenchange
+       * @type {EventTarget~Event}
+       */
+      this.trigger('fullscreenchange');
+    }
   }
 
   /**
-   * native click events on the SWF aren't triggered on IE11, Win8.1RT
-   * use stageclick events triggered from inside the SWF instead
-   *
-   * @private
-   * @listens stageclick
+   * when the document fschange event triggers it calls this
    */
-  handleStageClick_() {
-    this.reportUserActivity();
+  documentFullscreenChange_(e) {
+    const fsApi = FullscreenApi;
+
+    this.isFullscreen(document[fsApi.fullscreenElement]);
+
+    // If cancelling fullscreen, remove event listener.
+    if (this.isFullscreen() === false) {
+      Events.off(document, fsApi.fullscreenchange, Fn.bind(this, this.documentFullscreenChange_));
+      if (prefixedFS) {
+        this.handleFullscreenChange_({}, false);
+      } else {
+        this.on(FullscreenApi.fullscreenchange, this.handleFullscreenChange_);
+      }
+    }
   }
 
   /**
@@ -2568,24 +2612,16 @@ class Player extends Component {
       // the browser supports going fullscreen at the element level so we can
       // take the controls fullscreen as well as the video
 
+      if (!prefixedFS) {
+        this.off(FullscreenApi.fullscreenchange, this.handleFullscreenChange_);
+      }
+
       // Trigger fullscreenchange event after change
       // We have to specifically add this each time, and remove
       // when canceling fullscreen. Otherwise if there's multiple
       // players on a page, they would all be reacting to the same fullscreen
       // events
-      Events.on(document, fsApi.fullscreenchange, Fn.bind(this, function documentFullscreenChange(e) {
-        this.isFullscreen(document[fsApi.fullscreenElement]);
-
-        // If cancelling fullscreen, remove event listener.
-        if (this.isFullscreen() === false) {
-          Events.off(document, fsApi.fullscreenchange, documentFullscreenChange);
-        }
-        /**
-         * @event Player#fullscreenchange
-         * @type {EventTarget~Event}
-         */
-        this.trigger('fullscreenchange');
-      }));
+      Events.on(document, fsApi.fullscreenchange, Fn.bind(this, this.documentFullscreenChange_));
 
       this.el_[fsApi.requestFullscreen]();
 
@@ -2617,6 +2653,8 @@ class Player extends Component {
 
     // Check for browser element fullscreen support
     if (fsApi.requestFullscreen) {
+      // remove the document level handler if we're getting called directly.
+      Events.off(document, fsApi.fullscreenchange, Fn.bind(this, this.documentFullscreenChange_));
       document[fsApi.exitFullscreen]();
     } else if (this.tech_.supportsFullScreen()) {
       this.techCall_('exitFullScreen');

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@ const apiMap = [`
`64`	`64`
`65`	`65`	`const specApi = apiMap[0];`
`66`	`66`	`let browserApi;`
	`67`	`+let prefixedAPI = true;`
`67`	`68`
`68`	`69`	`// determine the supported set of functions`
`69`	`70`	`for (let i = 0; i < apiMap.length; i++) {`
`@@ -79,6 +80,9 @@ if (browserApi) {`
`79`	`80`	`for (let i = 0; i < browserApi.length; i++) {`
`80`	`81`	`FullscreenApi[specApi[i]] = browserApi[i];`
`81`	`82`	`}`
	`83`	`+`
	`84`	`+ prefixedAPI = browserApi[0] !== specApi[0];`
`82`	`85`	`}`
`83`	`86`
`84`	`87`	`export default FullscreenApi;`
	`88`	`+export { prefixedAPI };`