From 4d773343beb5c0f9758b152cc1c244db7406eb50 Mon Sep 17 00:00:00 2001 From: Peter Iliev Date: Mon, 11 Sep 2017 15:03:19 -0700 Subject: [PATCH 1/5] [player-1545] Adding in variable to enable skippable ads for iOS10+ --- js/google_ima.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/js/google_ima.js b/js/google_ima.js index ab190c0..8941ba4 100644 --- a/js/google_ima.js +++ b/js/google_ima.js @@ -34,6 +34,7 @@ require("../html5-common/js/utils/utils.js"); this.ready = false; this.runningUnitTests = false; this.sharedVideoElement = null; + this.enableIosSkippableAds = false; //private member variables of this GoogleIMA object var _amc = null; @@ -295,6 +296,12 @@ require("../html5-common/js/utils/utils.js"); this.imaIframeZIndex = metadata.iframeZIndex; } + this.enableIosSkippableAds = false; + if (metadata.hasOwnProperty("enableIosSkippableAds")) + { + this.enableIosSkippableAds = metadata.enableIosSkippableAds; + } + //On second video playthroughs, we will not be initializing the ad manager again. //Attempt to create the ad display container here instead of after the sdk has loaded if (!_IMAAdDisplayContainer) @@ -630,7 +637,10 @@ require("../html5-common/js/utils/utils.js"); { //resumeAd will only be called if we have exited fullscreen //so this is safe to call - this.sharedVideoElement.webkitEnterFullscreen(); + if (OO.isIosMajorVersion < 10) + { + this.sharedVideoElement.webkitEnterFullscreen(); + } this.sharedVideoElement.play(); } _IMAAdsManager.resume(); @@ -1053,6 +1063,8 @@ require("../html5-common/js/utils/utils.js"); google.ima.settings.setVpaidMode(google.ima.ImaSdkSettings.VpaidMode.ENABLED); } + google.ima.settings.setDisableCustomPlaybackForIOS10Plus(this.enableIosSkippableAds); + _IMA_SDK_tryInitAdContainer(); _trySetupAdsRequest(); }); @@ -1535,6 +1547,12 @@ require("../html5-common/js/utils/utils.js"); } //Since IMA handles its own UI, we want the video player to hide its UI elements _amc.hidePlayerUi(this.showAdControls, false); + + //in the case where skippable ads are enabled we want to exit fullscreen + //because custom playback is disabled and ads can't be rendered in fullscreen. + if (OO.iosMajorVersion >= 10 && this.enableIosSkippableAds === true) { + this.sharedVideoElement.webkitExitFullscreen(); + } } else { From e8147f4eeb9f20c4af02ba5a64c848fe91777c26 Mon Sep 17 00:00:00 2001 From: Peter Iliev Date: Mon, 11 Sep 2017 17:28:33 -0700 Subject: [PATCH 2/5] [player-1545] Fixing unit tests for IMA and adding a unit test to check if param sets correct setting in IMA sdk. --- test/unit-test-helpers/mock_ima.js | 2 ++ test/unit-tests/ima_test.js | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/test/unit-test-helpers/mock_ima.js b/test/unit-test-helpers/mock_ima.js index 0488332..024b4b0 100644 --- a/test/unit-test-helpers/mock_ima.js +++ b/test/unit-test-helpers/mock_ima.js @@ -12,6 +12,7 @@ google = google.ima.linearAds = true; google.ima.delayAdRequest = false; google.ima.adsRenderingSettingsInstance = null; + google.ima.disableCustomPlaybackForIOS10Plus = false; }, Ad : function() { //see https://developers.google.com/interactive-media-ads/docs/sdks/html5/v3/apis#ima.Ad @@ -72,6 +73,7 @@ google = setVpaidMode : function() {}, setLocale : function() {}, setDisableFlashAds : function() {}, + setDisableCustomPlaybackForIOS10Plus : function(newValue) {google.ima.disableCustomPlaybackForIOS10Plus = newValue;} }, AdsManagerLoadedEvent : { diff --git a/test/unit-tests/ima_test.js b/test/unit-tests/ima_test.js index c731e4e..462f7b6 100644 --- a/test/unit-tests/ima_test.js +++ b/test/unit-tests/ima_test.js @@ -1829,4 +1829,30 @@ describe('ad_manager_ima', function() expect(_.isEmpty(google.ima.adsRenderingSettingsInstance)).to.be(false); expect(google.ima.adsRenderingSettingsInstance.loadVideoTimeout).to.be(15000); }); + + it('Skippable Ads: Test page level param', function () + { + //default case + ima.initialize(amc, playerId); + expect(google.ima.disableCustomPlaybackForIOS10Plus).to.be(false); + expect(ima.enableIosSkippableAds).to.be(false); + + var content = + { + enableIosSkippableAds : false + }; + ima.loadMetadata(content, {}, {}); + ima.initialize(amc, playerId); + expect(google.ima.disableCustomPlaybackForIOS10Plus).to.be(false); + expect(ima.enableIosSkippableAds).to.be(false); + + content = + { + enableIosSkippableAds : true + }; + ima.loadMetadata(content, {}, {}); + ima.initialize(amc, playerId); + expect(google.ima.disableCustomPlaybackForIOS10Plus).to.be(true); + expect(ima.enableIosSkippableAds).to.be(true); + }); }); From 6e0b4e63ae1bf29a073919d893a0ec5998e6d91a Mon Sep 17 00:00:00 2001 From: Peter Iliev Date: Mon, 11 Sep 2017 17:45:04 -0700 Subject: [PATCH 3/5] [player-1545] Moving code to set google sdk settings to be after sdkload and loadMetadata called, so we have the page level overrides and avoid a possible race condition. Adjusting the unit tests to work too. --- js/google_ima.js | 35 +++++++++++++++++++---------------- test/unit-tests/ima_test.js | 20 ++++++++++++++++---- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/js/google_ima.js b/js/google_ima.js index 8941ba4..55099d5 100644 --- a/js/google_ima.js +++ b/js/google_ima.js @@ -1050,21 +1050,6 @@ require("../html5-common/js/utils/utils.js"); return; } - //These are required by Google for tracking purposes. - google.ima.settings.setPlayerVersion(PLUGIN_VERSION); - google.ima.settings.setPlayerType(PLAYER_TYPE); - google.ima.settings.setLocale(OO.getLocale()); - if (this.useInsecureVpaidMode) - { - google.ima.settings.setVpaidMode(google.ima.ImaSdkSettings.VpaidMode.INSECURE); - } - else - { - google.ima.settings.setVpaidMode(google.ima.ImaSdkSettings.VpaidMode.ENABLED); - } - - google.ima.settings.setDisableCustomPlaybackForIOS10Plus(this.enableIosSkippableAds); - _IMA_SDK_tryInitAdContainer(); _trySetupAdsRequest(); }); @@ -1087,6 +1072,24 @@ require("../html5-common/js/utils/utils.js"); _IMAAdDisplayContainer.destroy(); } + //**It's now safe to set SDK settings, we have all the page level overrides and + //the SDK is guaranteed to be loaded. + + //These are required by Google for tracking purposes. + google.ima.settings.setPlayerVersion(PLUGIN_VERSION); + google.ima.settings.setPlayerType(PLAYER_TYPE); + google.ima.settings.setLocale(OO.getLocale()); + if (this.useInsecureVpaidMode) + { + google.ima.settings.setVpaidMode(google.ima.ImaSdkSettings.VpaidMode.INSECURE); + } + else + { + google.ima.settings.setVpaidMode(google.ima.ImaSdkSettings.VpaidMode.ENABLED); + } + + google.ima.settings.setDisableCustomPlaybackForIOS10Plus(this.enableIosSkippableAds); + //Prefer to use player skin plugins element to allow for click throughs. Use plugins element if not available _uiContainer = _amc.ui.playerSkinPluginsElement ? _amc.ui.playerSkinPluginsElement[0] : _amc.ui.pluginsElement[0]; //iphone performance is terrible if we don't use the custom playback (i.e. filling in the second param for adDisplayContainer) @@ -1547,7 +1550,7 @@ require("../html5-common/js/utils/utils.js"); } //Since IMA handles its own UI, we want the video player to hide its UI elements _amc.hidePlayerUi(this.showAdControls, false); - + //in the case where skippable ads are enabled we want to exit fullscreen //because custom playback is disabled and ads can't be rendered in fullscreen. if (OO.iosMajorVersion >= 10 && this.enableIosSkippableAds === true) { diff --git a/test/unit-tests/ima_test.js b/test/unit-tests/ima_test.js index 462f7b6..fb16b75 100644 --- a/test/unit-tests/ima_test.js +++ b/test/unit-tests/ima_test.js @@ -1830,29 +1830,41 @@ describe('ad_manager_ima', function() expect(google.ima.adsRenderingSettingsInstance.loadVideoTimeout).to.be(15000); }); - it('Skippable Ads: Test page level param', function () + it('IOS Skippable Ads: Test page level param default', function () { //default case ima.initialize(amc, playerId); + ima.loadMetadata({}, {}, {}); + ima.registerUi(); expect(google.ima.disableCustomPlaybackForIOS10Plus).to.be(false); expect(ima.enableIosSkippableAds).to.be(false); + }); + it('IOS Skippable Ads: Test page level param false', function () + { var content = { enableIosSkippableAds : false }; - ima.loadMetadata(content, {}, {}); ima.initialize(amc, playerId); + ima.loadMetadata(content, {}, {}); + ima.registerUi(); expect(google.ima.disableCustomPlaybackForIOS10Plus).to.be(false); expect(ima.enableIosSkippableAds).to.be(false); + }); - content = + it('IOS Skippable Ads: Test page level param true', function () + { + var content = { enableIosSkippableAds : true }; - ima.loadMetadata(content, {}, {}); ima.initialize(amc, playerId); + ima.loadMetadata(content, {}, {}); + ima.registerUi(); expect(google.ima.disableCustomPlaybackForIOS10Plus).to.be(true); expect(ima.enableIosSkippableAds).to.be(true); }); + + }); From 8fbb24fa46135104a722c525b6a31b4d9f817142 Mon Sep 17 00:00:00 2001 From: Peter Iliev Date: Mon, 11 Sep 2017 18:26:08 -0700 Subject: [PATCH 4/5] [player-1545] Fixing PR comments. --- js/google_ima.js | 9 +++++++-- test/unit-test-helpers/mock_ima.js | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/js/google_ima.js b/js/google_ima.js index 55099d5..182d637 100644 --- a/js/google_ima.js +++ b/js/google_ima.js @@ -637,7 +637,7 @@ require("../html5-common/js/utils/utils.js"); { //resumeAd will only be called if we have exited fullscreen //so this is safe to call - if (OO.isIosMajorVersion < 10) + if (!_inlinePlaybackSupported()) { this.sharedVideoElement.webkitEnterFullscreen(); } @@ -1553,7 +1553,7 @@ require("../html5-common/js/utils/utils.js"); //in the case where skippable ads are enabled we want to exit fullscreen //because custom playback is disabled and ads can't be rendered in fullscreen. - if (OO.iosMajorVersion >= 10 && this.enableIosSkippableAds === true) { + if (_inlinePlaybackSupported() && OO.isIphone && this.enableIosSkippableAds === true) { this.sharedVideoElement.webkitExitFullscreen(); } } @@ -2067,6 +2067,11 @@ require("../html5-common/js/utils/utils.js"); } }; + var _inlinePlaybackSupported = function() + { + return !(OO.iosMajorVersion < 10 && OO.isIphone); + }; + var _throwError = function(outputStr) { //TODO consolidate code to exit gracefully if we have an error. diff --git a/test/unit-test-helpers/mock_ima.js b/test/unit-test-helpers/mock_ima.js index 024b4b0..c493418 100644 --- a/test/unit-test-helpers/mock_ima.js +++ b/test/unit-test-helpers/mock_ima.js @@ -7,6 +7,7 @@ google = adManagerInstance : null, //for unit test convenience adLoaderInstance : null, //for unit test convenience adsRenderingSettingsInstance : null, //for unit test convenience + disableCustomPlaybackForIOS10Plus : false, //for unit test convenience resetDefaultValues : function() { google.ima.linearAds = true; From b5e73a6d9f9f1595655bdfd3acc864e0c1298035 Mon Sep 17 00:00:00 2001 From: Peter Iliev Date: Mon, 11 Sep 2017 19:09:57 -0700 Subject: [PATCH 5/5] [player-1545] video element not being available to exit fullscreen. --- js/google_ima.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/js/google_ima.js b/js/google_ima.js index bf8593c..a07d425 100644 --- a/js/google_ima.js +++ b/js/google_ima.js @@ -1372,14 +1372,11 @@ require("../html5-common/js/utils/utils.js"); _IMAAdsManager = adsManagerLoadedEvent.getAdsManager(_playheadTracker, adsSettings); // When the ads manager is ready, we are ready to apply css changes to the video element - // If the sharedVideoElement is not used, mark it as null before applying css if (this.videoControllerWrapper) { this.videoControllerWrapper.readyForCss = true; } - if (!_IMAAdsManager.isCustomPlaybackUsed()) { - this.setupSharedVideoElement(null); - } + if (this.videoControllerWrapper) { this.videoControllerWrapper.applyStoredCss(); @@ -1622,8 +1619,12 @@ require("../html5-common/js/utils/utils.js"); //in the case where skippable ads are enabled we want to exit fullscreen //because custom playback is disabled and ads can't be rendered in fullscreen. - if (_inlinePlaybackSupported() && OO.isIphone && this.enableIosSkippableAds === true) { - this.sharedVideoElement.webkitExitFullscreen(); + if (_inlinePlaybackSupported() && OO.isIphone && this.enableIosSkippableAds === true) + { + if (this.sharedVideoElement) + { + this.sharedVideoElement.webkitExitFullscreen(); + } } } else