Skip to content

Commit

Permalink
refactor: Remove playOnLoad from setContentWith(AdTag|AdsResponse|ads…
Browse files Browse the repository at this point in the history
…Request). This stopped working with the refactor, and due to how the player and contrib-ads handle autoplay, is no longer supported. Fixes #524."
  • Loading branch information
shawnbuso committed Jun 13, 2018
1 parent 82e9b04 commit 722930a
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 44 deletions.
3 changes: 1 addition & 2 deletions examples/playlist/ads.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ Ads.prototype.onPlaylistItemClick = function(event) {
}
this.player.ima.setContentWithAdTag(
this.contents[event.target.id],
null,
false);
null);
this.player.poster(this.posters[event.target.id]);
this.player.ima.requestAds();
}
Expand Down
18 changes: 6 additions & 12 deletions src/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,14 +527,12 @@ Controller.prototype.onPlayerVolumeChanged = function(volume) {
* blank to use the existing content.
* @param {?string} adTag The ad tag to be requested when the content loads.
* Leave blank to use the existing ad tag.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
Controller.prototype.setContentWithAdTag =
function(contentSrc, adTag, playOnLoad) {
function(contentSrc, adTag) {
this.reset();
this.settings.adTagUrl = adTag ? adTag : this.settings.adTagUrl;
this.playerWrapper.changeSource(contentSrc, playOnLoad);
this.playerWrapper.changeSource(contentSrc);
};


Expand All @@ -546,15 +544,13 @@ Controller.prototype.setContentWithAdTag =
* blank to use the existing content.
* @param {?string} adsResponse The ads response to be requested when the
* content loads. Leave blank to use the existing ads response.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
Controller.prototype.setContentWithAdsResponse =
function(contentSrc, adsResponse, playOnLoad) {
function(contentSrc, adsResponse) {
this.reset();
this.settings.adsResponse =
adsResponse ? adsResponse : this.settings.adsResponse;
this.playerWrapper.changeSource(contentSrc, playOnLoad);
this.playerWrapper.changeSource(contentSrc);
};

/**
Expand All @@ -565,15 +561,13 @@ Controller.prototype.setContentWithAdsResponse =
* blank to use the existing content.
* @param {?Object} adsRequest The ads request to be requested when the
* content loads. Leave blank to use the existing ads request.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
Controller.prototype.setContentWithAdsRequest =
function(contentSrc, adsRequest, playOnLoad) {
function(contentSrc, adsRequest) {
this.reset();
this.settings.adsRequest =
adsRequest ? adsRequest : this.settings.adsRequest;
this.playerWrapper.changeSource(contentSrc, playOnLoad);
this.playerWrapper.changeSource(contentSrc);
};


Expand Down
18 changes: 6 additions & 12 deletions src/ima-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,9 @@ const ImaPlugin = function(player, options) {
* blank to use the existing content.
* @param {?string} adTag The ad tag to be requested when the content loads.
* Leave blank to use the existing ad tag.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
this.setContentWithAdTag = function(contentSrc, adTag, playOnLoad) {
this.controller.setContentWithAdTag(contentSrc, adTag, playOnLoad);
this.setContentWithAdTag = function(contentSrc, adTag) {
this.controller.setContentWithAdTag(contentSrc, adTag);
}.bind(this);


Expand All @@ -178,13 +176,11 @@ const ImaPlugin = function(player, options) {
* blank to use the existing content.
* @param {?string} adsResponse The ads response to be requested when the
* content loads. Leave blank to use the existing ads response.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
this.setContentWithAdsResponse =
function(contentSrc, adsResponse, playOnLoad) {
function(contentSrc, adsResponse) {
this.controller.setContentWithAdsResponse(
contentSrc, adsResponse, playOnLoad);
contentSrc, adsResponse);
}.bind(this);

/**
Expand All @@ -195,13 +191,11 @@ const ImaPlugin = function(player, options) {
* blank to use the existing content.
* @param {?Object} adsRequest The ads request to be requested when the
* content loads. Leave blank to use the existing ads request.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
this.setContentWithAdsRequest =
function(contentSrc, adsRequest, playOnLoad) {
function(contentSrc, adsRequest) {
this.controller.setContentWithAdsRequest(
contentSrc, adsRequest, playOnLoad);
contentSrc, adsRequest);
}.bind(this);


Expand Down
20 changes: 2 additions & 18 deletions src/player-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,10 +513,8 @@ PlayerWrapper.prototype.onAdsReady = function() {
* Changes the player source.
* @param {?string} contentSrc The URI for the content to be played. Leave
* blank to use the existing content.
* @param {?boolean} playOnLoad True to play the content once it has loaded,
* false to only load the content but not start playback.
*/
PlayerWrapper.prototype.changeSource = function(contentSrc, playOnLoad) {
PlayerWrapper.prototype.changeSource = function(contentSrc) {
// Only try to pause the player when initialised with a source already
if (this.vjsPlayer.currentSrc()) {
this.vjsPlayer.currentTime(0);
Expand All @@ -525,11 +523,7 @@ PlayerWrapper.prototype.changeSource = function(contentSrc, playOnLoad) {
if (contentSrc) {
this.vjsPlayer.src(contentSrc);
}
if (playOnLoad) {
this.vjsPlayer.one('loadedmetadata', this.playContentFromZero.bind(this));
} else {
this.vjsPlayer.one('loadedmetadata', this.seekContentToZero.bind(this));
}
this.vjsPlayer.one('loadedmetadata', this.seekContentToZero.bind(this));
};

/**
Expand All @@ -541,16 +535,6 @@ PlayerWrapper.prototype.seekContentToZero = function() {
this.vjsPlayer.currentTime(0);
};

/**
* Seeks content to 00:00:00 and starts playback. This is used as an event
* handler for the loadedmetadata event, since seeking is not possible until
* that event has fired.
*/
PlayerWrapper.prototype.playContentFromZero = function() {
this.vjsPlayer.currentTime(0);
this.vjsPlayer.play();
};

/**
* Triggers an event on the VJS player
* @param {string} name The event name.
Expand Down

0 comments on commit 722930a

Please sign in to comment.