Merge pull request #107 from MailOnline/next-release

Next release
MailOnline · Dec 5, 2018 · 113895f · 113895f
2 parents abe414e + 1825a16
commit 113895f
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 30 deletions.
diff --git a/src/adContainer/helpers/__tests__/createIframe.spec.js b/src/adContainer/helpers/__tests__/createIframe.spec.js
@@ -9,7 +9,6 @@ describe('createIframe', () => {
       url: 'https://www.example.com/'
     });
 
-    // TODO: Test srcdoc once jsdom supports it. See https://github.com/jsdom/jsdom/pull/2389 for more info
     supportsSrcdoc.mockReturnValue(false);
   });
 
@@ -37,7 +36,7 @@ describe('createIframe', () => {
     try {
       await createIframe(document.createElement('div'), test);
     } catch (error) {
-      expect(error.message).toBe('Error creating iframe, the placeholder is probably not in the DOM');
+      expect(error).toBeInstanceOf(Error);
     }
   });
 });
diff --git a/src/adContainer/helpers/createIframe.js b/src/adContainer/helpers/createIframe.js
@@ -1,4 +1,3 @@
-import defer from '../../utils/defer';
 import getContentDocument from './getContentDocument';
 import getOrigin from './getOrigin';
 import supportsSrcdoc from './supportsSrcdoc';
@@ -11,10 +10,8 @@ const iframeContent = (id, targetOrigin) => `<!DOCTYPE html>
   </body>
 </html>`;
 
-const createIframe = (placeholder, id) => {
-  const deferred = defer();
+const createBaseIframe = () => {
   const iframe = document.createElement('IFRAME');
-  const content = iframeContent(id, getOrigin());
 
   iframe.sandbox = 'allow-forms allow-popups allow-scripts allow-same-origin';
   iframe.style.margin = '0';
@@ -24,35 +21,43 @@ const createIframe = (placeholder, id) => {
   iframe.style.height = '0';
   iframe.style.position = 'absolute';
 
-  if (supportsSrcdoc()) {
-    iframe.src = 'about:srcdoc';
-    iframe.srcdoc = content;
-    placeholder.appendChild(iframe);
-  } else {
-    iframe.src = 'about:blank';
-    placeholder.appendChild(iframe);
+  return iframe;
+};
 
-    const iframeDocument = getContentDocument(iframe);
+const createIframe = (placeholder, id) => new Promise((resolve, reject) => {
+  const content = iframeContent(id, getOrigin());
+  let iframe;
 
-    if (iframeDocument) {
-      iframeDocument.write(content);
+  /*
+    NOTE: favor about:blank instead of srcdoc because some browsers
+  */
+  try {
+    iframe = createBaseIframe();
+    iframe.src = 'about:blank';
+    placeholder.appendChild(iframe);
+    getContentDocument(iframe).write(content);
+  } catch (error) {
+    placeholder.removeChild(iframe);
+
+    if (supportsSrcdoc()) {
+      iframe = createBaseIframe();
+      iframe.src = 'about:srcdoc';
+      iframe.srcdoc = content;
+      placeholder.appendChild(iframe);
     } else {
-      placeholder.removeChild(iframe);
-      deferred.reject(new Error('Error creating iframe, the placeholder is probably not in the DOM'));
+      reject(error);
     }
   }
 
   const handleMessage = ({data}) => {
     /* istanbul ignore else */
     if (data === `${id}_ready`) {
       window.removeEventListener('message', handleMessage);
-      deferred.resolve(iframe);
+      resolve(iframe);
     }
   };
 
   window.addEventListener('message', handleMessage, false);
-
-  return deferred.promise;
-};
+});
 
 export default createIframe;
diff --git a/src/adUnit/VpaidAdUnit.js b/src/adUnit/VpaidAdUnit.js
@@ -3,8 +3,7 @@ import linearEvents from '../tracker/linearEvents';
 import {
   acceptInvitation,
   creativeView,
-  adCollapse,
-  close
+  adCollapse
 } from '../tracker/nonLinearEvents';
 import {getClickThrough} from '../vastSelectors';
 import {volumeChanged, adProgress} from './adUnitEvents';
@@ -61,7 +60,8 @@ const {
   midpoint,
   thirdQuartile,
   clickThrough,
-  error: errorEvt
+  error: errorEvt,
+  closeLinear
 } = linearEvents;
 
 // eslint-disable-next-line id-match
@@ -170,10 +170,12 @@ class VpaidAdUnit extends VideoAdUnit {
         });
       },
       [adUserClose]: () => {
-        this.emit(close, {
+        this.emit(closeLinear, {
           adUnit: this,
-          type: close
+          type: closeLinear
         });
+
+        this[_protected].finish();
       },
       [adUserMinimize]: () => {
         this.emit(adCollapse, {

diff --git a/src/adUnit/__tests__/VpaidAdUnit.spec.js b/src/adUnit/__tests__/VpaidAdUnit.spec.js
@@ -54,13 +54,13 @@ import linearEvents, {
   resume,
   clickThrough,
   iconClick,
+  closeLinear,
   iconView
 } from '../../tracker/linearEvents';
 import {
   acceptInvitation,
   creativeView,
-  adCollapse,
-  close
+  adCollapse
 } from '../../tracker/nonLinearEvents';
 import addIcons from '../helpers/icons/addIcons';
 import retrieveIcons from '../helpers/icons/retrieveIcons';
@@ -609,6 +609,20 @@ describe('VpaidAdUnit', () => {
 
         expect(callback).toHaveBeenCalledTimes(1);
       });
+
+      test('must be called if the user closes the ad unit', async () => {
+        const callback = jest.fn();
+
+        adUnit.onFinish(callback);
+
+        await adUnit.start();
+
+        expect(callback).not.toHaveBeenCalled();
+
+        adUnit.creativeAd.emit(adUserClose);
+
+        expect(callback).toHaveBeenCalledTimes(1);
+      });
     });
 
     describe('onError', () => {
@@ -747,7 +761,7 @@ describe('VpaidAdUnit', () => {
         vpaidEvt: adUserMinimize
       },
       {
-        vastEvt: close,
+        vastEvt: closeLinear,
         vpaidEvt: adUserClose
       },
       {

diff --git a/src/tracker/linearEvents.js b/src/tracker/linearEvents.js
@@ -165,8 +165,16 @@ export const iconClick = 'iconClick';
  */
 export const iconView = 'iconView';
 
+/**
+ * The viewer has chosen to close the linear ad unit. This is currently inuse by some of the largest mobile SDKs to mark the dismissal of the end card companion that follows the video, as well as a close of the video itself, if applicable
+ *
+ * @event LinearEvents#closeLinear
+ */
+export const closeLinear = 'closeLinear';
+
 const linearEvents = {
   clickThrough,
+  closeLinear,
   complete,
   error,
   exitFullscreen,