Fixed a number of errors in the log after testing

- Fixed a console error in dom tests from loading a track source
- Switched to non-fetching poster urls in tests to prevent errors
- Stubbed XHR for TextTrack tests to prevent log errors
- Fixed text track console errors that stubbing async didn't catch
    because there's some async happening in tracks that makes it so
- XHR isn't even used until the test is complete
- Removed extra code
- Added player.dispose more places and fixed attributes test

CHANGELOG.md

@@ -113,6 +113,7 @@ CHANGELOG
 * @dmlap @gkatsev improve Flash tech error property and add an error setter to the base tech ([view](https://github.com/videojs/video.js/pull/2517))
 * @dmlap update to videojs-swf 5.0.0-rc1 ([view](https://github.com/videojs/video.js/pull/2528))
 * @dmlap expose start and end buffered times ([view](https://github.com/videojs/video.js/pull/2501))
+* @heff fixed a number of console errors after testing ([view](https://github.com/videojs/video.js/pull/2513))
 
 --------------------
 

package.json

@@ -80,7 +80,7 @@
     "karma-sinon": "^1.0.3",
     "load-grunt-tasks": "^3.1.0",
     "qunitjs": "^1.18.0",
-    "sinon": "~1.9.1",
+    "sinon": "^1.16.1",
     "time-grunt": "^1.1.1",
     "uglify-js": "~2.3.6",
     "videojs-doc-generator": "0.0.1"

src/js/tech/tech.js

@@ -251,12 +251,14 @@ class Tech extends Component {
    */
   dispose() {
     // clear out text tracks because we can't reuse them between techs
-    let tt = this.textTracks();
-    let i = tt.length;
-    while(i--) {
-      this.removeRemoteTextTrack(tt[i]);
-    }
+    let textTracks = this.textTracks();
 
+    if (textTracks) {
+      let i = textTracks.length;
+      while(i--) {
+        this.removeRemoteTextTrack(textTracks[i]);
+      }
+    }
 
     // Turn off any manual progress or timeupdate tracking
     if (this.manualProgress) { this.manualProgressOff(); }

src/js/tracks/text-track.js

@@ -255,10 +255,9 @@ var parseCues = function(srcContent, track) {
 var loadTrack = function(src, track) {
   XHR(src, Fn.bind(this, function(err, response, responseBody){
     if (err) {
-      return log.error(err);
+      return log.error(err, response);
     }
 
-
     track.loaded_ = true;
     parseCues(responseBody, track);
   }));

src/js/xhr.js

@@ -80,7 +80,7 @@ var xhr = function(options, callback){
     window.clearTimeout(abortTimeout);
 
     if (!err || typeof err === 'string') {
-      err = new Error(err);
+      err = new Error(err || 'XHR Failed with a response of: '+(request && (request.response || request.responseText)));
     }
 
     callback(err, request);

test/unit/player.test.js

@@ -220,7 +220,7 @@ test('should wrap the original tag in the player div', function(){
 test('should set and update the poster value', function(){
   var tag, poster, updatedPoster, player;
 
-  poster = 'http://example.com/poster.jpg';
+  poster = '#';
   updatedPoster = 'http://example.com/updated-poster.jpg';
 
   tag = TestHelpers.makeTag();

test/unit/poster.test.js

@@ -7,8 +7,8 @@ q.module('PosterImage', {
   'setup': function(){
     // Store the original background support so we can test different vals
     this.origVal = browser.BACKGROUND_SIZE_SUPPORTED;
-    this.poster1 = 'http://example.com/poster.jpg';
-    this.poster2 = 'http://example.com/UPDATED.jpg';
+    this.poster1 = '#poster1';
+    this.poster2 = '#poster2';
 
     // Create a mock player object that responds as a player would
     this.mockPlayer = {
@@ -31,40 +31,33 @@ q.module('PosterImage', {
 });
 
 test('should create and update a poster image', function(){
-  var posterImage;
-
-  // IE11 adds quotes in the returned background url so need to normalize the result
-  function normalizeUrl(url){
-    return url.replace(new RegExp('\\"', 'g'),'');
-  }
-
   browser.BACKGROUND_SIZE_SUPPORTED = true;
-  posterImage = new PosterImage(this.mockPlayer);
-  equal(normalizeUrl(posterImage.el().style.backgroundImage), 'url('+this.poster1+')', 'Background image used');
+
+  let posterImage = new PosterImage(this.mockPlayer);
+  let backgroundImage = posterImage.el().style.backgroundImage;
+  notEqual(backgroundImage.indexOf(this.poster1), -1, 'Background image used');
 
   // Update with a new poster source and check the new value
   this.mockPlayer.poster_ = this.poster2;
   this.mockPlayer.trigger('posterchange');
-  equal(normalizeUrl(posterImage.el().style.backgroundImage), 'url('+this.poster2+')', 'Background image updated');
+  backgroundImage = posterImage.el().style.backgroundImage;
+  notEqual(backgroundImage.indexOf(this.poster2), -1, 'Background image updated');
 });
 
 test('should create and update a fallback image in older browsers', function(){
-  var posterImage;
-
   browser.BACKGROUND_SIZE_SUPPORTED = false;
-  posterImage = new PosterImage(this.mockPlayer);
-  equal(posterImage.fallbackImg_.src, this.poster1, 'Fallback image created');
+
+  let posterImage = new PosterImage(this.mockPlayer);
+  notEqual(posterImage.fallbackImg_.src.indexOf(this.poster1), -1, 'Fallback image created');
 
   // Update with a new poster source and check the new value
   this.mockPlayer.poster_ = this.poster2;
   this.mockPlayer.trigger('posterchange');
-  equal(posterImage.fallbackImg_.src, this.poster2, 'Fallback image updated');
+  notEqual(posterImage.fallbackImg_.src.indexOf(this.poster2), -1, 'Fallback image updated');
 });
 
 test('should remove itself from the document flow when there is no poster', function(){
-  var posterImage;
-
-  posterImage = new PosterImage(this.mockPlayer);
+  let posterImage = new PosterImage(this.mockPlayer);
   equal(posterImage.el().style.display, '', 'Poster image shows by default');
 
   // Update with an empty string

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

@@ -25,6 +25,8 @@ test('should be displayed when text tracks list is not empty', function() {
 
   ok(!player.controlBar.captionsButton.hasClass('vjs-hidden'), 'control is displayed');
   equal(player.textTracks().length, 1, 'textTracks contains one item');
+
+  player.dispose();
 });
 
 test('should be displayed when a text track is added to an empty track list', function() {
@@ -34,6 +36,8 @@ test('should be displayed when a text track is added to an empty track list', fu
 
   ok(!player.controlBar.captionsButton.hasClass('vjs-hidden'), 'control is displayed');
   equal(player.textTracks().length, 1, 'textTracks contains one item');
+
+  player.dispose();
 });
 
 test('should not be displayed when text tracks list is empty', function() {
@@ -41,6 +45,8 @@ test('should not be displayed when text tracks list is empty', function() {
 
   ok(player.controlBar.captionsButton.hasClass('vjs-hidden'), 'control is not displayed');
   equal(player.textTracks().length, 0, 'textTracks is empty');
+
+  player.dispose();
 });
 
 test('should not be displayed when last text track is removed', function() {
@@ -52,6 +58,8 @@ test('should not be displayed when last text track is removed', function() {
 
   ok(player.controlBar.captionsButton.hasClass('vjs-hidden'), 'control is not displayed');
   equal(player.textTracks().length, 0, 'textTracks is empty');
+
+  player.dispose();
 });
 
 test('menu should contain "Settings", "Off" and one track', function() {
@@ -68,6 +76,8 @@ test('menu should contain "Settings", "Off" and one track', function() {
   equal(menuItems[0].track.label, 'captions settings', 'menu contains "captions settings"');
   equal(menuItems[1].track.label, 'captions off', 'menu contains "captions off"');
   equal(menuItems[2].track.label, 'test', 'menu contains "test" track');
+
+  player.dispose();
 });
 
 test('menu should update with addRemoteTextTrack', function() {
@@ -81,6 +91,8 @@ test('menu should update with addRemoteTextTrack', function() {
 
   equal(player.controlBar.captionsButton.items.length, 4, 'menu does contain added track');
   equal(player.textTracks().length, 2, 'textTracks contains two items');
+
+  player.dispose();
 });
 
 test('menu should update with removeRemoteTextTrack', function() {
@@ -94,6 +106,8 @@ test('menu should update with removeRemoteTextTrack', function() {
 
   equal(player.controlBar.captionsButton.items.length, 3, 'menu does not contain removed track');
   equal(player.textTracks().length, 1, 'textTracks contains one item');
+
+  player.dispose();
 });
 
 if (!browser.IS_IE8) {
@@ -121,5 +135,7 @@ if (!browser.IS_IE8) {
 
     trackMenuItem.trigger('click');
     equal(changes, 2, 'clicks trigger change events');
+
+    player.dispose();
   });
 }

test/unit/tracks/text-track-list-converter.test.js

@@ -3,8 +3,16 @@ import TextTrack from '../../../src/js/tracks/text-track.js';
 import TextTrackList from '../../../src/js/tracks/text-track-list.js';
 import Html5 from '../../../src/js/tech/html5.js';
 import document from 'global/document';
+import window from 'global/window';
 
-q.module('Text Track List Converter');
+q.module('Text Track List Converter', {
+  setup: function() {
+    this.xhr = sinon.useFakeXMLHttpRequest();
+  },
+  teardown: function() {
+    this.xhr.restore();
+  }
+});
 
 let clean = (item) => {
   delete item.id;

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

@@ -47,6 +47,8 @@ test('should update settings', function() {
 
   Events.trigger(player.el().querySelector('.vjs-done-button'), 'click');
   deepEqual(safeParseTuple(window.localStorage.getItem('vjs-text-track-settings'))[1], newSettings, 'values are saved');
+
+  player.dispose();
 });
 
 test('should restore default settings', function() {
@@ -81,6 +83,8 @@ test('should restore default settings', function() {
   equal(player.el().querySelector('.vjs-edge-style select').selectedIndex, 0, 'edge-style is set to default value');
   equal(player.el().querySelector('.vjs-font-family select').selectedIndex, 0, 'font-family is set to default value');
   equal(player.el().querySelector('.vjs-font-percent select').selectedIndex, 2, 'font-percent is set to default value');
+
+  player.dispose();
 });
 
 test('should open on click', function() {
@@ -89,6 +93,8 @@ test('should open on click', function() {
   });
   Events.trigger(player.el().querySelector('.vjs-texttrack-settings'), 'click');
   ok(!player.textTrackSettings.hasClass('vjs-hidden'), 'settings open');
+
+  player.dispose();
 });
 
 test('should close on done click', function() {
@@ -98,6 +104,8 @@ test('should close on done click', function() {
   Events.trigger(player.el().querySelector('.vjs-texttrack-settings'), 'click');
   Events.trigger(player.el().querySelector('.vjs-done-button'), 'click');
   ok(player.textTrackSettings.hasClass('vjs-hidden'), 'settings closed');
+
+  player.dispose();
 });
 
 test('if persist option is set, restore settings on init', function() {
@@ -117,6 +125,8 @@ test('if persist option is set, restore settings on init', function() {
   equal(restore, 1, 'restore was called');
 
   TextTrackSettings.prototype.restoreSettings = oldRestoreSettings;
+
+  player.dispose();
 });
 
 test('if persist option is set, save settings when "done"', function() {
@@ -136,6 +146,8 @@ test('if persist option is set, save settings when "done"', function() {
   equal(save, 1, 'save was called');
 
   TextTrackSettings.prototype.saveSettings = oldSaveSettings;
+
+  player.dispose();
 });
 
 test('do not try to restore or save settings if persist option is not set', function() {
@@ -166,6 +178,8 @@ test('do not try to restore or save settings if persist option is not set', func
 
   TextTrackSettings.prototype.saveSettings = oldSaveSettings;
   TextTrackSettings.prototype.restoreSettings = oldRestoreSettings;
+
+  player.dispose();
 });
 
 test('should restore saved settings', function() {
@@ -190,6 +204,8 @@ test('should restore saved settings', function() {
   });
 
   deepEqual(player.textTrackSettings.getValues(), newSettings);
+
+  player.dispose();
 });
 
 test('should not restore saved settings', function() {
@@ -214,4 +230,6 @@ test('should not restore saved settings', function() {
   });
 
   deepEqual(player.textTrackSettings.getValues(), {});
+
+  player.dispose();
 });

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

@@ -256,4 +256,6 @@ test('fires cuechange when cues become active and inactive', function() {
   player.tech.trigger('timeupdate');
 
   equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange');
+
+  player.dispose();
 });

test/unit/tracks/tracks.test.js

@@ -11,9 +11,17 @@ import Component from '../../../src/js/component.js';
 import * as browser from '../../../src/js/utils/browser.js';
 import TestHelpers from '../test-helpers.js';
 import document from 'global/document';
+import window from 'global/window';
 import TechFaker from '../tech/tech-faker.js';
 
-q.module('Tracks');
+q.module('Tracks', {
+  setup: function() {
+    this.xhr = sinon.useFakeXMLHttpRequest();
+  },
+  teardown: function() {
+    this.xhr.restore();
+  }
+});
 
 test('should place title list item into ul', function() {
   var player, chaptersButton;
@@ -47,6 +55,8 @@ test('Player track methods call the tech', function() {
   player.textTracks();
 
   equal(calls, 2, 'both textTrack and addTextTrack defer to the tech');
+
+  player.dispose();
 });
 
 test('TextTrackDisplay initializes tracks on player ready', function() {
@@ -154,7 +164,9 @@ test('update texttrack buttons on removetrack or addtrack', function() {
           events[type] = [];
         }
         events[type].push(handler);
-      }
+      },
+      // Requrired in player.dispose()
+      removeEventListener: function(){}
     };
   };
 
@@ -163,13 +175,13 @@ test('update texttrack buttons on removetrack or addtrack', function() {
   track.kind = 'captions';
   track.label = 'en';
   track.language = 'English';
-  track.src = 'en.vtt';
+  track.src = '#en.vtt';
   tag.appendChild(track);
   track = document.createElement('track');
   track.kind = 'captions';
   track.label = 'es';
   track.language = 'Spanish';
-  track.src = 'es.vtt';
+  track.src = '#es.vtt';
   tag.appendChild(track);
 
   player =  TestHelpers.makePlayer({}, tag);
@@ -195,6 +207,8 @@ test('update texttrack buttons on removetrack or addtrack', function() {
   CaptionsButton.prototype.update = oldCaptionsUpdate;
   SubtitlesButton.prototype.update = oldSubsUpdate;
   ChaptersButton.prototype.update = oldChaptersUpdate;
+
+  player.dispose();
 });
 
 test('if native text tracks are not supported, create a texttrackdisplay', function() {
@@ -238,25 +252,8 @@ test('if native text tracks are not supported, create a texttrackdisplay', funct
   Html5.TEST_VID = oldTestVid;
   browser.IS_FIREFOX = oldIsFirefox;
   Component.registerComponent('TextTrackDisplay', oldTextTrackDisplay);
-});
 
-test('Player track methods call the tech', function() {
-  var player,
-      calls = 0;
-
-  player = TestHelpers.makePlayer();
-
-  player.tech.textTracks = function() {
-    calls++;
-  };
-  player.tech.addTextTrack = function() {
-    calls++;
-  };
-
-  player.addTextTrack();
-  player.textTracks();
-
-  equal(calls, 2, 'both textTrack and addTextTrack defer to the tech');
+  player.dispose();
 });
 
 test('html5 tech supports native text tracks if the video supports it, unless mode is a number', function() {

test/unit/utils/dom.test.js

@@ -90,25 +90,25 @@ test('should set element attributes from object', function(){
 });
 
 test('should read tag attributes from elements, including HTML5 in all browsers', function(){
-  var tags = '<video id="vid1" controls autoplay loop muted preload="none" src="http://google.com" poster="http://www2.videojs.com/img/video-js-html5-video-player.png" data-test="asdf" data-empty-string=""></video>';
-  tags += '<video id="vid2">';
-  // Not putting source and track inside video element because
-  // oldIE needs the HTML5 shim to read tags inside HTML5 tags.
-  // Still may not work in oldIE.
-  tags += '<source id="source" src="http://google.com" type="video/mp4" media="fdsa" title="test" >';
-  tags += '<track id="track" default src="http://google.com" kind="captions" srclang="en" label="testlabel" title="test" >';
-  tags += '</video>';
+  // Creating the source/track tags outside of the video tag prevents log errors
+  let tags = `
+  <video id="vid1" controls autoplay loop muted preload="none" src="http://google.com" poster="http://www2.videojs.com/img/video-js-html5-video-player.png" data-test="asdf" data-empty-string=""></video>
+  <source id="source" src="http://google.com" type="video/mp4" media="fdsa" title="test" >
+  <track id="track" default src="http://google.com" kind="captions" srclang="en" label="testlabel" title="test" >
+  `;
 
-  document.getElementById('qunit-fixture').innerHTML += tags;
+  let fixture = document.getElementById('qunit-fixture');
 
-  var vid1Vals = Dom.getElAttributes(document.getElementById('vid1'));
-  var vid2Vals = Dom.getElAttributes(document.getElementById('vid2'));
-  var sourceVals = Dom.getElAttributes(document.getElementById('source'));
-  var trackVals = Dom.getElAttributes(document.getElementById('track'));
+  // Have to use innerHTML to append for IE8. AppendChild doesn't work.
+  // Also it must be added to the page body, not just in memory.
+  fixture.innerHTML += tags;
 
-  // was using deepEqual, but ie8 would send all properties as attributes
+  let vid1Vals = Dom.getElAttributes(fixture.getElementsByTagName('video')[0]);
+  let sourceVals = Dom.getElAttributes(fixture.getElementsByTagName('source')[0]);
+  let trackVals = Dom.getElAttributes(fixture.getElementsByTagName('track')[0]);
 
   // vid1
+  // was using deepEqual, but ie8 would send all properties as attributes
   equal(vid1Vals['autoplay'], true);
   equal(vid1Vals['controls'], true);
   equal(vid1Vals['data-test'], 'asdf');
@@ -120,9 +120,6 @@ test('should read tag attributes from elements, including HTML5 in all browsers'
   equal(vid1Vals['preload'], 'none');
   equal(vid1Vals['src'], 'http://google.com');
 
-  // vid2
-  equal(vid2Vals['id'], 'vid2');
-
   // sourceVals
   equal(sourceVals['title'], 'test');
   equal(sourceVals['media'], 'fdsa');

test/unit/video.test.js

@@ -62,9 +62,7 @@ test('should expose plugin registry function', function() {
   var pluginName, pluginFunction, player;
 
   pluginName = 'foo';
-  pluginFunction = function(options) {
-    console.log(this);
-  };
+  pluginFunction = function(options) {};
 
   ok(videojs.plugin, 'should exist');