Skip to content

Commit

Permalink
Add a flag to ensure we only trigger the play event when the contrib-…
Browse files Browse the repository at this point in the history
…ads play middleware terminates, and not when another middleware terminates.
  • Loading branch information
ldayananda committed Apr 19, 2018
1 parent 1482511 commit f9762bf
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/playMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,21 @@ obj.playMiddleware = function(player) {
// ad supported player
if (player.ads && player.ads._shouldBlockPlay === true) {
player.ads.debug('Using playMiddleware to block content playback');
player.ads._playBlocked = true;
return videojsReference.middleware.TERMINATOR;
}
},
play(terminated, value) {
if (player.ads && terminated) {
if (player.ads && player.ads._playBlocked && terminated) {
player.ads.debug('Play call to Tech was terminated.');
// Trigger play event to match the user's intent to play.
// The call to play on the Tech has been blocked, so triggering
// the event on the Player will not affect the Tech's playback state.
player.trigger('play');
// At this point the player has technically started
player.addClass('vjs-has-started');
// Reset playBlocked
player.ads._playBlocked = false;
}
}
};
Expand Down
3 changes: 3 additions & 0 deletions src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ const contribAdsPlugin = function(options) {

// Should we block calls to play on the content player?
_shouldBlockPlay: false,
// Was play blocked by the plugin?
_playBlocked: false,
// Tracks whether play has been requested for this source,
// either by the play method or user interaction
_playRequested: false,
Expand All @@ -213,6 +215,7 @@ const contribAdsPlugin = function(options) {
player.ads._hasThereBeenALoadedMetaData = false;
player.ads._cancelledPlay = false;
player.ads._shouldBlockPlay = false;
player.ads._playBlocked = false;
player.ads.nopreroll_ = false;
player.ads.nopostroll_ = false;
player.ads._playRequested = false;
Expand Down
59 changes: 59 additions & 0 deletions test/integration/test.playMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,63 @@ QUnit.test("playMiddleware doesn\'t block play in content playback", function(as
});

this.player.ready(this.player.play);
});

QUnit.test("don't trigger play event if another middleware terminates", function(assert) {
const done = assert.async();
const fixture = document.querySelector('#qunit-fixture');
const vid = document.createElement('video');
const playSpy = sinon.spy();

let shouldTerminate = true;
let anotherMiddleware = function(player) {
return {
setSource(srcObj, next) {
next(null, srcObj);
},
callPlay() {
if (shouldTerminate === true) {
shouldTerminate = false;
return videojs.middleware.TERMINATOR;
}
},
play(terminated, value) {
}
};
};
let localPlayer;

// Register the other middleware
videojs.use('video/webm', anotherMiddleware);

// Don't use the shared player, setup new localPlayer
fixture.appendChild(vid);
localPlayer = videojs(vid);

// Setup before example integration
localPlayer.on('nopreroll', () => {
localPlayer.ready(localPlayer.play);
});

// Don't play preroll ads
localPlayer.exampleAds({
'adServerUrl': '/base/test/integration/lib/inventory.json',
playPreroll: false
});

localPlayer.on('play', playSpy);

// Wait for the middleware to run
localPlayer.setTimeout(() => {
assert.strictEqual(localPlayer.ads._playBlocked, false,
'play should not have been blocked');
assert.strictEqual(playSpy.callCount, 0,
'play event should not be triggered');
done();
}, 200);

localPlayer.src({
src: 'http://vjs.zencdn.net/v/oceans.webm',
type: 'video/webm'
});
});
31 changes: 30 additions & 1 deletion test/unit/test.playMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ QUnit.module('Play Middleware', {}, function() {
this.player = {
ads: {
_shouldBlockPlay: false,
_playBlocked: false,
debug: () => {}
},
trigger: (event) => {
Expand Down Expand Up @@ -109,6 +110,8 @@ QUnit.module('Play Middleware', {}, function() {
this.player.ads._shouldBlockPlay = true
assert.equal(m.callPlay(), this.videojs.middleware.TERMINATOR,
'callPlay returns terminator');
assert.strictEqual(this.player.ads._playBlocked, true,
'_playBlocked is set');
});

QUnit.test('playMiddleware callPlay will not terminate if _shouldBlockPlay is false', function(assert) {
Expand All @@ -121,6 +124,8 @@ QUnit.module('Play Middleware', {}, function() {
'callPlay should not return an object');
assert.notEqual(m.callPlay(), this.videojs.middleware.TERMINATOR,
'callPlay should not return the terminator');
assert.strictEqual(this.player.ads._playBlocked, false,
'_playBlocked should not be set');
});

QUnit.test("playMiddleware callPlay will not terminate if the player doesn't have this plugin", function(assert) {
Expand All @@ -139,18 +144,40 @@ QUnit.module('Play Middleware', {}, function() {

assert.equal(m.callPlay(), undefined,
'callPlay should not return an object');
assert.strictEqual(this.player.ads._playBlocked, false,
'_playBlocked should not be set');
});

QUnit.test('playMiddleware play will trigger play event if callPlay terminates', function(assert) {
const m = pm.playMiddleware(this.player);

this.sandbox.stub(pm, 'isMiddlewareMediatorSupported').returns(true);
this.player.ads._shouldBlockPlay = true;
// Mock that the callPlay method terminated
this.player.ads._playBlocked = true;

// Play terminates, there's no value returned
m.play(true, null);
assert.equal(this.triggeredEvent, 'play');
assert.equal(this.addedClass, 'vjs-has-started');
assert.equal(this.player.ads._playBlocked, false,
'_playBlocked is reset');
});

QUnit.test("playMiddleware play will not trigger play event if another middleware terminated", function(assert) {
const m = pm.playMiddleware(this.player);

this.sandbox.stub(pm, 'isMiddlewareMediatorSupported').returns(true);
// Mock that another middleware terminated but the playMiddleware did not
this.player.ads._shouldBlockPlay = true;
this.player.ads._playBlocked = false;
this.sandbox.stub(m, 'callPlay').returns(undefined);

// Another middleware terminated so the first argument is true
m.play(true, null);
assert.equal(this.triggeredEvent, null, 'no events should be triggered');
assert.equal(this.addedClass, null, 'no classes should be added');
assert.equal(this.player.ads._playBlocked, false, '_playBlocked has not changed');
});

QUnit.test("playMiddleware play will not trigger play event if the player doesn't have this plugin", function(assert) {
Expand All @@ -167,7 +194,6 @@ QUnit.module('Play Middleware', {}, function() {
const m = pm.playMiddleware(nonAdsPlayer);

this.sandbox.stub(pm, 'isMiddlewareMediatorSupported').returns(true);
this.player.ads._shouldBlockPlay = true;

m.play(true, null);
assert.equal(evt, null, 'the play event should not have been triggered');
Expand All @@ -176,11 +202,14 @@ QUnit.module('Play Middleware', {}, function() {

QUnit.test("playMiddleware won't trigger play event if callPlay doesn't terminate", function(assert) {
const m = pm.playMiddleware(this.player);
const originalPlayBlocked = this.player.ads._playBlocked;

this.sandbox.stub(pm, 'isMiddlewareMediatorSupported').returns(true);
m.play(false, {});
assert.equal(this.triggeredEvent, null, 'no events should be triggered');
assert.equal(this.addedClass, null, 'no classes should be added');
assert.strictEqual(this.player.ads._playBlocked, originalPlayBlocked,
'_playBlocked remains unchanged');
});

});
Expand Down

0 comments on commit f9762bf

Please sign in to comment.