Skip to content

Commit

Permalink
Merge pull request #102 from MailOnline/fix/QA_fixes
Browse files Browse the repository at this point in the history
Fix/qa fixes
  • Loading branch information
carpasse authored Nov 26, 2018
2 parents 8418048 + ec15af0 commit ff1cff7
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 474 deletions.
460 changes: 0 additions & 460 deletions CHANGELOG.md

This file was deleted.

6 changes: 1 addition & 5 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Video Ad SDK
[![Build Status](https://api.travis-ci.org/MailOnline/mol-video-ad-sdk.svg?branch=master)](https://travis-ci.org/MailOnline/mol-video-ad-sdk) [![GitHub license](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/MailOnline/mol-video-ad-sdk/blob/master/LICENSE) [![semantic-release](https://img.shields.io/badge/%20%20%F0%9F%93%A6%F0%9F%9A%80-semantic--release-e10079.svg)](https://github.com/semantic-release/semantic-release) [![codecov](https://codecov.io/gh/MailOnline/mol-video-ad-sdk/branch/master/graph/badge.svg)](https://codecov.io/gh/MailOnline/mol-video-ad-sdk)[![Known Vulnerabilities](https://snyk.io/test/github/MailOnline/mol-video-ad-sdk/badge.svg?targetFile=package.json)](https://snyk.io/test/github/MailOnline/mol-video-ad-sdk?targetFile=package.json)

[![Build Status](https://api.travis-ci.org/MailOnline/mol-video-ad-sdk.svg?branch=master)](https://travis-ci.org/MailOnline/mol-video-ad-sdk) [![GitHub license](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/MailOnline/mol-video-ad-sdk/blob/master/LICENSE) [![semantic-release](https://img.shields.io/badge/%20%20%F0%9F%93%A6%F0%9F%9A%80-semantic--release-e10079.svg)](https://github.com/semantic-release/semantic-release) [![codecov](https://codecov.io/gh/MailOnline/mol-video-ad-sdk/branch/master/graph/badge.svg)](https://codecov.io/gh/MailOnline/mol-video-ad-sdk) [![Known Vulnerabilities](https://snyk.io/test/github/MailOnline/mol-video-ad-sdk/badge.svg?targetFile=package.json)](https://snyk.io/test/github/MailOnline/mol-video-ad-sdk?targetFile=package.json)

To run video ads in the browser there are many alternatives. The most famous one is probably Google's [IMA SDK](https://developers.google.com/interactive-media-ads/docs/sdks/html5/) for HTML5. There are two main cons with that SDK. It only works through DoubleClick and it is a black box very hard to debug and to maintain. This SDK tries to offer an alternative to play video ads that can work with any player in the world and any ad server that supports the VAST specification. And since it is open source you can read the code and debug if you need to.

Expand Down Expand Up @@ -28,10 +27,7 @@ $ yarn test
to run the tests.

## Discussion

Please open an issue if you have any questions or concerns.


## License

This project is licensed under the MIT license, Copyright (c) 2018 MailOnline. For more information see [LICENSE](./LICENSE)
7 changes: 3 additions & 4 deletions demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ <h1 class="title main">MOL Video Ad SDK Suite Inspector</h1>
<video
controls
playsinline="true"
poster="https://static.fsf.org/nosvn/FSF30-video/fsf30-poster.png"
src="https://static.fsf.org/nosvn/FSF30-video/FSF_30_360p.webm"
poster="https://mango.blender.org/wp-content/gallery/final-renders/graded_edit_001679.jpg"
src="https://videos.dailymail.co.uk/video/mol/tears_of_steel_720p.mp4"
width="640"
height="360">
Sorry, your browser doesn't support embedded videos.
Expand All @@ -31,8 +31,7 @@ <h1 class="title main">MOL Video Ad SDK Suite Inspector</h1>
</div>
</section>
<section>
<p><a rel="license" href="http://creativecommons.org/licenses/by-sa/4.0/"><img alt="Creative Commons License" style="border-width:0" src="https://i.creativecommons.org/l/by-sa/4.0/88x31.png"></a> <a href="https://www.fsf.org/blogs/community/user-liberation-watch-and-share-our-new-video"><em>User Liberation</em></a> by the <a href="http://www.fsf.org">Free Software Foundation</a> is licensed under the <a rel="license" href="http://creativecommons.org/licenses/by-sa/4.0/">Creative Commons Attribution-ShareAlike 4.0 International License</a>.</p>
<p><i>For more info please visit: <a href="https://u.fsf.org/user-liberation">https://u.fsf.org/user-liberation</a></i></p>
<p><a rel="license" href="http://creativecommons.org/licenses/by-sa/3.0/"><img alt="Creative Commons License" style="border-width:0" src="https://i.creativecommons.org/l/by-sa/3.0/88x31.png"></a> <a href="https://mango.blender.org/sharing/"></a><em>Tears of Steel</em></a> by the (CC) Blender Foundation | </strong><strong><a href="https://mango.blender.org/">mango.blender.org</a></strong> is licensed under the <a rel="license" href="http://creativecommons.org/licenses/by-sa/3.0/">Creative Commons Attribution-ShareAlike 3.0 Unported</a>.</p>
</section>
</div>
</body>
Expand Down
3 changes: 1 addition & 2 deletions demo/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ document.addEventListener('DOMContentLoaded', () => {
};
const onError = (evt) => {
console.log('### onError', evt);
console.error(adUnit.error);
};
const onRunFinish = (evt) => {
console.log('### onRunFinish', evt);
Expand All @@ -52,7 +51,7 @@ document.addEventListener('DOMContentLoaded', () => {
videoElement.addEventListener('contentloadedmetadata', resumeContent);
videoElement.addEventListener('canplay', resumeContent);
videoElement.src = src;
videoElement.play();
videoElement.load();
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/adUnit/VpaidAdUnit.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,12 @@ class VpaidAdUnit extends VideoAdUnit {
});
},
[adVideoComplete]: () => {
this[_protected].finish();

this.emit(complete, {
adUnit: this,
type: complete
});

this[_protected].finish();
},
[adVideoFirstQuartile]: () => {
this.emit(firstQuartile, {
Expand Down
52 changes: 52 additions & 0 deletions src/runner/__tests__/runWaterfall.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import VideoAdContainer from '../../adContainer/VideoAdContainer';
import VastAdUnit from '../../adUnit/VastAdUnit';
import {trackError} from '../../tracker';
import {_protected} from '../../adUnit/VideoAdUnit';
import isIOS from '../../utils/isIOS';

jest.mock('../../utils/isIOS');
jest.mock('../../vastRequest/requestAd', () => jest.fn());
jest.mock('../../vastRequest/requestNextAd', () => jest.fn());
jest.mock('../run', () => jest.fn());
Expand All @@ -34,6 +36,7 @@ describe('runWaterfall', () => {
let adUnit;

beforeEach(() => {
isIOS.mockReturnValue(false);
adTag = 'https://test.example.com/adtag';
vastAdChain = [
{
Expand Down Expand Up @@ -64,6 +67,55 @@ describe('runWaterfall', () => {
jest.resetAllMocks();
});

describe('videoElement', () => {
it('must call load synchronously for iOS devices if you pass the video element', () => {
const {videoElement} = adContainer;

Object.defineProperty(videoElement, 'load', {
value: jest.fn()
});
isIOS.mockReturnValue(true);

runWaterfall(adTag, placeholder, {
...options,
videoElement
});

expect(videoElement.load).toHaveBeenCalledTimes(1);
});

it('must not call load synchronously for iOS devices if you don\'t pass the video element', () => {
const {videoElement} = adContainer;

Object.defineProperty(videoElement, 'load', {
value: jest.fn()
});
isIOS.mockReturnValue(true);

runWaterfall(adTag, placeholder, {
...options
});

expect(videoElement.load).toHaveBeenCalledTimes(0);
});

it('must not call load if the device is not iOS', () => {
const {videoElement} = adContainer;

Object.defineProperty(videoElement, 'load', {
value: jest.fn()
});
isIOS.mockReturnValue(false);

runWaterfall(adTag, placeholder, {
...options,
videoElement
});

expect(videoElement.load).toHaveBeenCalledTimes(0);
});
});

describe('after fetching Vast response', () => {
test('must call onError if Vast response is undefined', async () => {
const onError = jest.fn();
Expand Down
9 changes: 8 additions & 1 deletion src/runner/runWaterfall.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import {trackError} from '../tracker';
import requestAd from '../vastRequest/requestAd';
import requestNextAd from '../vastRequest/requestNextAd';
import isIOS from '../utils/isIOS';
import run from './run';

const validateVastChain = (vastChain, options) => {
Expand Down Expand Up @@ -152,7 +153,6 @@ const runWaterfall = (adTag, placeholder, options) => {
adUnit = newAdUnit;
onAdStartHandler(adUnit);
};

const opts = {
...options,
onAdReady: callbackHandler(options.onAdReady),
Expand All @@ -161,6 +161,13 @@ const runWaterfall = (adTag, placeholder, options) => {
onRunFinish: callbackHandler(options.onRunFinish)
};

if (options.videoElement && isIOS()) {
/*
It seems that if the video doesn't load synchronously inside a touchend or click event handler, the user gesture breaks on iOS and it won't allow a play.
*/
options.videoElement.load();
}

waterfall(
() => requestAd(adTag, opts),
placeholder,
Expand Down
55 changes: 55 additions & 0 deletions src/utils/__tests__/isIOS.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/* eslint-disable filenames/match-regex */
import isIOS from '../isIOS';

describe('isIOS', () => {
let origUserAgent;

beforeEach(() => {
origUserAgent = navigator.userAgent;

Object.defineProperty(navigator, 'userAgent', {
writable: true
});
});

afterEach(() => {
Object.defineProperty(navigator, 'userAgent', {
value: origUserAgent,
writable: true
});
});

it('must be a function', () => {
expect(isIOS).toBeInstanceOf(Function);
});

it('must return true if the useAgent contains iPad or iPod or iPhone', () => {
expect(isIOS()).toBe(false);

navigator.userAgent = `iPhone ${origUserAgent}`;
expect(isIOS()).toBe(true);

navigator.userAgent = `iPad ${origUserAgent}`;
expect(isIOS()).toBe(true);

navigator.userAgent = `iPod ${origUserAgent}`;
expect(isIOS()).toBe(true);
});

it('must return false for IE user agents that contain iPhone', () => {
window.MSStream = true;

expect(isIOS()).toBe(false);

navigator.userAgent = `iPhone ${origUserAgent}`;
expect(isIOS()).toBe(false);

navigator.userAgent = `iPad ${origUserAgent}`;
expect(isIOS()).toBe(false);

navigator.userAgent = `iPod ${origUserAgent}`;
expect(isIOS()).toBe(false);

delete window.MSStream;
});
});
4 changes: 4 additions & 0 deletions src/utils/isIOS.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/* eslint-disable filenames/match-regex */
const isIOS = () => /iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream;

export default isIOS;

0 comments on commit ff1cff7

Please sign in to comment.