From 7e7d98d7ce89fedd605a40c9c6a7cac20bb74258 Mon Sep 17 00:00:00 2001 From: Chris Sinchok Date: Mon, 3 Nov 2014 18:09:05 -0600 Subject: [PATCH 1/4] Fixes for the vast plugin, against our branch of videojs-contrib-ads --- bower.json | 2 +- spec/VastPluginSpec.js | 22 ++++++++++++---------- videojs.vast.js | 15 +++++++-------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/bower.json b/bower.json index 71a5205..4f8a8e8 100644 --- a/bower.json +++ b/bower.json @@ -30,6 +30,6 @@ "dependencies": { "videojs": "4.4.3", "vast-client-js": "1.1.2", - "videojs-contrib-ads": "0.4.0" + "videojs-contrib-ads": "theonion/videojs-contrib-ads#e4718b919ef772ca24619a30e2bf5f6cc27ac8f1" } } diff --git a/spec/VastPluginSpec.js b/spec/VastPluginSpec.js index f967ee9..6dbd0f9 100644 --- a/spec/VastPluginSpec.js +++ b/spec/VastPluginSpec.js @@ -34,10 +34,10 @@ describe('videojs.vast plugin', function() { } })(); player = videojs(id); - player.ads(); - player.vast({ - url: "" - }); + // player.ads(); + // player.vast({ + // url: "" + // }); }); @@ -70,11 +70,11 @@ describe('videojs.vast plugin', function() { }); it("should bail out if no url is provided", function() { - spyOn(this.p, "trigger"); this.p.ads(); var result = this.p.vast({}); - expect(result).toBe(null); - expect(this.p.trigger).toHaveBeenCalledWith("adtimeout"); + spyOn(this.p, "trigger").and.callThrough(); + this.p.trigger('readyforpreroll'); + expect(this.p.trigger).toHaveBeenCalledWith("adscanceled"); }); it("should request an ad if a source is already loaded", function() { @@ -122,6 +122,7 @@ describe('videojs.vast plugin', function() { describe("tearDown", function() { it("should end the linear ad", function() { + player.ads(); player.vast({ url: 'balhbahblhab' }); spyOn(player.ads, "endLinearAdMode"); @@ -143,6 +144,7 @@ describe('videojs.vast plugin', function() { describe("preroll", function() { beforeEach(function() { + player.ads(); player.vast({ url: 'balhbahblhab' }); player.vastTracker = { clickThroughURLTemplate: "a whole new page", @@ -163,7 +165,7 @@ describe('videojs.vast plugin', function() { }); it("should end the ad", function() { - spyOn(player, "one"); + spyOn(player, "one"); player.vast.preroll(); expect(player.one).toHaveBeenCalledWith("ended", jasmine.any(Function)); }); @@ -187,7 +189,7 @@ describe('videojs.vast plugin', function() { spyOn(player, "trigger"); player.vast.getContent("some url"); - expect(player.trigger).toHaveBeenCalledWith("adtimeout"); + expect(player.trigger).toHaveBeenCalledWith("adscanceled"); }); describe("linear ads", function() { @@ -231,7 +233,7 @@ describe('videojs.vast plugin', function() { expect(DMVAST.util.track).toHaveBeenCalledWith( jasmine.any(String), jasmine.any(Object) ); - expect(player.trigger).toHaveBeenCalledWith("adtimeout"); + expect(player.trigger).toHaveBeenCalledWith("adscanceled"); }); }); diff --git a/videojs.vast.js b/videojs.vast.js index 39c39e0..29881d0 100644 --- a/videojs.vast.js +++ b/videojs.vast.js @@ -87,7 +87,7 @@ player.vast.sources = player.vast.createSourceObjects(creative.mediaFiles); if (!player.vast.sources.length) { - player.trigger('adtimeout'); + player.trigger('adscanceled'); return; } @@ -158,7 +158,7 @@ if (!player.vastTracker) { // No pre-roll, start video - player.trigger('adtimeout'); + player.trigger('adscanceled'); } }); }, @@ -274,12 +274,6 @@ return null; } - // if we don't have a vast url, just bail out - if (!settings.url) { - player.trigger('adtimeout'); - return null; - } - // set up vast plugin, then set up events here player.vast = new Vast(player, settings); @@ -306,6 +300,11 @@ }); player.on('readyforpreroll', function() { + // if we don't have a vast url, just bail out + if (!settings.url) { + player.trigger('adscanceled'); + return null; + } // set up and start playing preroll player.vast.preroll(); }); From bf99bf6a5c4f4800e5212508007bc61fd1589912 Mon Sep 17 00:00:00 2001 From: Chris Sinchok Date: Thu, 6 Nov 2014 16:06:14 -0600 Subject: [PATCH 2/4] Pulling the trackers out into a separate method --- Gruntfile.js | 2 - spec/VastPluginSpec.js | 1 - spec/desireds.js | 82 --------------------------------- spec/sauce-wd.js | 100 ----------------------------------------- videojs.vast.js | 87 ++++++++++++++++++----------------- 5 files changed, 47 insertions(+), 225 deletions(-) delete mode 100644 spec/desireds.js delete mode 100644 spec/sauce-wd.js diff --git a/Gruntfile.js b/Gruntfile.js index d18b3d7..1999291 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -5,8 +5,6 @@ var specs = "spec/*Spec.js", var _ = require('lodash'); -var desireds = require('./spec/desireds.js'); - var gruntConfig = { env: { // dynamically filled diff --git a/spec/VastPluginSpec.js b/spec/VastPluginSpec.js index f967ee9..a6bff6d 100644 --- a/spec/VastPluginSpec.js +++ b/spec/VastPluginSpec.js @@ -236,6 +236,5 @@ describe('videojs.vast plugin', function() { }); }); - }); }); \ No newline at end of file diff --git a/spec/desireds.js b/spec/desireds.js deleted file mode 100644 index bb01d31..0000000 --- a/spec/desireds.js +++ /dev/null @@ -1,82 +0,0 @@ -'use strict'; - -module.exports = { - chrome_mav: { - browserName: 'chrome', - version: '31', - platform: 'Mac 10.9', - name: "Chrome 31 on Mavericks" - }, - chrome_snow: { - browserName: 'chrome', - version: '27', - platform: 'Mac 10.6', - name: "Chrome 27 on Snow Leopard" - }, - firefox_mav: { - browserName: 'firefox', - version: '26', - platform: 'Mac 10.9', - name: "Firefox 26 on Mavericks" - }, - // firefox_snow: { - // browserName: 'firefox', - // version: '15', - // platform: 'Mac 10.6', - // name: "Firefox 15 on Snow Leopard" - // }, - // ie8: { - // browserName: 'internet explorer', - // version: '8', - // platform: 'Windows XP', - // name: "IE 8 on Windows XP" - // }, - // ie9: { - // browserName: 'internet explorer', - // version: '9', - // platform: 'Windows 7', - // name: "IE 9 on Windows 7" - // }, - // ie10: { - // browserName: 'internet explorer', - // version: '10', - // platform: 'Windows 8', - // name: "IE 10 on Windows 8" - // }, - // ie11: { - // browserName: 'internet explorer', - // version: '11', - // platform: 'Windows 8.1', - // name: "IE 11 on Windows 8.1" - // }, - // io5: { - // browserName: 'iphone', - // version: '5.0', - // platform: 'Mac 10.6', - // name: "iPhone 5 test" - // }, - // io6: { - // browserName: 'iphone', - // version: '6.0', - // platform: 'Mac 10.8', - // name: "iPhone 6 test" - // }, - // io7: { - // browserName: 'iphone', - // version: '7.0', - // platform: 'Mac 10.9', - // name: "iPhone 7 test" - // }, - // android4: { - // browserName: 'android', - // version: '4.0', - // platform: 'Linux', - // name: "Android 4.0 test" - // }, - // android4_3: { - // browserName: 'android', - // version: '4.3', - // platform: 'Linux', - // name: "Android 4.3 test" - // } -}; \ No newline at end of file diff --git a/spec/sauce-wd.js b/spec/sauce-wd.js deleted file mode 100644 index be549a5..0000000 --- a/spec/sauce-wd.js +++ /dev/null @@ -1,100 +0,0 @@ -var wd = require('wd'); -require('colors'); -var _ = require("lodash"); -var chai = require("chai"); -var chaiAsPromised = require("chai-as-promised"); - -chai.use(chaiAsPromised); -chai.should(); -chaiAsPromised.transferPromiseness = wd.transferPromiseness; - -// checking sauce credential -if(!process.env.SAUCE_USERNAME || !process.env.SAUCE_ACCESS_KEY){ - console.warn( - '\nPlease configure your sauce credential:\n\n' + - 'export SAUCE_USERNAME=\n' + - 'export SAUCE_ACCESS_KEY=\n\n' - ); - throw new Error("Missing sauce credentials"); -} - -// http configuration, not needed for simple runs -wd.configureHttp( { - timeout: 60000, - retryDelay: 15000, - retries: 5 -}); - -var desired = JSON.parse(process.env.DESIRED || '{browserName: "chrome"}'); - -desired.tags = [process.env.TRAVIS_NODE_VERSION.toString(), 'CI']; -desired["tunnel-identifier"] = process.env.TRAVIS_JOB_NUMBER; - -describe('tutorial (' + desired.browserName + ')', function() { - var browser; - var allPassed = true; - - before(function(done) { - var username = process.env.SAUCE_USERNAME; - var accessKey = process.env.SAUCE_ACCESS_KEY; - browser = wd.promiseChainRemote("ondemand.saucelabs.com", 80, username, accessKey); - if(process.env.VERBOSE){ - // optional logging - browser.on('status', function(info) { - console.log(info.cyan); - }); - browser.on('command', function(meth, path, data) { - console.log(' > ' + meth.yellow, path.grey, data || ''); - }); - } - browser - .init(desired) - .nodeify(done); - }); - - afterEach(function(done) { - allPassed = allPassed && (this.currentTest.state === 'passed'); - done(); - }); - - after(function(done) { - browser - .quit() - .sauceJobStatus(allPassed) - .nodeify(done); - }); - - it("should get home page", function(done) { - browser - .get("http://nodejs.org/") - .title() - .should.become("node.js") - .elementById("intro") - .text() - .should.eventually.include('JavaScript runtime') - .nodeify(done); - }); - - _(2).times(function(i) { // repeat twice - - it("should go to the doc page (" + i + ")", function(done) { - browser - .elementById('docsbutton') - .click() - .waitForElementByCss("#content header", wd.asserters.textInclude('Manual'), 10000) - .title() - .should.eventually.include("Manual") - .nodeify(done); - }); - - it("should return to the home page(" + i + ")", function(done) { - browser - .elementById('logo') - .click() - .waitForElementById("intro", wd.asserters.textInclude('JavaScript runtime'), 10000) - .title() - .should.not.eventually.include("Manual") - .nodeify(done); - }); - }); -}); diff --git a/videojs.vast.js b/videojs.vast.js index 39c39e0..1fecb5f 100644 --- a/videojs.vast.js +++ b/videojs.vast.js @@ -93,46 +93,6 @@ player.vastTracker = new vast.tracker(ad, creative); - var errorOccurred = false, - canplayFn = function() { - this.vastTracker.load(); - }, - timeupdateFn = function() { - if (isNaN(this.vastTracker.assetDuration)) { - this.vastTracker.assetDuration = this.duration(); - } - this.vastTracker.setProgress(this.currentTime()); - }, - playFn = function() { - this.vastTracker.setPaused(false); - }, - pauseFn = function() { - this.vastTracker.setPaused(true); - }, - errorFn = function() { - // Inform ad server we couldn't play the media file for this ad - vast.util.track(ad.errorURLTemplates, {ERRORCODE: 405}); - errorOccurred = true; - player.trigger('ended'); - }; - - player.on('canplay', canplayFn); - player.on('timeupdate', timeupdateFn); - player.on('play', playFn); - player.on('pause', pauseFn); - player.on('error', errorFn); - - player.one('ended', function() { - player.off('canplay', canplayFn); - player.off('timeupdate', timeupdateFn); - player.off('play', playFn); - player.off('pause', pauseFn); - player.off('error', errorFn); - if (!errorOccurred) { - this.vastTracker.complete(); - } - }); - foundCreative = true; } @@ -163,6 +123,51 @@ }); }, + setupEvents: function() { + + var errorOccurred = false, + canplayFn = function() { + player.vastTracker.load(); + }, + timeupdateFn = function() { + if (isNaN(player.vastTracker.assetDuration)) { + player.vastTracker.assetDuration = player.duration(); + } + player.vastTracker.setProgress(player.currentTime()); + }, + playFn = function() { + if (player.ads.state === 'ad-playback') { + player.vastTracker.setPaused(false); + } + }, + pauseFn = function() { + player.vastTracker.setPaused(true); + }, + errorFn = function() { + // Inform ad server we couldn't play the media file for this ad + vast.util.track(player.vastTracker.ad.errorURLTemplates, {ERRORCODE: 405}); + errorOccurred = true; + player.trigger('ended'); + }; + + player.on('canplay', canplayFn); + player.on('timeupdate', timeupdateFn); + player.on('play', playFn); + player.on('pause', pauseFn); + player.on('error', errorFn); + + player.one('ended', function() { + player.off('canplay', canplayFn); + player.off('timeupdate', timeupdateFn); + player.off('play', playFn); + player.off('pause', pauseFn); + player.off('error', errorFn); + if (!errorOccurred) { + player.vastTracker.complete(); + } + }); + }, + preroll: function() { player.ads.startLinearAdMode(); player.vast.showControls = player.controls(); @@ -223,6 +228,8 @@ } }; + player.vast.setupEvents(); + player.one('ended', player.vast.tearDown); player.trigger('vast-preroll-ready'); From a4c94c48de0b0d8231fd6b1ccdbe706382db0595 Mon Sep 17 00:00:00 2001 From: Chris Sinchok Date: Thu, 6 Nov 2014 16:19:41 -0600 Subject: [PATCH 3/4] Cleaning up play/pause events --- videojs.vast.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/videojs.vast.js b/videojs.vast.js index aeccceb..429847e 100644 --- a/videojs.vast.js +++ b/videojs.vast.js @@ -135,13 +135,11 @@ } player.vastTracker.setProgress(player.currentTime()); }, - playFn = function() { - if (player.ads.state === 'ad-playback') { - player.vastTracker.setPaused(false); - } - }, pauseFn = function() { player.vastTracker.setPaused(true); + player.one('play', function(){ + player.vastTracker.setPaused(false); + }); }, errorFn = function() { // Inform ad server we couldn't play the media file for this ad @@ -152,14 +150,12 @@ player.on('canplay', canplayFn); player.on('timeupdate', timeupdateFn); - player.on('play', playFn); player.on('pause', pauseFn); player.on('error', errorFn); - player.one('ended', function() { + player.one('vast-preroll-removed', function() { player.off('canplay', canplayFn); player.off('timeupdate', timeupdateFn); - player.off('play', playFn); player.off('pause', pauseFn); player.off('error', errorFn); if (!errorOccurred) { From ffe84c065a15d7e3d0dda578a07b52a848ff908b Mon Sep 17 00:00:00 2001 From: Chris Sinchok Date: Tue, 18 Nov 2014 10:10:06 -0600 Subject: [PATCH 4/4] New videojs-contrib-ads version --- bower.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bower.json b/bower.json index 4f8a8e8..ea51344 100644 --- a/bower.json +++ b/bower.json @@ -30,6 +30,6 @@ "dependencies": { "videojs": "4.4.3", "vast-client-js": "1.1.2", - "videojs-contrib-ads": "theonion/videojs-contrib-ads#e4718b919ef772ca24619a30e2bf5f6cc27ac8f1" + "videojs-contrib-ads": "0.5.0" } }