From e90a603d57836f6b8962a661b4668fd81c9b72a4 Mon Sep 17 00:00:00 2001 From: Arthur Hulsman Date: Tue, 6 Mar 2018 17:27:59 +0100 Subject: Removed a double this.enabled variable and updated a comment in ads.js. Also made sure the adsmanager promise also can fail, so we can use it to wait for getting the advertisement ready when someone clicks the play button. Otherwise there it can look glitchy when the actual video starts playing and the video ad plays a few seconds later because the vast tag was slow to retrieve. Also fixed a typo. --- src/js/plugins/ads.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'src/js/plugins') diff --git a/src/js/plugins/ads.js b/src/js/plugins/ads.js index 0faf0f2f..9318e01d 100644 --- a/src/js/plugins/ads.js +++ b/src/js/plugins/ads.js @@ -36,9 +36,8 @@ class Ads { this.playing = false; this.initialized = false; this.blocked = false; - this.enabled = utils.is.url(player.config.ads.tag); - // Check if a tag URL is provided. + // Check if ads are enabled. if (!this.enabled) { return; } @@ -83,14 +82,12 @@ class Ads { // thing doesn't resolve within our set time; we bail this.startSafetyTimer(12000, 'ready()'); - // Setup a simple promise to resolve if the IMA loader is ready - this.loaderPromise = new Promise(resolve => { - this.on('ADS_LOADER_LOADED', () => resolve()); - }); - // Setup a promise to resolve if the IMA manager is ready - this.managerPromise = new Promise(resolve => { + this.managerPromise = new Promise((resolve, reject) => { + // The ad is pre-loaded and ready this.on('ADS_MANAGER_LOADED', () => resolve()); + // Ads failed + this.on('ERROR', () => reject()); }); // Clear the safety timer -- cgit v1.2.3 From 409b588458fd598a88d56f3922da83ff82d80f5f Mon Sep 17 00:00:00 2001 From: Arthur Hulsman Date: Wed, 7 Mar 2018 15:17:30 +0100 Subject: Made sure that cue points for midrolls are not displayed when the ad rule for a midroll doesn't exceed the total play time of a video. --- src/js/plugins/ads.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/js/plugins') diff --git a/src/js/plugins/ads.js b/src/js/plugins/ads.js index 9318e01d..0497c42c 100644 --- a/src/js/plugins/ads.js +++ b/src/js/plugins/ads.js @@ -206,7 +206,7 @@ class Ads { // Add advertisement cue's within the time line if available this.cuePoints.forEach(cuePoint => { - if (cuePoint !== 0 && cuePoint !== -1) { + if (cuePoint !== 0 && cuePoint !== -1 && cuePoint < this.player.duration) { const seekElement = this.player.elements.progress; if (seekElement) { -- cgit v1.2.3 From 819f7d1080ef2748fa1c123ad0f54d075c654ab9 Mon Sep 17 00:00:00 2001 From: Arthur Hulsman Date: Wed, 7 Mar 2018 15:43:48 +0100 Subject: Resizing the ad container while having it on display none will return offset width and height of 0, which will cause ads not to play when ad sizes are set within the clients DSP. Also making sure that the inner containers of the ad container are full size. The container is now hidden/ displayed using z-index. --- src/js/plugins/ads.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src/js/plugins') diff --git a/src/js/plugins/ads.js b/src/js/plugins/ads.js index 0497c42c..eca523f7 100644 --- a/src/js/plugins/ads.js +++ b/src/js/plugins/ads.js @@ -111,7 +111,6 @@ class Ads { // Create the container for our advertisements this.elements.container = utils.createElement('div', { class: this.player.config.classNames.ads, - hidden: '', }); this.player.elements.container.appendChild(this.elements.container); @@ -451,8 +450,8 @@ class Ads { * Resume our video. */ resumeContent() { - // Hide our ad container - utils.toggleHidden(this.elements.container, true); + // Hide the advertisement container + this.elements.container.style.zIndex = ''; // Ad is stopped this.playing = false; @@ -467,8 +466,8 @@ class Ads { * Pause our video */ pauseContent() { - // Show our ad container. - utils.toggleHidden(this.elements.container, false); + // Show the advertisement container + this.elements.container.style.zIndex = '3'; // Ad is playing. this.playing = true; -- cgit v1.2.3 From 69ffcbad27353bfd0238ba7e79e6caf3835b0efd Mon Sep 17 00:00:00 2001 From: Arthur Hulsman Date: Fri, 9 Mar 2018 11:17:24 +0100 Subject: Ad block detection would not work when calling play() right after creating the player instance, so the adsManager now also rejects on such a case. Also made sure that calling play() will wait for the adsManager promise to resolve or otherwise return the media.play() method. --- src/js/plugins/ads.js | 67 ++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 36 deletions(-) (limited to 'src/js/plugins') diff --git a/src/js/plugins/ads.js b/src/js/plugins/ads.js index eca523f7..cc11fa13 100644 --- a/src/js/plugins/ads.js +++ b/src/js/plugins/ads.js @@ -35,35 +35,6 @@ class Ads { this.enabled = player.config.ads.enabled; this.playing = false; this.initialized = false; - this.blocked = false; - - // Check if ads are enabled. - if (!this.enabled) { - return; - } - - // Check if the Google IMA3 SDK is loaded or load it ourselves - if (!utils.is.object(window.google)) { - utils.loadScript( - player.config.urls.googleIMA.api, - () => { - this.ready(); - }, - () => { - // Script failed to load or is blocked - this.blocked = true; - this.player.debug.log('Ads error: Google IMA SDK failed to load'); - }, - ); - } else { - this.ready(); - } - } - - /** - * Get the ads instance ready. - */ - ready() { this.elements = { container: null, displayContainer: null, @@ -75,26 +46,50 @@ class Ads { this.safetyTimer = null; this.countdownTimer = null; - // Set listeners on the Plyr instance - this.listeners(); - - // Start ticking our safety timer. If the whole advertisement - // thing doesn't resolve within our set time; we bail - this.startSafetyTimer(12000, 'ready()'); + if (this.enabled) { + // Check if the Google IMA3 SDK is loaded or load it ourselves + if (!utils.is.object(window.google)) { + utils.loadScript( + player.config.urls.googleIMA.api, + () => { + this.ready(); + }, + () => { + // Script failed to load or is blocked + this.handleEventListeners('ERROR'); + this.player.debug.log('Ads error: Google IMA SDK failed to load'); + }, + ); + } else { + this.ready(); + } + } - // Setup a promise to resolve if the IMA manager is ready + // Setup a promise to resolve when the IMA manager is ready this.managerPromise = new Promise((resolve, reject) => { // The ad is pre-loaded and ready this.on('ADS_MANAGER_LOADED', () => resolve()); // Ads failed this.on('ERROR', () => reject()); }); + } + + /** + * Get the ads instance ready. + */ + ready() { + // Start ticking our safety timer. If the whole advertisement + // thing doesn't resolve within our set time; we bail + this.startSafetyTimer(12000, 'ready()'); // Clear the safety timer this.managerPromise.then(() => { this.clearSafetyTimer('onAdsManagerLoaded()'); }); + // Set listeners on the Plyr instance + this.listeners(); + // Setup the IMA SDK this.setupIMA(); } -- cgit v1.2.3 From 6a2ca534d219233b20941bfc987f7a4a488502c7 Mon Sep 17 00:00:00 2001 From: Arthur Hulsman Date: Fri, 9 Mar 2018 14:29:37 +0100 Subject: Removed redundant wrappers within the adsmanager promises. --- src/js/plugins/ads.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/js/plugins') diff --git a/src/js/plugins/ads.js b/src/js/plugins/ads.js index cc11fa13..62162e84 100644 --- a/src/js/plugins/ads.js +++ b/src/js/plugins/ads.js @@ -68,9 +68,9 @@ class Ads { // Setup a promise to resolve when the IMA manager is ready this.managerPromise = new Promise((resolve, reject) => { // The ad is pre-loaded and ready - this.on('ADS_MANAGER_LOADED', () => resolve()); + this.on('ADS_MANAGER_LOADED', resolve); // Ads failed - this.on('ERROR', () => reject()); + this.on('ERROR', reject); }); } @@ -503,7 +503,7 @@ class Ads { // Re-set our adsManager promises this.managerPromise = new Promise(resolve => { - this.on('ADS_MANAGER_LOADED', () => resolve()); + this.on('ADS_MANAGER_LOADED', resolve); this.player.debug.log(this.manager); }); -- cgit v1.2.3