From ef30ea59e0f63e1d8eb011932aa7db69df68be41 Mon Sep 17 00:00:00 2001 From: Andrey Polischuk Date: Tue, 6 Apr 2021 02:08:20 +0300 Subject: [PATCH] fix: fix viewable impression by vast spec --- src/adUnit/VideoAdUnit.js | 97 +++++++------ src/adUnit/__tests__/VideoAdUnit.spec.js | 175 +++++++++++++---------- 2 files changed, 152 insertions(+), 120 deletions(-) diff --git a/src/adUnit/VideoAdUnit.js b/src/adUnit/VideoAdUnit.js index d0b1d82f..89207c7a 100644 --- a/src/adUnit/VideoAdUnit.js +++ b/src/adUnit/VideoAdUnit.js @@ -2,10 +2,7 @@ import {linearEvents} from '../tracker'; import {getViewable} from '../vastSelectors'; import {finish} from './adUnitEvents'; -import { - onElementVisibilityChange, - onElementResize -} from './helpers/dom/elementObservers'; +import {onElementVisibilityChange, onElementResize} from './helpers/dom/elementObservers'; import preventManualProgress from './helpers/dom/preventManualProgress'; import Emitter from './helpers/Emitter'; import retrieveIcons from './helpers/icons/retrieveIcons'; @@ -13,13 +10,7 @@ import addIcons from './helpers/icons/addIcons'; import viewmode from './helpers/vpaid/viewmode'; import safeCallback from './helpers/safeCallback'; -const { - start, - viewable, - viewUndetermined, - iconClick, - iconView -} = linearEvents; +const {start, viewable, notViewable, viewUndetermined, iconClick, iconView} = linearEvents; // eslint-disable-next-line id-match export const _protected = Symbol('_protected'); @@ -43,6 +34,14 @@ class VideoAdUnit extends Emitter { }); }, finished: false, + handleViewableImpression: (event) => { + this[_protected].viewable = event; + + this.emit(event, { + adUnit: this, + type: event + }); + }, onErrorCallbacks: [], onFinishCallbacks: [], started: false, @@ -58,7 +57,7 @@ class VideoAdUnit extends Emitter { }; /** Ad unit type */ - type=null; + type = null; /** If an error occurs it will contain the reference to the error otherwise it will be bull */ error = null; @@ -82,9 +81,7 @@ class VideoAdUnit extends Emitter { constructor (vastChain, videoAdContainer, {viewability = false, responsive = false, logger = console} = {}) { super(logger); - const { - onFinishCallbacks - } = this[_protected]; + const {onFinishCallbacks, handleViewableImpression} = this[_protected]; /** Reference to the {@link VastChain} used to load the ad. */ this.vastChain = vastChain; @@ -98,22 +95,20 @@ class VideoAdUnit extends Emitter { onFinishCallbacks.push(preventManualProgress(this.videoAdContainer.videoElement)); if (this.icons) { - const { - drawIcons, - hasPendingIconRedraws, - removeIcons - } = addIcons(this.icons, { + const {drawIcons, hasPendingIconRedraws, removeIcons} = addIcons(this.icons, { logger, - onIconClick: (icon) => this.emit(iconClick, { - adUnit: this, - data: icon, - type: iconClick - }), - onIconView: (icon) => this.emit(iconView, { - adUnit: this, - data: icon, - type: iconView - }), + onIconClick: (icon) => + this.emit(iconClick, { + adUnit: this, + data: icon, + type: iconClick + }), + onIconView: (icon) => + this.emit(iconView, { + adUnit: this, + data: icon, + type: iconView + }), videoAdContainer }); @@ -128,24 +123,38 @@ class VideoAdUnit extends Emitter { if (viewableImpression) { this.once(start, () => { - const unsubscribe = onElementVisibilityChange(this.videoAdContainer.element, (visible) => { - if (this.isFinished()) { - return; - } + let timeoutId; - if (visible !== false && !this[_protected].viewable) { - const status = visible ? viewable : viewUndetermined; + const unsubscribe = onElementVisibilityChange( + this.videoAdContainer.element, + (visible) => { + if (this.isFinished() || this[_protected].viewable) { + return; + } - this.emit(status, { - adUnit: this, - type: status - }); + if (typeof visible !== 'boolean') { + handleViewableImpression(viewUndetermined); - this[_protected].viewable = status; - } - }, {viewabilityOffset: 0.5}); + return; + } - onFinishCallbacks.push(unsubscribe); + if (visible) { + timeoutId = setTimeout(handleViewableImpression, 2000, viewable); + } else { + clearTimeout(timeoutId); + } + }, + {viewabilityOffset: 0.5} + ); + + onFinishCallbacks.push(() => { + unsubscribe(); + clearTimeout(timeoutId); + + if (!this[_protected].viewable) { + handleViewableImpression(notViewable); + } + }); }); } diff --git a/src/adUnit/__tests__/VideoAdUnit.spec.js b/src/adUnit/__tests__/VideoAdUnit.spec.js index 2f2a5bf6..f129674c 100644 --- a/src/adUnit/__tests__/VideoAdUnit.spec.js +++ b/src/adUnit/__tests__/VideoAdUnit.spec.js @@ -1,24 +1,11 @@ /* eslint-disable no-loop-func, max-nested-callbacks */ -import { - vpaidInlineAd, - vpaidInlineParsedXML, - vastVpaidInlineXML -} from '../../../fixtures'; +import {vpaidInlineAd, vpaidInlineParsedXML, vastVpaidInlineXML} from '../../../fixtures'; import VideoAdContainer from '../../adContainer/VideoAdContainer'; -import { - iconClick, - iconView, - start, - viewable, - viewUndetermined -} from '../../tracker/linearEvents'; +import {iconClick, iconView, notViewable, start, viewable, viewUndetermined} from '../../tracker/linearEvents'; import {getViewable} from '../../vastSelectors'; import addIcons from '../helpers/icons/addIcons'; import retrieveIcons from '../helpers/icons/retrieveIcons'; -import { - onElementResize, - onElementVisibilityChange -} from '../helpers/dom/elementObservers'; +import {onElementResize, onElementVisibilityChange} from '../helpers/dom/elementObservers'; import VideoAdUnit, {_protected} from '../VideoAdUnit'; import {finish} from '../adUnitEvents'; import preventManualProgress from '../helpers/dom/preventManualProgress'; @@ -28,13 +15,11 @@ const mockRemoveIcons = jest.fn(); const mockHasPendingRedraws = jest.fn(() => false); jest.mock('../helpers/icons/addIcons.js', () => - jest.fn( - () => ({ - drawIcons: mockDrawIcons, - hasPendingIconRedraws: mockHasPendingRedraws, - removeIcons: mockRemoveIcons - }) - ) + jest.fn(() => ({ + drawIcons: mockDrawIcons, + hasPendingIconRedraws: mockHasPendingRedraws, + removeIcons: mockRemoveIcons + })) ); jest.mock('../helpers/icons/retrieveIcons.js'); jest.mock('../helpers/dom/elementObservers', () => ({ @@ -104,12 +89,14 @@ describe('VideoAdUnit', () => { describe('icons', () => { test('must be added to the class instance', () => { - const icons = [{ - height: 20, - width: 20, - xPosition: 'left', - yPosition: 'top' - }]; + const icons = [ + { + height: 20, + width: 20, + xPosition: 'left', + yPosition: 'top' + } + ]; retrieveIcons.mockReturnValue(icons); const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); @@ -125,12 +112,14 @@ describe('VideoAdUnit', () => { }); test('must remove the icons on ad finish', () => { - const icons = [{ - height: 20, - width: 20, - xPosition: 'left', - yPosition: 'top' - }]; + const icons = [ + { + height: 20, + width: 20, + xPosition: 'left', + yPosition: 'top' + } + ]; retrieveIcons.mockReturnValue(icons); const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); @@ -143,12 +132,14 @@ describe('VideoAdUnit', () => { test(`must emit '${iconClick}' event on click`, async () => { addIcons.mockClear(); - const icons = [{ - height: 20, - width: 20, - xPosition: 'left', - yPosition: 'top' - }]; + const icons = [ + { + height: 20, + width: 20, + xPosition: 'left', + yPosition: 'top' + } + ]; retrieveIcons.mockReturnValue(icons); const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); @@ -167,21 +158,25 @@ describe('VideoAdUnit', () => { const passedArgs = await promise; - expect(passedArgs).toEqual([{ - adUnit, - data: icons[0], - type: iconClick - }]); + expect(passedArgs).toEqual([ + { + adUnit, + data: icons[0], + type: iconClick + } + ]); }); test(`must emit '${iconView}' event on view`, async () => { addIcons.mockClear(); - const icons = [{ - height: 20, - width: 20, - xPosition: 'left', - yPosition: 'top' - }]; + const icons = [ + { + height: 20, + width: 20, + xPosition: 'left', + yPosition: 'top' + } + ]; retrieveIcons.mockReturnValue(icons); const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); @@ -200,26 +195,18 @@ describe('VideoAdUnit', () => { const passedArgs = await promise; - expect(passedArgs).toEqual([{ - adUnit, - data: icons[0], - type: iconView - }]); + expect(passedArgs).toEqual([ + { + adUnit, + data: icons[0], + type: iconView + } + ]); }); }); describe('method', () => { - [ - 'start', - 'resume', - 'pause', - 'paused', - 'duration', - 'currentTime', - 'setVolume', - 'getVolume', - 'cancel' - ].forEach((method) => { + ['start', 'resume', 'pause', 'paused', 'duration', 'currentTime', 'setVolume', 'getVolume', 'cancel'].forEach((method) => { test(`${method} must throw if called`, () => { const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); @@ -319,12 +306,14 @@ describe('VideoAdUnit', () => { }); test('must redraw the icons', async () => { - const icons = [{ - height: 20, - width: 20, - xPosition: 'left', - yPosition: 'top' - }]; + const icons = [ + { + height: 20, + width: 20, + xPosition: 'left', + yPosition: 'top' + } + ]; retrieveIcons.mockReturnValue(icons); adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); @@ -521,7 +510,7 @@ describe('VideoAdUnit', () => { }); }); - test('must emit viewable on visibility change if true', () => { + test('must emit viewable when the ad unit meets criteria for a viewable impression', async () => { const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); jest.spyOn(adUnit, 'emit'); @@ -530,13 +519,26 @@ describe('VideoAdUnit', () => { expect(onElementVisibilityChange).toHaveBeenCalledTimes(1); expect(adUnit.emit).not.toHaveBeenCalledWith(viewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(notViewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(viewUndetermined); + simulateVisibilityChange(false); expect(adUnit.emit).not.toHaveBeenCalledWith(viewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(notViewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(viewUndetermined); + + const eventPromise = new Promise((resolve) => adUnit.on(viewable, resolve)); + simulateVisibilityChange(true); + + await eventPromise; + expect(adUnit.emit).toHaveBeenCalledWith(viewable, expect.any(Object)); + expect(adUnit.emit).not.toHaveBeenCalledWith(notViewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(viewUndetermined); }); - test('must emit viewUndetermined if visibility is not determined', () => { + test('must emit viewUndetermined if cannot be determined whether the ad unit meets criteria for a viewable impression', () => { const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); jest.spyOn(adUnit, 'emit'); @@ -545,12 +547,33 @@ describe('VideoAdUnit', () => { expect(onElementVisibilityChange).toHaveBeenCalledTimes(1); expect(adUnit.emit).not.toHaveBeenCalledWith(viewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(notViewable); expect(adUnit.emit).not.toHaveBeenCalledWith(viewUndetermined); + simulateVisibilityChange(undefined); expect(adUnit.emit).not.toHaveBeenCalledWith(viewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(notViewable); expect(adUnit.emit).toHaveBeenCalledWith(viewUndetermined, expect.any(Object)); }); + test('must emit notViewable on finish if ad unit never meets criteria for a viewable impression', () => { + const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer); + + jest.spyOn(adUnit, 'emit'); + expect(onElementVisibilityChange).not.toHaveBeenCalled(); + adUnit.emit(start); + expect(onElementVisibilityChange).toHaveBeenCalledTimes(1); + + expect(adUnit.emit).not.toHaveBeenCalledWith(viewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(notViewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(viewUndetermined); + + adUnit[_protected].finish(); + expect(adUnit.emit).not.toHaveBeenCalledWith(viewable); + expect(adUnit.emit).not.toHaveBeenCalledWith(viewUndetermined); + expect(adUnit.emit).toHaveBeenCalledWith(notViewable, expect.any(Object)); + }); + test('must do nothing if finished', () => { const adUnit = new VideoAdUnit(vpaidChain, videoAdContainer);