From 091bdf9261549e49d453f4591cde9c6d555bfa00 Mon Sep 17 00:00:00 2001
From: Brandon Casey <BrandonOCasey@users.noreply.github.com>
Date: Wed, 18 Jan 2017 00:53:11 -0500
Subject: [PATCH] feat: Return the native Promise from play() (#3907)

Return the native Promise from `play()` if it exists. `undefined` is returned otherwise.
This comes in as part of the greater effort to remove method chaining.

BREAKING CHANGE: `play()` no longer returns the player object but instead the native Promise or nothing.
---
 src/js/player.js         | 22 ++++++++++++++--------
 src/js/tech/html5.js     | 24 ++++++++++--------------
 test/unit/player.test.js | 28 ++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/src/js/player.js b/src/js/player.js
index 0803f0aed..c2df80ce2 100644
--- a/src/js/player.js
+++ b/src/js/player.js
@@ -1564,20 +1564,26 @@ class Player extends Component {
   /**
    * start media playback
    *
-   * @return {Player}
-   *         A reference to the player object this function was called on
+   * @return {Promise|undefined}
+   *         Returns a `Promise` if the browser returns one, for most browsers this will
+   *         return undefined.
    */
   play() {
     // Only calls the tech's play if we already have a src loaded
     if (this.src() || this.currentSrc()) {
-      this.techCall_('play');
-    } else {
-      this.tech_.one('loadstart', function() {
-        this.play();
-      });
+      return this.techGet_('play');
     }
 
-    return this;
+    this.ready(function() {
+      this.tech_.one('loadstart', function() {
+        const retval = this.play();
+
+        // silence errors (unhandled promise from play)
+        if (retval !== undefined && typeof retval.then === 'function') {
+          retval.then(null, (e) => {});
+        }
+      });
+    });
   }
 
   /**
diff --git a/src/js/tech/html5.js b/src/js/tech/html5.js
index 8fc4d37c9..05ba9a715 100644
--- a/src/js/tech/html5.js
+++ b/src/js/tech/html5.js
@@ -455,19 +455,6 @@ class Html5 extends Tech {
     this.removeOldTracks_(techTracks, elTracks);
   }
 
-  /**
-   * Called by {@link Player#play} to play using the `Html5` `Tech`.
-   */
-  play() {
-    const playPromise = this.el_.play();
-
-    // Catch/silence error when a pause interrupts a play request
-    // on browsers which return a promise
-    if (playPromise !== undefined && typeof playPromise.then === 'function') {
-      playPromise.then(null, (e) => {});
-    }
-  }
-
   /**
    * Set current time for the `HTML5` tech.
    *
@@ -1570,7 +1557,16 @@ Html5.resetMediaElement = function(el) {
    * @method Html5#load
    * @see [Spec]{@link https://www.w3.org/TR/html5/embedded-content-0.html#dom-media-load}
    */
-  'load'
+  'load',
+
+  /**
+   * A wrapper around the media elements `play` function. This will call the `HTML5`s
+   * media element `play` function.
+   *
+   * @method Html5#play
+   * @see [Spec]{@link https://www.w3.org/TR/html5/embedded-content-0.html#dom-media-play}
+   */
+  'play'
 ].forEach(function(prop) {
   Html5.prototype[prop] = function() {
     return this.el_[prop]();
diff --git a/test/unit/player.test.js b/test/unit/player.test.js
index 65ad9a5df..2df1eed2d 100644
--- a/test/unit/player.test.js
+++ b/test/unit/player.test.js
@@ -1066,6 +1066,34 @@ QUnit.test('should be scrubbing while seeking', function(assert) {
   player.dispose();
 });
 
+if (window.Promise) {
+  QUnit.test('play promise should resolve to native promise if returned', function(assert) {
+    const player = TestHelpers.makePlayer({});
+    const done = assert.async();
+
+    player.tech_.play = () => window.Promise.resolve('foo');
+    const p = player.play();
+
+    assert.ok(p, 'play returns something');
+    assert.equal(typeof p.then, 'function', 'play returns a promise');
+    p.then(function(val) {
+      assert.equal(val, 'foo', 'should resolve to native promise value');
+
+      player.dispose();
+      done();
+    });
+  });
+}
+
+QUnit.test('play promise should resolve to native value if returned', function(assert) {
+  const player = TestHelpers.makePlayer({});
+
+  player.tech_.play = () => 'foo';
+  const p = player.play();
+
+  assert.equal(p, 'foo', 'play returns foo');
+});
+
 QUnit.test('should throw on startup no techs are specified', function(assert) {
   const techOrder = videojs.options.techOrder;