Add test

We've been printing the resulting value from a successful call to the ghrelease, this was useful for debugging but isn't necessary anymore.
Add the result to the error conditional with some headings.

CHANGELOG.md

@@ -1,3 +1,25 @@
+<a name="6.3.3"></a>
+## [6.3.3](https://github.com/videojs/video.js/compare/v6.3.2...v6.3.3) (2017-10-10)
+
+### Bug Fixes
+
+* a possible breaking change caused by the use of remainingTimeDisplay ([#4655](https://github.com/videojs/video.js/issues/4655)) ([b1de506](https://github.com/videojs/video.js/commit/b1de506))
+
+### Documentation
+
+* **hooks:** Fix Typo ([#4652](https://github.com/videojs/video.js/issues/4652)) ([6738f76](https://github.com/videojs/video.js/commit/6738f76))
+
+<a name="6.3.2"></a>
+## [6.3.2](https://github.com/videojs/video.js/compare/v6.3.1...v6.3.2) (2017-10-04)
+
+### Bug Fixes
+
+* Fix a typo in current time display component. ([#4647](https://github.com/videojs/video.js/issues/4647)) ([4658c7b](https://github.com/videojs/video.js/commit/4658c7b))
+
+### Documentation
+
+* Document how to add a version number to a plugin ([#4642](https://github.com/videojs/video.js/issues/4642)) ([85a34d1](https://github.com/videojs/video.js/commit/85a34d1))
+
 <a name="6.3.1"></a>
 ## [6.3.1](https://github.com/videojs/video.js/compare/v6.3.0...v6.3.1) (2017-10-03)
 

build/gh-release.js

@@ -35,10 +35,10 @@ if (args.prerelease || npmargs.some(function(arg) { return /next/.test(arg); }))
 
 ghrelease(options, function(err, result) {
   if (err) {
-    console.log('Unable to publish release to github');
-    console.log(err);
+    console.error('Unable to publish release to github');
+    console.error('err:', err);
+    console.error('result:', result);
   } else {
     console.log('Publish release to github!');
-    console.log(result);
   }
 });

docs/guides/hooks.md

@@ -14,7 +14,7 @@ Hooks exist so that users can "hook" on to certain Video.js player lifecycle
 
 ## Current Hooks
 
-Currently, the following hooks are avialable:
+Currently, the following hooks are available:
 
 ### beforesetup
 

docs/guides/plugins.md

@@ -225,6 +225,25 @@ The `dispose` method has several effects:
 
 In addition, if the player is disposed, the disposal of all its advanced plugin instances will be triggered as well.
 
+#### Version
+
+Adding a version number to a plugin is done by defining a `VERSION` property on the plugin before registering it:
+
+```js
+ExamplePlugin.VERSION = '1.0.1';
+
+videojs.registerPlugin('examplePlugin', ExamplePlugin);
+```
+
+Retrieve it using `videojs.getPluginVersion`:
+
+```js
+var version = videojs.getPluginVersion('examplePlugin');
+console.log(version);  // 1.0.1
+```
+
+Note that the [plugin generator](https://github.com/videojs/generator-videojs-plugin) already takes care of adding a version number for you.
+
 ### Advanced Example Advanced Plugin
 
 What follows is a complete ES6 advanced plugin that logs a custom message when the player's state changes between playing and paused. It uses all the described advanced features:

package.json

@@ -1,7 +1,7 @@
 {
   "name": "video.js",
   "description": "An HTML5 and Flash video player with a common API and skin for both.",
-  "version": "6.3.1",
+  "version": "6.3.3",
   "main": "./dist/video.cjs.js",
   "style": "./dist/video-js.css",
   "copyright": "Copyright Brightcove, Inc. <https://www.brightcove.com/>",

src/js/big-play-button.js

@@ -3,6 +3,7 @@
  */
 import Button from './button.js';
 import Component from './component.js';
+import {isPromise} from './utils/promise';
 
 /**
  * The initial play button that shows before the video has played. The hiding of the
@@ -58,10 +59,8 @@ class BigPlayButton extends Button {
 
     const playFocus = () => playToggle.focus();
 
-    if (playPromise && playPromise.then) {
-      const ignoreRejectedPlayPromise = () => {};
-
-      playPromise.then(playFocus, ignoreRejectedPlayPromise);
+    if (isPromise(playPromise)) {
+      playPromise.then(playFocus, () => {});
     } else {
       this.setTimeout(playFocus, 1);
     }

src/js/control-bar/play-toggle.js

@@ -67,7 +67,6 @@ class PlayToggle extends Button {
    * @listens Player#seeked
    */
   handleSeeked(event) {
-    // remove the ended class
     this.removeClass('vjs-ended');
 
     if (this.player_.paused()) {
@@ -86,6 +85,7 @@ class PlayToggle extends Button {
    * @listens Player#play
    */
   handlePlay(event) {
+    this.removeClass('vjs-ended');
     this.removeClass('vjs-paused');
     this.addClass('vjs-playing');
     // change the button text to "Pause"

src/js/control-bar/time-controls/current-time-display.js

@@ -64,7 +64,7 @@ class CurrentTimeDisplay extends TimeDisplay {
     if (!this.player_.duration()) {
       return;
     }
-    this.updateFormattedTime_(this.player.duration());
+    this.updateFormattedTime_(this.player_.duration());
   }
 
 }

src/js/control-bar/time-controls/remaining-time-display.js

@@ -64,7 +64,13 @@ class RemainingTimeDisplay extends TimeDisplay {
       return;
     }
 
-    this.updateFormattedTime_(this.player_.remainingTimeDisplay());
+    // @deprecated We should only use remainingTimeDisplay
+    // as of video.js 7
+    if (this.player_.remainingTimeDisplay) {
+      this.updateFormattedTime_(this.player_.remainingTimeDisplay());
+    } else {
+      this.updateFormattedTime_(this.player_.remainingTime());
+    }
   }
 
   /**

src/js/player.js

@@ -24,6 +24,7 @@ import MediaError from './media-error.js';
 import safeParseTuple from 'safe-json-parse/tuple';
 import {assign} from './utils/obj';
 import mergeOptions from './utils/merge-options.js';
+import {silencePromise} from './utils/promise';
 import textTrackConverter from './tracks/text-track-list-converter.js';
 import ModalDialog from './modal-dialog';
 import Tech from './tech/tech.js';
@@ -456,8 +457,6 @@ class Player extends Component {
 
     this.on('fullscreenchange', this.handleFullscreenChange_);
     this.on('stageclick', this.handleStageClick_);
-
-    this.changingSrc_ = false;
   }
 
   /**
@@ -1613,35 +1612,33 @@ class Player extends Component {
   }
 
   /**
-   * start media playback
+   * Begin playback.
    *
    * @return {Promise|undefined}
-   *         Returns a `Promise` if the browser returns one, for most browsers this will
-   *         return undefined.
+   *         Returns a `Promise` only if the browser returns one and the player
+   *         is ready to begin playback. For most browsers and non-ready
+   *         situations, this will return undefined.
    */
   play() {
-    if (this.changingSrc_) {
-      this.ready(function() {
-        const retval = this.techGet_('play');
 
-        // silence errors (unhandled promise from play)
-        if (retval !== undefined && typeof retval.then === 'function') {
-          retval.then(null, (e) => {});
-        }
+    // If the player is not ready, queue up a call to the tech's `play()`
+    // method for when it _is_ ready.
+    if (!this.isReady_) {
+      this.ready(() => {
+        silencePromise(this.techGet_('play'));
       });
 
-    // Only calls the tech's play if we already have a src loaded
-    } else if (this.isReady_ && (this.src() || this.currentSrc())) {
+    // If the player is ready and there is a source, call the tech's `play()`
+    // method.
+    } else if (this.src() || this.currentSrc()) {
       return this.techGet_('play');
-    } else {
-      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) => {});
-          }
+    // Finally, if the player is ready, but we don't have a source, wait for
+    // one to be set.
+    } else {
+      this.ready(() => {
+        this.tech_.one('loadstart', () => {
+          silencePromise(this.techGet_('play'));
         });
       });
     }
@@ -2274,58 +2271,77 @@ class Player extends Component {
    *         URL. Otherwise, returns nothing/undefined.
    */
   src(source) {
-    // getter usage
-    if (typeof source === 'undefined') {
+
+    // Getter usage.
+    if (source === undefined) {
       return this.cache_.src;
     }
-    // filter out invalid sources and turn our source into
-    // an array of source objects
+
+    // Queues "error 4" to be triggered on the player on the next tick. This
+    // is done on a delay to give users a chance to listen for "error" events.
+    const queueError = () => {
+      this.setTimeout(() => {
+        this.error({
+          code: 4,
+          message: this.localize(this.options_.notSupportedMessage)
+        });
+      }, 1);
+    };
+
+    // Filter out invalid sources and normalize the passed source into an array
+    // of source objects.
     const sources = filterSource(source);
 
-    // if a source was passed in then it is invalid because
-    // it was filtered to a zero length Array. So we have to
-    // show an error
+    // Show an error if there are no sources after filtering.
     if (!sources.length) {
-      this.setTimeout(function() {
-        this.error({ code: 4, message: this.localize(this.options_.notSupportedMessage) });
-      }, 0);
-      return;
+      return queueError();
     }
 
-    // intial sources
+    // Cache initial sources.
     this.cache_.sources = sources;
-    this.changingSrc_ = true;
-
-    // intial source
     this.cache_.source = sources[0];
 
-    // middlewareSource is the source after it has been changed by middleware
+    // Set the player to a non-ready state while middleware asynchronously
+    // determines the final source.
+    this.isReady_ = false;
+
+    // This asynchronously resolves any configured middleware. The callback is
+    // called when the process completes and given the `middlewareSource`,
+    // which is the final source we'll use to find a compatible tech.
     middleware.setSource(this, sources[0], (middlewareSource, mws) => {
+
+      // Cache middleware.
       this.middleware_ = mws;
 
-      const err = this.src_(middlewareSource);
+      // Attempt to set the source on a tech.
+      const foundTech = this.src_(middlewareSource);
 
-      if (err) {
+      // We could not find a compatible tech.
+      if (foundTech) {
+
+        // If no compatible tech was found and there are still sources we have
+        // not tried, start the process over (including middleware) with the
+        // next source in the sources array.
         if (sources.length > 1) {
           return this.src(sources.slice(1));
         }
 
-        // We need to wrap this in a timeout to give folks a chance to add error event handlers
-        this.setTimeout(function() {
-          this.error({ code: 4, message: this.localize(this.options_.notSupportedMessage) });
-        }, 0);
-
-        // we could not find an appropriate tech, but let's still notify the delegate that this is it
-        // this needs a better comment about why this is needed
+        // If there is no compatible tech, but there are no sources left to
+        // try, we queue up an error state on the player and trigger "ready"
+        // to inform any subscribers that the player is ready.
+        queueError();
         this.triggerReady();
-
         return;
       }
 
-      this.changingSrc_ = false;
-      // video element listed source
+      // At this point, we've found a compatible tech for the middleware source,
+      // so we can tell any subscribers that the player is ready again.
+      this.triggerReady();
+
+      // Cache the source URL.
       this.cache_.src = middlewareSource.src;
 
+      // Notify middlewares of a new tech.
       middleware.setTech(mws, this.tech_);
     });
   }
@@ -2346,37 +2362,34 @@ class Player extends Component {
   src_(source) {
     const sourceTech = this.selectSource([source]);
 
+    // There is no tech available that can play the chosen source.
     if (!sourceTech) {
       return true;
     }
 
+    // The tech that was found is not the tech that is currently loaded into
+    // the player; so, we need to unload the old tech, load a new one, and
+    // set the source on it.
     if (!titleCaseEquals(sourceTech.tech, this.techName_)) {
-      this.changingSrc_ = true;
-
-      // load this technology with the chosen source
       this.loadTech_(sourceTech.tech, sourceTech.source);
       return false;
     }
 
-    // wait until the tech is ready to set the source
-    this.ready(function() {
+    // The existing tech will work with this source; so, we can set the
+    // source on the existing tech.
+    //
+    // The `setSource` method was added with source handlers; so, older techs
+    // won't support it. Also, we need to check the direct prototype for
+    // the case where subclasses of `Tech` do not support source handlers.
+    if (this.tech_.constructor.prototype.hasOwnProperty('setSource')) {
+      this.techCall_('setSource', source);
+    } else {
+      this.techCall_('src', source.src);
+    }
 
-      // The setSource tech method was added with source handlers
-      // so older techs won't support it
-      // We need to check the direct prototype for the case where subclasses
-      // of the tech do not support source handlers
-      if (this.tech_.constructor.prototype.hasOwnProperty('setSource')) {
-        this.techCall_('setSource', source);
-      } else {
-        this.techCall_('src', source.src);
-      }
-
-      if (this.options_.preload === 'auto') {
-        this.load();
-      }
-
-    // Set the source synchronously if possible (#2326)
-    }, true);
+    if (this.options_.preload === 'auto') {
+      this.load();
+    }
 
     return false;
   }

src/js/utils/promise.js

@@ -0,0 +1,28 @@
+
+/**
+ * Returns whether an object is `Promise`-like (i.e. has a `then` method).
+ *
+ * @param  {Object}  value
+ *         An object that may or may not be `Promise`-like.
+ *
+ * @return {Boolean}
+ *         Whether or not the object is `Promise`-like.
+ */
+export function isPromise(value) {
+  return value !== undefined && typeof value.then === 'function';
+}
+
+/**
+ * Silence a Promise-like object.
+ *
+ * This is useful for avoiding non-harmful, but potentially confusing "uncaught
+ * play promise" rejection error messages.
+ *
+ * @param  {Object} value
+ *         An object that may or may not be `Promise`-like.
+ */
+export function silencePromise(value) {
+  if (isPromise(value)) {
+    value.then(null, (e) => {});
+  }
+}

test/unit/player.test.js

@@ -1601,18 +1601,84 @@ QUnit.test('src_ does not call loadTech is name is titleCaseEquals', function(as
         tech: 'html5'
       };
     },
+    options_: {},
+    tech_: {
+      constructor: {
+        prototype: {}
+      }
+    },
+    techCall_() {},
     techName_: 'Html5',
-    ready() {},
+    // ready() {},
+    load() {},
     loadTech_() {
       loadTechCalled++;
     }
   };
 
-  Player.prototype.src_.call(playerProxy);
+  Player.prototype.src_.call(playerProxy, {});
 
   assert.equal(loadTechCalled, 0, 'loadTech was not called');
 });
 
+QUnit.test('subsequent calls to src() will put the player in a non-ready state, so calling ready() immediately after will correctly wait until the new source is set', function(assert) {
+  const tag = TestHelpers.makeTag();
+  const player = videojs(tag);
+  const onReadySpy = sinon.spy();
+  const readySpy = sinon.spy();
+
+  player.on('ready', onReadySpy);
+  player.ready(readySpy);
+
+  assert.equal(onReadySpy.callCount, 0, 'no readiness yet...');
+  assert.equal(readySpy.callCount, 0, 'no readiness yet...');
+
+  this.clock.tick(1);
+
+  assert.equal(onReadySpy.callCount, 1, 'saw an initial "ready"');
+  assert.equal(readySpy.callCount, 1, 'saw an initial ready() callback');
+
+  player.src({
+    src: 'http://example.com/video.mp4',
+    type: 'video/mp4'
+  });
+
+  player.ready(readySpy);
+
+  assert.equal(onReadySpy.callCount, 1, 'did not see a "ready" because source setting is async');
+  assert.equal(readySpy.callCount, 1, 'did not see a ready() callback because source setting is async');
+
+  this.clock.tick(1);
+
+  assert.equal(onReadySpy.callCount, 1, 'did not see a "ready" because middleware queues up another async operation');
+  assert.equal(readySpy.callCount, 1, 'did not see a ready() callback because middleware queues up another async operation');
+
+  this.clock.tick(1);
+
+  assert.equal(onReadySpy.callCount, 2, 'saw second "ready" because source setting and tech selection are complete');
+  assert.equal(readySpy.callCount, 2, 'saw second ready() callback because source setting and tech selection are complete');
+
+  player.src({
+    src: 'http://example.com/video2.mp4',
+    type: 'video/mp4'
+  });
+
+  player.ready(readySpy);
+
+  assert.equal(onReadySpy.callCount, 2, 'did not see a "ready" because source setting is async');
+  assert.equal(readySpy.callCount, 2, 'did not see a ready() callback because source setting is async');
+
+  this.clock.tick(1);
+
+  assert.equal(onReadySpy.callCount, 2, 'did not see a "ready" because middleware queues up another async operation');
+  assert.equal(readySpy.callCount, 2, 'did not see a ready() callback because middleware queues up another async operation');
+
+  this.clock.tick(1);
+
+  assert.equal(onReadySpy.callCount, 3, 'saw third "ready" because source setting and tech selection are complete');
+  assert.equal(readySpy.callCount, 3, 'saw third ready() callback because source setting and tech selection are complete');
+});
+
 QUnit.test('options: plugins', function(assert) {
   const optionsSpy = sinon.spy();
 

test/unit/tracks/text-tracks.test.js

@@ -574,7 +574,7 @@ if (Html5.isSupported()) {
     player.addRemoteTextTrack(track, false);
     player.src({src: 'example.mp4', type: 'video/mp4'});
 
-    this.clock.tick(1);
+    this.clock.tick(2);
     assert.equal(player.textTracks().length, 0, 'we do not have any tracks left');
 
     player.dispose();