fix: properly return promise from requestFullscreen and exitFullscreen (#7299)

Mapping the promise returned from the helpers to the executor's resolve, and reject
methods. We also need to catch the error in the promise chain that's
created inside exitFullscreenHelper_.

Fixes #7298.

Author: gkatsev

package-lock.json

@@ -139,11 +139,11 @@
     "remark-validate-links": "^8.0.2",
     "replace": "^1.2.1",
     "rollup": "^2.2.0",
-    "rollup-plugin-istanbul": "^3.0.0",
     "rollup-plugin-alias": "^1.5.2",
     "rollup-plugin-babel": "^4.4.0",
     "rollup-plugin-commonjs": "^9.3.4",
     "rollup-plugin-ignore": "^1.0.5",
+    "rollup-plugin-istanbul": "^3.0.0",
     "rollup-plugin-json": "^3.1.0",
     "rollup-plugin-multi-entry": "^2.0.2",
     "rollup-plugin-node-resolve": "^4.2.4",
@@ -153,7 +153,7 @@
     "semver": "^5.7.0",
     "shelljs": "^0.8.3",
     "shx": "^0.3.2",
-    "sinon": "^7.3.2",
+    "sinon": "^11.1.1",
     "tui-jsdoc-template": "^1.2.2",
     "uglify-js": "^3.6.0",
     "unified": "^7.0.2",

package.json

@@ -2850,7 +2850,7 @@ class Player extends Component {
 
         if (promise) {
           promise.then(offHandler, offHandler);
-          return promise;
+          promise.then(resolve, reject);
         }
       });
     }
@@ -2928,7 +2928,8 @@ class Player extends Component {
 
         if (promise) {
           promise.then(offHandler, offHandler);
-          return promise;
+          // map the promise to our resolve/reject methods
+          promise.then(resolve, reject);
         }
       });
     }
@@ -2941,7 +2942,9 @@ class Player extends Component {
       const promise = document[this.fsApi_.exitFullscreen]();
 
       if (promise) {
-        promise.then(() => this.isFullscreen(false));
+        // we're splitting the promise here, so, we want to catch the
+        // potential error so that this chain doesn't have unhandled errors
+        silencePromise(promise.then(() => this.isFullscreen(false)));
       }
 
       return promise;

src/js/player.js

@@ -39,7 +39,9 @@ module.exports = function(config) {
   config.files = [
     'node_modules/es5-shim/es5-shim.js',
     'node_modules/es6-shim/es6-shim.js',
-    'node_modules/sinon/pkg/sinon.js',
+    // make sinon be available via karma's server but don't include it directly
+    { pattern: 'node_modules/sinon/pkg/sinon.js', included: false, served: true },
+    'test/sinon.js',
     'dist/video-js.css',
     'test/dist/bundle.js',
     'test/dist/browserify.js',

test/karma.conf.js

@@ -0,0 +1,12 @@
+/* eslint-disable no-undef */
+
+const s = document.createElement('script');
+
+// on IE11 and Safari 9, load the last supported sinon version
+if (/(?:MSIE|Trident\/7.0|Version\/9.*Safari)/.test(navigator.userAgent)) {
+  s.src = 'https://unpkg.com/sinon@9.2.4/pkg/sinon-no-sourcemaps.js';
+} else {
+  s.src = '/test/base/node_modules/sinon/pkg/sinon.js';
+}
+
+document.write(s.outerHTML);

test/sinon.js

@@ -3,6 +3,7 @@ import Player from '../../src/js/player.js';
 import TestHelpers from './test-helpers.js';
 import sinon from 'sinon';
 import window from 'global/window';
+import document from 'global/document';
 
 const FullscreenTestHelpers = {
   makePlayer(prefixed, playerOptions, videoTag) {
@@ -229,7 +230,7 @@ QUnit.test('full window can be preferred to fullscreen tech', function(assert) {
 });
 
 QUnit.test('fullwindow mode should exit when ESC event triggered', function(assert) {
-  const player = FullscreenTestHelpers.makePlayer(true);
+  const player = TestHelpers.makePlayer();
 
   player.enterFullWindow();
   assert.ok(player.isFullWindow, 'enterFullWindow should be called');
@@ -280,3 +281,70 @@ QUnit.test('fullscreenerror event from Html5 should pass through player', functi
 
   player.dispose();
 });
+
+// only run where we have sinon.promise
+const skipOrTest = sinon.promise ? 'test' : 'skip';
+
+QUnit[skipOrTest]('requestFullscreen returns a rejected promise if unable to go fullscreen', function(assert) {
+  const player = TestHelpers.makePlayer();
+  const playerEl = player.el();
+  const stub = sinon.stub(playerEl, player.fsApi_.requestFullscreen);
+  const promise = sinon.promise();
+
+  stub.returns(promise);
+  promise.reject(new Error('Cannot go fullscreen'));
+
+  assert.rejects(
+    player.requestFullscreen(),
+    new Error('Cannot go fullscreen'),
+    'our promise was rejected'
+  );
+
+  stub.restore();
+});
+
+QUnit[skipOrTest]('requestFullscreen returns a resolved promise if we were fullscreen', function(assert) {
+  const player = TestHelpers.makePlayer();
+  const playerEl = player.el();
+  const stub = sinon.stub(playerEl, player.fsApi_.requestFullscreen);
+  const promise = sinon.promise();
+
+  stub.returns(promise);
+  // pretend we successfully went fullscreen.
+  promise.resolve();
+
+  player.requestFullscreen().then(() => assert.ok(true, 'our promise resolved'));
+
+  stub.restore();
+});
+
+QUnit[skipOrTest]('exitFullscreen returns a rejected promise if document is not active', function(assert) {
+  const player = TestHelpers.makePlayer();
+  const stub = sinon.stub(document, player.fsApi_.exitFullscreen);
+  const promise = sinon.promise();
+
+  stub.returns(promise);
+  promise.reject(new Error('Document not active'));
+
+  assert.rejects(
+    player.exitFullscreen(),
+    new Error('Document not active'),
+    'our promise was rejected'
+  );
+
+  stub.restore();
+});
+
+QUnit[skipOrTest]('exitFullscreen returns a resolved promise if we were fullscreen', function(assert) {
+  const player = TestHelpers.makePlayer();
+  const stub = sinon.stub(document, player.fsApi_.exitFullscreen);
+  const promise = sinon.promise();
+
+  stub.returns(promise);
+  // pretend we successfully exited.
+  promise.resolve();
+
+  player.exitFullscreen().then(() => assert.ok(true, 'our promise resolved'));
+
+  stub.restore();
+});