Skip to content

Commit

Permalink
fix: Address iOS playsinline flash of BPB + poster (#360)
Browse files Browse the repository at this point in the history
* address ios content flash

* fix tests

* vjs-waiting fix
  • Loading branch information
incompl authored Apr 5, 2018
1 parent da8ea1a commit 33de864
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 19 deletions.
6 changes: 6 additions & 0 deletions src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ const contribAdsPlugin = function(options) {
}
});

// video.js removes the vjs-waiting class on timeupdate. We want
// to make sure this still happens during content restoration.
player.on('contenttimeupdate', function() {
player.removeClass('vjs-waiting');
});

// We now auto-play when an ad gets loaded if we're playing ads in the same video
// element as the content.
// The problem is that in IE11, we cannot play in addurationchange but in iOS8, we
Expand Down
10 changes: 10 additions & 0 deletions src/plugin.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,13 @@ Styles for ad integrations.
-webkit-animation-delay: 0.44s;
animation-delay: 0.44s;
}

/*
Hide BPB and poster while ad loading and resuming after an ad.
*/
.vjs-ad-loading .vjs-big-play-button,
.vjs-ad-loading .vjs-poster,
.vjs-ad-content-resuming .vjs-big-play-button,
.vjs-ad-content-resuming .vjs-poster {
display: none;
}
19 changes: 19 additions & 0 deletions src/states/Midroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ export default class Midroll extends AdState {
init(player) {
player.ads.adType = 'midroll';
adBreak.start(player);
player.addClass('vjs-ad-loading');
}

/*
* An ad has actually started playing.
* Remove the loading spinner.
*/
onAdStarted(player) {
player.removeClass('vjs-ad-loading');
}

/*
Expand All @@ -27,6 +36,8 @@ export default class Midroll extends AdState {

if (this.inAdBreak()) {
this.contentResuming = true;
player.addClass('vjs-ad-content-resuming');
player.removeClass('vjs-ad-loading');
adBreak.end(player);
}
}
Expand All @@ -43,4 +54,12 @@ export default class Midroll extends AdState {
}
}

/*
* Cleanup CSS classes.
*/
cleanup(player) {
player.removeClass('vjs-ad-loading');
player.removeClass('vjs-ad-content-resuming');
}

}
26 changes: 14 additions & 12 deletions src/states/Postroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default class Postroll extends AdState {

// No postroll, ads are done
} else {
this.contentResuming = true;
this.resumeContent(player);
this.transitionTo(AdsDone);
}
}
Expand Down Expand Up @@ -73,7 +73,7 @@ export default class Postroll extends AdState {

if (this.inAdBreak()) {
player.removeClass('vjs-ad-loading');
this.contentResuming = true;
this.resumeContent(player);
adBreak.end(player, () => {
this.transitionTo(AdsDone);
});
Expand All @@ -91,7 +91,7 @@ export default class Postroll extends AdState {
} else {
player.ads.debug('Postroll abort (skipLinearAdMode)');
player.trigger('adskip');
this.abort();
this.abort(player);
}
}

Expand All @@ -100,7 +100,7 @@ export default class Postroll extends AdState {
*/
onAdTimeout(player) {
player.ads.debug('Postroll abort (adtimeout)');
this.abort();
this.abort(player);
}

/*
Expand All @@ -115,7 +115,7 @@ export default class Postroll extends AdState {
if (player.ads.inAdBreak()) {
player.ads.endLinearAdMode();
} else {
this.abort();
this.abort(player);
}
}

Expand Down Expand Up @@ -146,24 +146,26 @@ export default class Postroll extends AdState {
}
}

resumeContent(player) {
this.contentResuming = true;
player.addClass('vjs-ad-content-resuming');
}

/*
* Helper for ending Postrolls. In the future we may want to
* refactor this class so that `cleanup` handles all of this.
*/
abort() {
const player = this.player;

this.contentResuming = true;
abort(player) {
this.resumeContent(player);
player.removeClass('vjs-ad-loading');
this.transitionTo(AdsDone);
}

/*
* Cleanup timeouts and state.
*/
cleanup() {
const player = this.player;

cleanup(player) {
player.removeClass('vjs-ad-content-resuming');
player.clearTimeout(this._postrollTimeout);
player.ads._contentEnding = false;
}
Expand Down
6 changes: 3 additions & 3 deletions src/states/Preroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export default class Preroll extends AdState {

if (this.inAdBreak()) {
player.removeClass('vjs-ad-loading');
player.addClass('vjs-ad-content-resuming');
adBreak.end(player);
this.contentResuming = true;
}
Expand Down Expand Up @@ -228,14 +229,13 @@ export default class Preroll extends AdState {
/*
* Cleanup timeouts and spinner.
*/
cleanup() {
const player = this.player;

cleanup(player) {
if (!player.ads._hasThereBeenALoadStartDuringPlayerLife) {
videojs.log.warn('Leaving Preroll state before loadstart event can cause issues.');
}

player.removeClass('vjs-ad-loading');
player.removeClass('vjs-ad-content-resuming');
player.clearTimeout(this._timeout);
}

Expand Down
2 changes: 1 addition & 1 deletion src/states/abstract/State.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default class State {
const player = this.player;
const previousState = this;

previousState.cleanup();
previousState.cleanup(player);
const newState = new NewState(player);

player.ads._state = newState;
Expand Down
2 changes: 2 additions & 0 deletions test/unit/states/test.Midroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import adBreak from '../../../src/adBreak.js';
QUnit.module('Midroll', {
beforeEach: function() {
this.player = {
addClass: () => {},
removeClass: () => {},
ads: {
_inLinearAdMode: true,
endLinearAdMode: () => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/states/test.Postroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ QUnit.test('can abort', function(assert) {
const removeClassSpy = sinon.spy(this.player, 'removeClass');

this.postroll.init(this.player);
this.postroll.abort();
this.postroll.abort(this.player);
assert.equal(this.postroll.contentResuming, true, 'contentResuming');
assert.ok(removeClassSpy.calledWith('vjs-ad-loading'), 'loading class removed');
});
Expand All @@ -138,7 +138,7 @@ QUnit.test('can clean up', function(assert) {
const clearSpy = sinon.spy(this.player, 'clearTimeout');

this.postroll.init(this.player);
this.postroll.cleanup();
this.postroll.cleanup(this.player);
assert.equal(this.player.ads._contentEnding, false, '_contentEnding');
assert.ok(clearSpy.calledWith(this.postroll._postrollTimeout), 'cleared timeout');
});
2 changes: 1 addition & 1 deletion test/unit/states/test.Preroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,6 @@ QUnit.test('remove ad loading class on cleanup', function(assert) {

const removeClassSpy = sinon.spy(this.player, 'removeClass');

this.preroll.cleanup();
this.preroll.cleanup(this.player);
assert.ok(removeClassSpy.calledWith('vjs-ad-loading'), 'loading class removed');
});

0 comments on commit 33de864

Please sign in to comment.