✨ Support data-param-* in amp-video-iframe (ampproject#26636)

Adds as query parameters for both `src` and `poster`.
samouri · Feb 6, 2020 · 3d21e45 · 3d21e45
1 parent 48f7bda
commit 3d21e45
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 35 deletions.
diff --git a/extensions/amp-video-iframe/0.1/amp-video-iframe.js b/extensions/amp-video-iframe/0.1/amp-video-iframe.js
@@ -26,16 +26,21 @@ import {
   originMatches,
 } from '../../../src/iframe-video';
 import {Services} from '../../../src/services';
+import {addParamsToUrl} from '../../../src/url';
+import {
+  createElementWithAttributes,
+  getDataParamsFromAttributes,
+  isFullscreenElement,
+  removeElement,
+} from '../../../src/dom';
 import {dev, devAssert, user, userAssert} from '../../../src/log';
 import {dict} from '../../../src/utils/object';
 import {
   disableScrollingOnIframe,
   looksLikeTrackingIframe,
 } from '../../../src/iframe-helper';
 import {getData, listen} from '../../../src/event-helper';
-import {htmlFor} from '../../../src/static-template';
 import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl';
-import {isFullscreenElement, removeElement} from '../../../src/dom';
 import {isLayoutSizeDefined} from '../../../src/layout';
 import {once} from '../../../src/utils/function';
 
@@ -76,6 +81,15 @@ const getAnalyticsEventTypePrefixRegex = once(
   () => new RegExp(`^${ANALYTICS_EVENT_TYPE_PREFIX}`)
 );
 
+/**
+ * @param {string} url
+ * @param {!Element} element
+ * @return {string}
+ * @private
+ */
+const addDataParamsToUrl = (url, element) =>
+  addParamsToUrl(url, getDataParamsFromAttributes(element));
+
 /**
  * @param {string} src
  * @return {string}
@@ -191,17 +205,19 @@ class AmpVideoIframe extends AMP.BaseElement {
   /** @override */
   createPlaceholderCallback() {
     const {element} = this;
-    const html = htmlFor(element);
-    const poster = html`
-      <amp-img layout="fill" placeholder></amp-img>
-    `;
-
-    poster.setAttribute(
-      'src',
-      this.user().assertString(element.getAttribute('poster'))
+    const src = addDataParamsToUrl(
+      user().assertString(element.getAttribute('poster')),
+      element
+    );
+    return createElementWithAttributes(
+      devAssert(element.ownerDocument),
+      'amp-img',
+      dict({
+        'src': src,
+        'layout': 'fill',
+        'placeholder': '',
+      })
     );
-
-    return poster;
   }
 
   /** @override */
@@ -229,10 +245,10 @@ class AmpVideoIframe extends AMP.BaseElement {
   getSrc_() {
     const {element} = this;
     const urlService = Services.urlForDoc(element);
-    const src = urlService.assertHttpsUrl(element.getAttribute('src'), element);
+    const src = element.getAttribute('src');
 
     if (urlService.getSourceOrigin(src) === urlService.getWinOrigin(this.win)) {
-      this.user().warn(
+      user().warn(
         TAG,
         'Origins of document inside amp-video-iframe and the host are the ' +
           'same, which allows for same-origin behavior. However in AMP ' +
@@ -242,7 +258,7 @@ class AmpVideoIframe extends AMP.BaseElement {
       );
     }
 
-    return maybeAddAmpFragment(src);
+    return maybeAddAmpFragment(addDataParamsToUrl(src, element));
   }
 
   /**

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
@@ -19,9 +19,9 @@ import {Services} from '../../../../src/services';
 import {VideoEvents} from '../../../../src/video-interface';
 import {
   addAttributesToElement,
+  createElementWithAttributes,
   whenUpgradedToCustomElement,
 } from '../../../../src/dom';
-import {htmlFor} from '../../../../src/static-template';
 import {listenOncePromise} from '../../../../src/event-helper';
 import {tryParseJson} from '../../../../src/json';
 
@@ -73,13 +73,11 @@ describes.realWin(
             height: size[1],
           };
 
-    function createVideoIframe({size, src} = {}) {
-      const html = htmlFor(doc);
-      const el = html`
-        <amp-video-iframe poster="foo.png"></amp-video-iframe>
-      `;
-      el.setAttribute('src', src || getIframeSrc());
-      addAttributesToElement(el, layoutConfigAttrs(size));
+    function createVideoIframe(attrs = {}, opt_size) {
+      const el = createElementWithAttributes(doc, 'amp-video-iframe', attrs);
+      const {src = getIframeSrc(), poster = 'foo.png'} = attrs;
+      addAttributesToElement(el, {src, poster});
+      addAttributesToElement(el, layoutConfigAttrs(opt_size));
       doc.body.appendChild(el);
       return el;
     }
@@ -116,6 +114,16 @@ describes.realWin(
     }
 
     describe('#layoutCallback', () => {
+      it('uses data-param-* in src', async () => {
+        const element = createVideoIframe({
+          'data-param-vid': 'my_vid',
+          'data-param-foo-bar': 'foo bar',
+        });
+        await layoutAndLoad(element);
+        const {src} = element.querySelector('iframe');
+        expect(src).to.match(/\?vid=my_vid&fooBar=foo%20bar#.*$/);
+      });
+
       it('sets metadata in iframe name', async () => {
         const metadata = {
           canonicalUrl: 'foo.html',
@@ -170,7 +178,7 @@ describes.realWin(
         ];
 
         trackingSizes.forEach(size => {
-          const {implementation_} = createVideoIframe({size});
+          const {implementation_} = createVideoIframe({}, size);
           allowConsoleError(() => {
             expect(() => implementation_.buildCallback()).to.throw();
           });
@@ -180,16 +188,23 @@ describes.realWin(
 
     describe('#createPlaceholderCallback', () => {
       it('creates an amp-img with the poster as src', () => {
-        const videoIframe = createVideoIframe();
-
-        const placeholder = videoIframe.implementation_.createPlaceholderCallback();
-
+        const poster = 'foo.bar';
+        const placeholder = createVideoIframe({poster}).createPlaceholder();
         expect(placeholder).to.have.attribute('placeholder');
         expect(placeholder.tagName.toLowerCase()).to.equal('amp-img');
         expect(placeholder.getAttribute('layout')).to.equal('fill');
-        expect(placeholder.getAttribute('src')).to.equal(
-          videoIframe.getAttribute('poster')
-        );
+        expect(placeholder.getAttribute('src')).to.equal(poster);
+      });
+
+      it("uses data-param-* in the poster's src", () => {
+        expect(
+          createVideoIframe({
+            'data-param-my-poster-param': 'my param',
+            'data-param-another': 'value',
+          })
+            .createPlaceholder()
+            .getAttribute('src')
+        ).to.match(/\?myPosterParam=my%20param&another=value$/);
       });
     });
 

diff --git a/extensions/amp-video-iframe/amp-video-iframe.md b/extensions/amp-video-iframe/amp-video-iframe.md
@@ -121,6 +121,22 @@ to be played, <a href="https://github.com/ampproject/amphtml/blob/master/spec/am
     <td width="40%"><strong>referrerpolicy</strong></td>
     <td>The <a href="https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/referrerPolicy"><code>referrerpolicy</code></a> to be set on the iframe element.</td>
   </tr>
+  <tr>
+    <td width="40%"><a id="data-param"></a><strong>data-param-*</strong></td>
+    <td>
+      All <code>data-param-*</code> attributes are added as query parameters
+      to the iframe's <code>src</code>. They may be used to pass custom values
+      through to the player document.<br />
+      Keys and values will be URI encoded. Keys will be camel cased.
+      <ul>
+        <li><code>data-param-foo="bar"</code> becomes <code>&foo=bar</code></li>
+        <li>
+          <code>data-param-channel-id="SOME_VALUE"</code> becomes
+          <code>&channelId=SOME_VALUE</code>
+        </li>
+      </ul>
+    </td>
+  </tr>
 </table>
 
 ## Usage
@@ -147,19 +163,35 @@ If you're a vendor that does _not_ provide a [custom video player component](../
 By hosting a generic [integration document](#integration) that can reference videos with URL parameters, authors don't need to provide the inner player document themselves, but only include an `<amp-video-iframe>` tag in the AMP document:
 
 ```html
+<!--
+  data-param-* attributes are added to src and poster, so this would use the
+  following composed urls:
+
+  src: https://vendor.example/amp-video-iframe
+      ?videoid=MY_VIDEO_ID
+      &channelid=MY_CHANNEL_ID
+
+  poster: https://vendor.example/poster.jpg
+      ?videoid=MY_VIDEO_ID
+      &channelid=MY_CHANNEL_ID
+-->
 <amp-video-iframe
   layout="responsive"
   width="16"
   height="9"
-  src="https://video-provider.example/amp-video-iframe.html?videoid=PROVIDED_VIDEO_ID"
-  poster="https://video-provider.example/amp-video-iframe-poster.jpg?videoid=PROVIDED_VIDEO_ID"
+  src="https://vendor.example/amp-video-iframe"
+  poster="https://vendor.example/poster.jpg"
+  data-param-videoid="MY_VIDEO_ID"
+  data-param-channelid="MY_CHANNEL_ID"
 >
 </amp-video-iframe>
 ```
 
-The dynamic server-side document `amp-video-iframe.html` includes the video as necessary per the configurable `PROVIDED_VIDEO_ID` parameter. This document bootstraps the [iframe integration script](#integration) so that the AMP document can coordinate with the player.
+The `src` and `poster` URLs are appended with [`data-param-*` attributes as query string](#data-param).
+
+The `/amp-video-iframe` document bootstraps the [integration script](#integration) so that the AMP document can coordinate with the player.
 
-Note: For most video providers, `amp-video-iframe` provides enough tools for common playback actions (see possible [methods](#method) and [events](#postEvent)). Refer to the [vendor-specific video player spec](../../spec/amp-3p-video.md) for more details on whether you can use `amp-video-iframe` or you should build a third-party player component instead.
+Note: For most video providers, `amp-video-iframe` provides enough tools for common playback actions (see [methods](#method) and [events](#postEvent)). Refer to the [vendor player spec](../../spec/amp-3p-video.md) for more details on whether you can use `amp-video-iframe` or you should build a third-party player component instead.
 
 ## <a id="integration"></a> Integration inside the frame