feat: remove handleManifestRedirects and always use XHR.responseURL i…

…f available (#1226)

BREAKING CHANGE: remove handleManifestRedirects option. Now XHR.responseURL will always be used when available.

README.md

@@ -39,7 +39,6 @@ Video.js Compatibility: 6.0, 7.0
       - [Source](#source)
     - [List](#list)
       - [withCredentials](#withcredentials)
-      - [handleManifestRedirects](#handlemanifestredirects)
       - [useCueTags](#usecuetags)
       - [parse708captions](#parse708captions)
       - [overrideNative](#overridenative)
@@ -282,16 +281,6 @@ is set to `true`.
 See html5rocks's [article](http://www.html5rocks.com/en/tutorials/cors/)
 for more info.
 
-##### handleManifestRedirects
-* Type: `boolean`
-* Default: `false`
-* can be used as a source option
-* can be used as an initialization option
-
-When the `handleManifestRedirects` property is set to `true`, manifest requests
-which are redirected will have their URL updated to the new URL for future
-requests.
-
 ##### useCueTags
 * Type: `boolean`
 * can be used as an initialization option

src/dash-playlist-loader.js

@@ -260,11 +260,10 @@ export default class DashPlaylistLoader extends EventTarget {
       this.isMaster_ = true;
     }
 
-    const { withCredentials = false, handleManifestRedirects = false } = options;
+    const { withCredentials = false } = options;
 
     this.vhs_ = vhs;
     this.withCredentials = withCredentials;
-    this.handleManifestRedirects = handleManifestRedirects;
 
     if (!srcUrlOrPlaylist) {
       throw new Error('A non-empty playlist URL or object is required');
@@ -339,7 +338,7 @@ export default class DashPlaylistLoader extends EventTarget {
     }
 
     // resolve the segment URL relative to the playlist
-    const uri = resolveManifestRedirect(this.handleManifestRedirects, playlist.sidx.resolvedUri);
+    const uri = resolveManifestRedirect(playlist.sidx.resolvedUri);
 
     const fin = (err, request) => {
       if (this.requestErrored_(err, request, startingState)) {
@@ -607,7 +606,7 @@ export default class DashPlaylistLoader extends EventTarget {
         this.masterLoaded_ = Date.now();
       }
 
-      this.masterPlaylistLoader_.srcUrl = resolveManifestRedirect(this.handleManifestRedirects, this.masterPlaylistLoader_.srcUrl, req);
+      this.masterPlaylistLoader_.srcUrl = resolveManifestRedirect(this.masterPlaylistLoader_.srcUrl, req);
 
       if (masterChanged) {
         this.handleMaster_();

src/master-playlist-controller.js

@@ -150,7 +150,6 @@ export class MasterPlaylistController extends videojs.EventTarget {
 
     const {
       src,
-      handleManifestRedirects,
       withCredentials,
       tech,
       bandwidth,
@@ -198,7 +197,6 @@ export class MasterPlaylistController extends videojs.EventTarget {
 
     this.requestOptions_ = {
       withCredentials,
-      handleManifestRedirects,
       maxPlaylistRetries,
       timeout: null
     };

src/playlist-loader.js

@@ -385,12 +385,11 @@ export default class PlaylistLoader extends EventTarget {
     }
     this.logger_ = logger('PlaylistLoader');
 
-    const { withCredentials = false, handleManifestRedirects = false } = options;
+    const { withCredentials = false} = options;
 
     this.src = src;
     this.vhs_ = vhs;
     this.withCredentials = withCredentials;
-    this.handleManifestRedirects = handleManifestRedirects;
 
     const vhsOptions = vhs.options_;
 
@@ -678,7 +677,7 @@ export default class PlaylistLoader extends EventTarget {
 
       playlist.lastRequest = Date.now();
 
-      playlist.resolvedUri = resolveManifestRedirect(this.handleManifestRedirects, playlist.resolvedUri, req);
+      playlist.resolvedUri = resolveManifestRedirect(playlist.resolvedUri, req);
 
       if (error) {
         return this.playlistRequestError(this.request, playlist, startingState);
@@ -839,7 +838,7 @@ export default class PlaylistLoader extends EventTarget {
         return this.trigger('error');
       }
 
-      this.src = resolveManifestRedirect(this.handleManifestRedirects, this.src, req);
+      this.src = resolveManifestRedirect(this.src, req);
 
       const manifest = this.parseManifest_({
         manifestString: req.responseText,

src/resolve-url.js

@@ -7,8 +7,8 @@ import _resolveUrl from '@videojs/vhs-utils/es/resolve-url.js';
 export const resolveUrl = _resolveUrl;
 
 /**
- * Checks whether xhr request was redirected and returns correct url depending
- * on `handleManifestRedirects` option
+ * If the xhr request was redirected, return the responseURL, otherwise,
+ * return the original url.
  *
  * @api private
  *
@@ -17,12 +17,11 @@ export const resolveUrl = _resolveUrl;
  *
  * @return {string}
  */
-export const resolveManifestRedirect = (handleManifestRedirect, url, req) => {
+export const resolveManifestRedirect = (url, req) => {
   // To understand how the responseURL below is set and generated:
   // - https://fetch.spec.whatwg.org/#concept-response-url
   // - https://fetch.spec.whatwg.org/#atomic-http-redirect-handling
   if (
-    handleManifestRedirect &&
     req &&
     req.responseURL &&
     url !== req.responseURL

src/videojs-http-streaming.js

@@ -622,7 +622,6 @@ class VhsHandler extends Component {
   setOptions_() {
     // defaults
     this.options_.withCredentials = this.options_.withCredentials || false;
-    this.options_.handleManifestRedirects = this.options_.handleManifestRedirects === false ? false : true;
     this.options_.limitRenditionByPlayerDimensions = this.options_.limitRenditionByPlayerDimensions === false ? false : true;
     this.options_.useDevicePixelRatio = this.options_.useDevicePixelRatio || false;
     this.options_.smoothQualityChange = this.options_.smoothQualityChange || false;
@@ -676,7 +675,6 @@ class VhsHandler extends Component {
       'smoothQualityChange',
       'customTagParsers',
       'customTagMappers',
-      'handleManifestRedirects',
       'cacheEncryptionKeys',
       'playlistSelector',
       'initialPlaylistSelector',

test/dash-playlist-loader.test.js

@@ -1,5 +1,6 @@
 import QUnit from 'qunit';
 import sinon from 'sinon';
+import window from 'global/window';
 import {
   default as DashPlaylistLoader,
   updateMaster,
@@ -2066,8 +2067,8 @@ QUnit.test('requests the manifest immediately when given a URL', function(assert
   assert.equal(this.requests[0].url, 'dash.mpd', 'requested the manifest');
 });
 
-QUnit.test('redirect manifest request when handleManifestRedirects is true', function(assert) {
-  const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs, { handleManifestRedirects: true });
+QUnit.test('redirect manifest request', function(assert) {
+  const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs, {});
 
   loader.load();
 
@@ -2080,8 +2081,8 @@ QUnit.test('redirect manifest request when handleManifestRedirects is true', fun
   assert.equal(loader.srcUrl, 'http://differenturi.com/test.mpd', 'url has redirected');
 });
 
-QUnit.test('redirect src request when handleManifestRedirects is true', function(assert) {
-  const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs, { handleManifestRedirects: true });
+QUnit.test('redirect src request', function(assert) {
+  const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs, {});
 
   loader.load();
 
@@ -2098,20 +2099,6 @@ QUnit.test('redirect src request when handleManifestRedirects is true', function
   assert.equal(childLoader.media_.resolvedUri, 'http://differenturi.com/placeholder-uri-0', 'url has redirected');
 });
 
-QUnit.test('do not redirect src request when handleManifestRedirects is not set', function(assert) {
-  const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs);
-
-  loader.load();
-
-  const modifiedRequest = this.requests.shift();
-
-  modifiedRequest.responseURL = 'http://differenturi.com/test.mpd';
-
-  this.standardXHRResponse(modifiedRequest);
-
-  assert.equal(loader.srcUrl, 'dash.mpd', 'url has not redirected');
-});
-
 QUnit.test('starts without any metadata', function(assert) {
   const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs);
 
@@ -2497,7 +2484,7 @@ QUnit.test('refreshes the xml if there is a minimumUpdatePeriod', function(asser
   this.clock.tick(4 * 1000);
 
   assert.equal(this.requests.length, 1, 'refreshed manifest');
-  assert.equal(this.requests[0].uri, 'dash-live.mpd', 'refreshed manifest');
+  assert.equal(this.requests[0].uri, window.location.href.split('/').slice(0, -1).join('/') + '/dash-live.mpd', 'refreshed manifest');
   assert.equal(minimumUpdatePeriods, 1, 'refreshed manifest');
 });
 
@@ -2517,7 +2504,7 @@ QUnit.test('stop xml refresh if minimumUpdatePeriod is removed', function(assert
   // First Refresh Tick: MPD loaded
   this.clock.tick(4 * 1000);
   assert.equal(this.requests.length, 1, 'refreshed manifest');
-  assert.equal(this.requests[0].uri, 'dash-live.mpd', 'refreshed manifest');
+  assert.equal(this.requests[0].uri, window.location.href.split('/').slice(0, -1).join('/') + '/dash-live.mpd', 'refreshed manifest');
   assert.equal(minimumUpdatePeriods, 1, 'total minimumUpdatePeriods');
 
   this.standardXHRResponse(this.requests[0], loader.masterXml_.replace('minimumUpdatePeriod="PT4S"', ''));
@@ -2544,7 +2531,7 @@ QUnit.test('continue xml refresh every targetDuration if minimumUpdatePeriod is
   // First Refresh Tick
   this.clock.tick(4 * 1000);
   assert.equal(this.requests.length, 1, 'refreshed manifest');
-  assert.equal(this.requests[0].uri, 'dash-live.mpd', 'refreshed manifest');
+  assert.equal(this.requests[0].uri, window.location.href.split('/').slice(0, -1).join('/') + '/dash-live.mpd', 'refreshed manifest');
   assert.equal(minimumUpdatePeriods, 1, 'total minimumUpdatePeriods');
 
   this.standardXHRResponse(this.requests[0], loader.masterXml_.replace('minimumUpdatePeriod="PT4S"', 'minimumUpdatePeriod="PT0S"'));

test/master-playlist-controller.test.js

@@ -329,17 +329,14 @@ QUnit.test('passes options to PlaylistLoader', function(assert) {
   let controller = new MasterPlaylistController(options);
 
   assert.notOk(controller.masterPlaylistLoader_.withCredentials, 'credentials wont be sent by default');
-  assert.notOk(controller.masterPlaylistLoader_.handleManifestRedirects, 'redirects are ignored by default');
 
   controller.dispose();
 
   controller = new MasterPlaylistController(Object.assign({
-    withCredentials: true,
-    handleManifestRedirects: true
+    withCredentials: true
   }, options));
 
   assert.ok(controller.masterPlaylistLoader_.withCredentials, 'withCredentials enabled');
-  assert.ok(controller.masterPlaylistLoader_.handleManifestRedirects, 'handleManifestRedirects enabled');
   controller.dispose();
 });
 

test/playlist-loader.test.js

@@ -1136,9 +1136,7 @@ QUnit.module('Playlist Loader', function(hooks) {
   });
 
   QUnit.test('recognizes redirect, when media requested', function(assert) {
-    const loader = new PlaylistLoader('manifest/media.m3u8', this.fakeVhs, {
-      handleManifestRedirects: true
-    });
+    const loader = new PlaylistLoader('manifest/media.m3u8', this.fakeVhs, {});
 
     loader.load();
 
@@ -2710,7 +2708,14 @@ QUnit.module('Playlist Loader', function(hooks) {
     });
 
     QUnit.test('works with existing query directives', function(assert) {
-      this.loader.src += '?foo=test';
+      // clear existing requests
+      this.requests.length = 0;
+
+      this.loader.dispose();
+      this.loader = new PlaylistLoader('http://example.com/media.m3u8?foo=test', this.fakeVhs);
+
+      this.loader.load();
+
       this.requests.shift().respond(
         200, null,
         '#EXTM3U\n' +

test/videojs-http-streaming.test.js

@@ -2823,94 +2823,6 @@ QUnit.test(
   }
 );
 
-QUnit.test('if handleManifestRedirects global option is used, it should be passed to PlaylistLoader', function(assert) {
-  const vhsOptions = videojs.options.vhs;
-
-  this.player.dispose();
-  videojs.options.vhs = {
-    handleManifestRedirects: true
-  };
-  this.player = createPlayer();
-  this.player.src({
-    src: 'http://example.com/media.m3u8',
-    type: 'application/vnd.apple.mpegurl'
-  });
-
-  this.clock.tick(1);
-
-  assert.ok(
-    this.player.tech_.vhs.masterPlaylistController_.masterPlaylistLoader_.handleManifestRedirects,
-    'handleManifestRedirects is set correctly'
-  );
-
-  videojs.options.vhs = vhsOptions;
-});
-
-QUnit.test('the handleManifestRedirects source option overrides the global default', function(assert) {
-  const vhsOptions = videojs.options.vhs;
-
-  this.player.dispose();
-  videojs.options.vhs = {
-    handleManifestRedirects: true
-  };
-  this.player = createPlayer();
-  this.player.src({
-    src: 'http://example.com/media.m3u8',
-    type: 'application/vnd.apple.mpegurl',
-    handleManifestRedirects: false
-  });
-
-  this.clock.tick(1);
-
-  assert.notOk(
-    this.player.tech_.vhs.masterPlaylistController_.masterPlaylistLoader_.handleManifestRedirects,
-    'handleManifestRedirects is set correctly'
-  );
-
-  videojs.options.vhs = vhsOptions;
-});
-
-QUnit.test('if handleManifestRedirects global option is used, it should be passed to DashPlaylistLoader', function(assert) {
-  const vhsOptions = videojs.options.vhs;
-
-  this.player.dispose();
-  videojs.options.vhs = {
-    handleManifestRedirects: true
-  };
-  this.player = createPlayer();
-  this.player.src({
-    src: 'http://example.com/media.mpd',
-    type: 'application/dash+xml'
-  });
-
-  this.clock.tick(1);
-
-  assert.ok(this.player.tech_.vhs.masterPlaylistController_.masterPlaylistLoader_.handleManifestRedirects);
-
-  videojs.options.vhs = vhsOptions;
-});
-
-QUnit.test('the handleManifestRedirects in DashPlaylistLoader option overrides the global default', function(assert) {
-  const vhsOptions = videojs.options.vhs;
-
-  this.player.dispose();
-  videojs.options.vhs = {
-    handleManifestRedirects: true
-  };
-  this.player = createPlayer();
-  this.player.src({
-    src: 'http://example.com/media.mpd',
-    type: 'application/dash+xml',
-    handleManifestRedirects: false
-  });
-
-  this.clock.tick(1);
-
-  assert.notOk(this.player.tech_.vhs.masterPlaylistController_.masterPlaylistLoader_.handleManifestRedirects);
-
-  videojs.options.vhs = vhsOptions;
-});
-
 QUnit.test('the withCredentials option overrides the global default', function(assert) {
   const vhsOptions = videojs.options.vhs;