diff --git a/extensions/amp-video-iframe/0.1/amp-video-iframe.js b/extensions/amp-video-iframe/0.1/amp-video-iframe.js index 88554a88a848..99c486fa4fc4 100644 --- a/extensions/amp-video-iframe/0.1/amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/amp-video-iframe.js @@ -80,6 +80,18 @@ const getAnalyticsEventTypePrefixRegex = once(() => new RegExp(`^${ANALYTICS_EVENT_TYPE_PREFIX}`)); +/** + * @param {string} src + * @return {string} + */ +function maybeAddAmpFragment(src) { + if (src.indexOf('#') > -1) { + return src; + } + return `${src}#amp=1`; +} + + /** @implements {../../../src/video-interface.VideoInterface} */ class AmpVideoIframe extends AMP.BaseElement { @@ -181,8 +193,8 @@ class AmpVideoIframe extends AMP.BaseElement { /** @override */ createPlaceholderCallback() { const {element} = this; - const poster = - htmlFor(element)``; + const html = htmlFor(element); + const poster = html``; poster.setAttribute('src', this.user().assertString(element.getAttribute('poster'))); @@ -226,7 +238,7 @@ class AmpVideoIframe extends AMP.BaseElement { element); } - return src; + return maybeAddAmpFragment(src); } /** diff --git a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js index 250c3bd07785..99c8865998f5 100644 --- a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js @@ -37,7 +37,7 @@ describes.realWin('amp-video-iframe', { extensions: ['amp-video-iframe'], }, }, env => { - + const {any} = sinon.match; const defaultFixture = 'video-iframe.html'; let win; @@ -62,22 +62,17 @@ describes.realWin('amp-video-iframe', { fixture || defaultFixture}`); } - function createVideoIframe(opt_size) { - const el = htmlFor(doc)``; - - if (opt_size) { - addAttributesToElement(el, { - 'layout': 'fixed', - 'width': opt_size[0], - 'height': opt_size[1], - }); - } else { - addAttributesToElement(el, { - 'layout': 'fill', - }); - } - - addAttributesToElement(el, {src: getIframeSrc(), poster: 'poster.html'}); + const layoutConfigAttrs = size => !size ? {layout: 'fill'} : { + layout: 'fixed', + width: size[0], + height: size[1], + }; + + function createVideoIframe({size, src} = {}) { + const html = htmlFor(doc); + const el = html``; + el.setAttribute('src', src || getIframeSrc()); + addAttributesToElement(el, layoutConfigAttrs(size)); doc.body.appendChild(el); return el; } @@ -91,7 +86,7 @@ describes.realWin('amp-video-iframe', { .returns(true); } - function whenLoaded(videoIframe) { + function layoutAndLoad(videoIframe) { return whenUpgradedToCustomElement(videoIframe).then(() => { videoIframe.implementation_.layoutCallback(); return listenOncePromise(videoIframe, VideoEvents.LOAD); @@ -109,8 +104,8 @@ describes.realWin('amp-video-iframe', { return entry; } - describe('#buildCallback', () => { - it('sets metadata', function* () { + describe('#layoutCallback', () => { + it('sets metadata in iframe name', async() => { const metadata = { canonicalUrl: 'foo.html', sourceUrl: 'bar.html', @@ -120,18 +115,35 @@ describes.realWin('amp-video-iframe', { const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const {name} = videoIframe.implementation_.iframe_; - // Sinon does not support sinon.match on to.equal - const dummySpy = sandbox.spy(); + expect(tryParseJson(name)).to.deep.equal(metadata); + }); + + it('sets amp=1 fragment in src', async() => { + const rawSrc = getIframeSrc(); + const videoIframe = createVideoIframe({src: rawSrc}); + + await layoutAndLoad(videoIframe); - dummySpy(tryParseJson(name)); + const {src} = videoIframe.implementation_.iframe_; + expect(src).to.equal(`${rawSrc}#amp=1`); + }); + + it('does not set amp=1 fragment in src when fragment present', async() => { + const rawSrc = `${getIframeSrc()}#my-fragment`; + const videoIframe = createVideoIframe({src: rawSrc}); + + await layoutAndLoad(videoIframe); - expect(dummySpy.withArgs(sinon.match(metadata))).to.have.been.calledOnce; + const {src} = videoIframe.implementation_.iframe_; + expect(src).to.equal(rawSrc); }); + }); + describe('#buildCallback', () => { it('rejects tracking iframes', () => { const trackingSizes = [ [10, 10], @@ -147,8 +159,10 @@ describes.realWin('amp-video-iframe', { ]; trackingSizes.forEach(size => { - const videoIframe = createVideoIframe(size); - expect(whenLoaded(videoIframe)).to.eventually.be.rejected; + const {implementation_} = createVideoIframe({size}); + allowConsoleError(() => { + expect(() => implementation_.buildCallback()).to.throw(); + }); }); }); }); @@ -170,10 +184,10 @@ describes.realWin('amp-video-iframe', { describe('#onMessage_', () => { - it('should load and register on canplay', function* () { + it('should load and register on canplay', async() => { // Fixture inside frame triggers `canplay`. const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const register = videoManagerStub.register.withArgs(videoIframe.implementation_); @@ -181,10 +195,10 @@ describes.realWin('amp-video-iframe', { expect(register).to.have.been.calledOnce; }); - it('should not dispatch invalid events', function* () { + it('should not dispatch invalid events', async() => { const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const dispatch = spyDispatch(videoIframe); @@ -196,10 +210,10 @@ describes.realWin('amp-video-iframe', { }); }); - it('should dispatch valid events', function* () { + it('should dispatch valid events', async() => { const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const dispatch = spyDispatch(videoIframe); @@ -222,14 +236,14 @@ describes.realWin('amp-video-iframe', { } }); - it('should return intersection ratio if in autoplay range', function* () { + it('should return intersection ratio if in autoplay range', async() => { const id = 1234; const time = 1.234; const intersectionRatio = 2 / 3; const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const postMessage = stubPostMessage(videoIframe); @@ -248,7 +262,7 @@ describes.realWin('amp-video-iframe', { .to.have.been.calledOnce; }); - it('should return 0 if not in autoplay range', function* () { + it('should return 0 if not in autoplay range', async() => { const id = 1234; const time = 1.234; const intersectionRatio = 1 / 3; @@ -256,7 +270,7 @@ describes.realWin('amp-video-iframe', { const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const postMessage = stubPostMessage(videoIframe); @@ -302,11 +316,11 @@ describes.realWin('amp-video-iframe', { ].forEach(({sufix, eventType, vars, accept}) => { const verb = accept ? 'dispatch' : 'reject'; - it(`should ${verb} custom analytics event ${sufix}`, function* () { + it(`should ${verb} custom analytics event ${sufix}`, async() => { const videoIframe = createVideoIframe(); const dispatch = spyDispatch(videoIframe); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); acceptMockedMessages(videoIframe); @@ -317,42 +331,40 @@ describes.realWin('amp-video-iframe', { }, }; - const expectedVars = vars || {}; - if (vars) { Object.assign(data.analytics, {vars}); } + const {implementation_} = videoIframe; + if (accept) { - videoIframe.implementation_.onMessage_({data}); - expect( - dispatch.withArgs(VideoAnalyticsEvents.CUSTOM), - expectedVars).to.have.been.calledOnce; + const expectedEventVars = {eventType, vars: vars || {}}; + const expectedDispatch = + dispatch.withArgs(VideoAnalyticsEvents.CUSTOM, expectedEventVars); + implementation_.onMessage_({data}); + expect(expectedDispatch).to.have.been.calledOnce; } else { allowConsoleError(() => { - expect(() => { - videoIframe.implementation_.onMessage_({data}); - }).to.throw(); + expect(() => implementation_.onMessage_({data})).to.throw(); }); - - expect( - dispatch.withArgs(VideoAnalyticsEvents.CUSTOM), - expectedVars).to.not.have.been.called; + expect(dispatch.withArgs(VideoAnalyticsEvents.CUSTOM, any)) + .to.not.have.been.called; } }); }); }); describe('#mutatedAttributesCallback', () => { - it('updates src', function* () { - const videoIframe = createVideoIframe(); + it('updates src', async() => { + const defaultSrc = getIframeSrc(defaultFixture); + const videoIframe = createVideoIframe({src: defaultSrc}); const {implementation_} = videoIframe; - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const {iframe_} = implementation_; - expect(iframe_.src).to.equal(getIframeSrc(defaultFixture)); + expect(iframe_.src).to.match(new RegExp(`^${defaultSrc}#`)); const newSrc = getIframeSrc('video-iframe-2.html'); @@ -360,7 +372,7 @@ describes.realWin('amp-video-iframe', { implementation_.mutatedAttributesCallback({'src': true}); - expect(iframe_.src).to.equal(newSrc); + expect(iframe_.src).to.match(new RegExp(`^${newSrc}#`)); }); }); @@ -379,10 +391,10 @@ describes.realWin('amp-video-iframe', { describe(`#${method}`, () => { const lowercaseMethod = method.toLowerCase(); - it(`should post '${lowercaseMethod}'`, function* () { + it(`should post '${lowercaseMethod}'`, async() => { const videoIframe = createVideoIframe(); - yield whenLoaded(videoIframe); + await layoutAndLoad(videoIframe); const postMessage = stubPostMessage(videoIframe);