🐛Video: temp workaround for Chrome 72-74 breakage (ampproject#21249)

* video: temp workaround for Chrome 72-74 breakage * fix test * actually fix test
torch2424 · Mar 5, 2019 · 4083661 · 4083661
1 parent f2a2243
commit 4083661
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 0 deletions.
diff --git a/extensions/amp-iframe/0.1/amp-iframe.js b/extensions/amp-iframe/0.1/amp-iframe.js
@@ -382,6 +382,17 @@ export class AmpIframe extends AMP.BaseElement {
     }
 
     this.propagateAttributes(ATTRIBUTES_TO_PROPAGATE, iframe);
+
+    // TEMPORARY: disable `allow=autoplay`
+    // This is a workaround for M72-M74 user-activation breakage.
+    // If this is still here in May 2019, please ping @aghassemi
+    // See https://github.com/ampproject/amphtml/issues/21242 for details.
+    // TODO(aghassemi, #21247)
+    let allowVal = iframe.getAttribute('allow') || '';
+    // allow syntax is complex, not worth parsing for temp code.
+    allowVal = allowVal.replace('autoplay', 'autoplay-disabled');
+    iframe.setAttribute('allow', allowVal);
+
     setSandbox(this.element, iframe, this.sandbox_);
     iframe.src = this.iframeSrc;
 

diff --git a/extensions/amp-iframe/0.1/test/test-amp-iframe.js b/extensions/amp-iframe/0.1/test/test-amp-iframe.js
@@ -201,6 +201,21 @@ describes.realWin('amp-iframe', {
       expect(iframe.getAttribute('marginwidth')).to.be.null;
     });
 
+    // This is temporary.
+    // TODO(aghassemi, #21247)
+    it('should disable allow=autoplay', function* () {
+      const ampIframe = createAmpIframe(env, {
+        src: iframeSrc,
+        width: 100,
+        height: 100,
+        allow: 'microphone; autoplay; camera',
+      });
+      yield waitForAmpIframeLayoutPromise(doc, ampIframe);
+      const iframe = ampIframe.querySelector('iframe');
+      expect(iframe.getAttribute('allow')).to
+          .equal('microphone; autoplay-disabled; camera');
+    });
+
     it('should default frameborder to 0 if not set', function* () {
       const ampIframe = createAmpIframe(env, {
         src: iframeSrc,

diff --git a/extensions/amp-ima-video/0.1/amp-ima-video.js b/extensions/amp-ima-video/0.1/amp-ima-video.js
@@ -18,6 +18,7 @@ import {Deferred} from '../../../src/utils/promise';
 import {ImaPlayerData} from '../../../ads/google/ima-player-data';
 import {Services} from '../../../src/services';
 import {VideoEvents} from '../../../src/video-interface';
+import {addUnsafeAllowAutoplay} from '../../../src/iframe-video';
 import {assertHttpsUrl} from '../../../src/url';
 import {
   childElementsByTag,
@@ -172,6 +173,10 @@ class AmpImaVideo extends AMP.BaseElement {
 
       this.applyFillContent(iframe);
 
+      // This is temporary until M74 launches.
+      // TODO(aghassemi, #21247)
+      addUnsafeAllowAutoplay(iframe);
+
       this.iframe_ = iframe;
 
       const deferred = new Deferred();

diff --git a/extensions/amp-youtube/0.1/amp-youtube.js b/extensions/amp-youtube/0.1/amp-youtube.js
@@ -19,6 +19,7 @@ import {Services} from '../../../src/services';
 import {VideoEvents} from '../../../src/video-interface';
 import {addParamsToUrl} from '../../../src/url';
 import {
+  addUnsafeAllowAutoplay,
   createFrameFor,
   isJsonOrObj,
   mutedOrUnmutedEvent,
@@ -235,6 +236,10 @@ class AmpYoutube extends AMP.BaseElement {
     // See https://developers.google.com/youtube/iframe_api_reference
     const iframe = createFrameFor(this, this.getVideoIframeSrc_());
 
+    // This is temporary until M74 launches.
+    // TODO(aghassemi, #21247)
+    addUnsafeAllowAutoplay(iframe);
+
     this.iframe_ = iframe;
 
     // Listening for VideoEvents.LOAD in AutoFullscreenManager.register may

diff --git a/src/iframe-video.js b/src/iframe-video.js
@@ -136,3 +136,18 @@ export function objOrParseJson(objOrStr) {
 export function mutedOrUnmutedEvent(isMuted) {
   return isMuted ? VideoEvents.MUTED : VideoEvents.UNMUTED;
 }
+
+
+/**
+ * TEMPORARY workaround for M72-M74 user-activation breakage.
+ * If this method is still here in May 2019, please ping @aghassemi
+ * Only used by trusted video players: IMA and YouTube.
+ * See https://github.com/ampproject/amphtml/issues/21242 for details.
+ * TODO(aghassemi, #21247)
+ * @param {Element} iframe
+ */
+export function addUnsafeAllowAutoplay(iframe) {
+  let val = iframe.getAttribute('allow') || '';
+  val += 'autoplay;';
+  iframe.setAttribute('allow', val);
+}