From 69ffcbad27353bfd0238ba7e79e6caf3835b0efd Mon Sep 17 00:00:00 2001
From: Arthur Hulsman <a.hulsman@orangegames.com>
Date: Fri, 9 Mar 2018 11:17:24 +0100
Subject: [PATCH] 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 ++++++++++++++++++---------------------
 src/js/plyr.js            |  7 ++--
 src/sass/plugins/ads.scss |  8 ++---
 3 files changed, 37 insertions(+), 45 deletions(-)

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();
+        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();
+            }
+        }
 
-        // Start ticking our safety timer. If the whole advertisement
-        // thing doesn't resolve within our set time; we bail
-        this.startSafetyTimer(12000, '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();
     }
diff --git a/src/js/plyr.js b/src/js/plyr.js
index 9b73423e..d4c3d392 100644
--- a/src/js/plyr.js
+++ b/src/js/plyr.js
@@ -309,14 +309,15 @@ class Plyr {
      * Play the media, or play the advertisement (if they are not blocked)
      */
     play() {
-        if (this.ads.enabled && !this.ads.initialized && !this.ads.blocked) {
+        // Return the promise (for HTML5)
+        if (this.ads.enabled && !this.ads.initialized) {
             this.ads.managerPromise.then(() => {
                 this.ads.play();
             }).catch(() => {
-                this.media.play();
+                return this.media.play();
             });
         } else {
-            this.media.play();
+            return this.media.play();
         }
     }
 
diff --git a/src/sass/plugins/ads.scss b/src/sass/plugins/ads.scss
index 98620508..89330611 100644
--- a/src/sass/plugins/ads.scss
+++ b/src/sass/plugins/ads.scss
@@ -12,15 +12,11 @@
     z-index: -1; // Hide it.
 
     // Make sure the inner container is big enough for the ad creative.
-    > div {
+    > div,
+    > div iframe {
         position: absolute;
         width: 100%;
         height: 100%;
-        iframe {
-            position: absolute;
-            width: 100%;
-            height: 100%;
-        }
     }
 
     &::after {