Initial changes, need to update tests

ampproject · jridgewell · Nov 4, 2016 · Aug 31, 2016 · Aug 31, 2016 · Sep 6, 2016
commit 5faa071d7f45ec8778f863a2462e34bfd23f448a
diff --git a/examples/a4a.amp.html b/examples/a4a.amp.html
@@ -11,79 +11,11 @@
   </head>
   <body>
 
-    <h1>A4A Examples</h1>
-
-    <h2>Fake ad nework</h2>
-    <amp-ad width="300" height="400"
-        type="fake"
-        data-use-a4a="true"
-        src="fake_amp.json">
-    </amp-ad>
-
     <h2>Adsense</h2>
     <amp-ad width=300 height=250
         type="adsense"
-        data-ad-client="ca-pub-2005682797531342"
-        data-ad-slot="7046626912">
-    </amp-ad>
-
-    <h2>Adsense</h2>
-    <amp-ad width=300 height=300
-        type="adsense"
-        data-ad-client="ca-pub-8125901705757971"
-        data-ad-slot="7783467241">
+        data-ad-client="ca-pub-6743622525202572">
     </amp-ad>
 
-  <h2>Doubleclick</h2>
-  <amp-ad width=320 height=50
-      type="doubleclick"
-      data-slot="/4119129/mobile_ad_banner"
-      >
-  </amp-ad>
-
-  <h2>Doubleclick with JSON based parameters</h2>
-  <amp-ad width=320 height=50
-      type="doubleclick"
-      data-slot="/4119129/mobile_ad_banner"
-      json='{"targeting":{"sport":["rugby","cricket"]},"categoryExclusions":["health"],"tagForChildDirectedTreatment":0}'
-      >
-  </amp-ad>
-
-  <h2>Doubleclick no ad with fallback</h2>
-  <amp-ad width=300 height=200
-      type="doubleclick"
-      data-slot="/4119129/doesnt-exist"
-      >
-      <div fallback>
-        <b>Thank you for trying AMP!</b>
-
-        <p>You got lucky! We have no ad to show to you!</p>
-      </div>
-  </amp-ad>
-
-  <h2>Doubleclick no ad with placeholder and fallback</h2>
-  <amp-ad width=300 height=200
-      type="doubleclick"
-      data-slot="/4119129/doesnt-exist"
-      >
-      <div placeholder>
-        <b>Placeholder here!!!</b>
-      </div>
-      <div fallback>
-        <b>Thank you for trying AMP!</b>
-
-        <p>You got lucky! We have no ad to show to you!</p>
-      </div>
-  </amp-ad>
-
-  <h2>Doubleclick with overriden size</h2>
-  <amp-ad width=420 height=100
-      data-override-width=320 data-override-height=50
-      type="doubleclick"
-      data-slot="/4119129/mobile_ad_banner"
-      class="red"
-      >
-  </amp-ad>
-
   </body>
 </html>
diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js
@@ -105,9 +105,7 @@ const SHARED_IFRAME_PROPERTIES = {
 export let AdResponseDef;
 
 /** @typedef {{
-      cssUtf16CharOffsets: Array<number>,
-      bodyUtf16CharOffsets: !Array<number>,
-      bodyAttributes: ?string,
+      minifiedCreative: string,
       customElementExtensions: Array<string>,
       customStylesheets: Array<string>
     }} */
@@ -707,11 +705,6 @@ export class AmpA4A extends AMP.BaseElement {
                 'frameborder': '0', 'allowfullscreen': '',
                 'allowtransparency': '', 'scrolling': 'no'}));
           this.applyFillContent(iframe);
-
-          const cssBlock = this.formatCSSBlock_(creative, creativeMetaData);
-          const bodyBlock = this.formatBody_(creative, creativeMetaData);
-          const bodyAttrString = creativeMetaData.bodyAttributes ?
-                  ' ' + creativeMetaData.bodyAttributes : '';
           const fontsArray = [];
           if (creativeMetaData.customStylesheets) {
             creativeMetaData.customStylesheets.forEach(s => {
@@ -721,20 +714,17 @@ export class AmpA4A extends AMP.BaseElement {
               }
             });
           }
-          const modifiedCreative =
-            `<!doctype html><html ⚡4ads>
-            <head>
-              <style amp-custom>${cssBlock}</style>
-              </head>
-            <body ${bodyAttrString}>${bodyBlock}</body>
-            </html>`;
           return installFriendlyIframeEmbed(
             iframe, this.element, {
               url: this.adUrl_,
-              html: modifiedCreative,
+              html: creativeMetaData.minifiedCreative,
               extensionIds: creativeMetaData.customElementExtensions || [],
               fonts: fontsArray,
             }).then(friendlyIframeEmbed => {
+              // Ensure visibility hidden has been removed (set by boilerplate).
+              const frameDoc = friendlyIframeEmbed.iframe.contentDocument ||
+                friendlyIframeEmbed.win.document;
+              frameDoc.body.style.visibility = 'visible';
               // Capture phase click handlers on the ad.
               this.registerExpandUrlParams_(friendlyIframeEmbed.win);
               // Bubble phase click handlers on the ad.
@@ -744,6 +734,7 @@ export class AmpA4A extends AMP.BaseElement {
               return true;
             });
         } catch (e) {
+          dev().error('Error injecting creative in friendly frame', e);
           // If we fail on any of the steps of Shadow DOM construction, just
           // render in iframe.
           // TODO: report!
@@ -854,94 +845,51 @@ export class AmpA4A extends AMP.BaseElement {
       return null;
     }
     try {
-      return this.buildCreativeMetaData_(/** @type {!Object} */ (JSON.parse(
-        creative.slice(metadataStart + METADATA_STRING.length, metadataEnd))));
-    } catch (err) {
-      dev().warn('A4A', 'Invalid amp metadata: %s',
+      const metaDataObj = JSON.parse(
         creative.slice(metadataStart + METADATA_STRING.length, metadataEnd));
-      return null;
-    }
-  }
-
-  /**
-   * @param {!Object} metaDataObj JSON extraced from creative
-   * @return {!CreativeMetaDataDef} if valid, null otherwise
-   * @private
-   */
-  buildCreativeMetaData_(metaDataObj) {
-    const metaData = {};
-    metaData.bodyUtf16CharOffsets = metaDataObj['bodyUtf16CharOffsets'];
-    if (!isValidOffsetArray(metaData.bodyUtf16CharOffsets)) {
-      // Invalid/Missing body offsets array.
-      throw new Error('Invalid/missing body offsets');
-    }
-    if (metaDataObj['cssUtf16CharOffsets']) {
-      metaData.cssUtf16CharOffsets = metaDataObj['cssUtf16CharOffsets'];
-      if (!isValidOffsetArray(metaData.cssUtf16CharOffsets)) {
-        throw new Error('Invalid CSS offsets');
-      }
-    }
-    if (metaDataObj['bodyAttributes']) {
-      metaData.bodyAttributes = metaDataObj['bodyAttributes'];
-      if (typeof metaData.bodyAttributes !== 'string') {
-        throw new Error('Invalid body attributes');
+      const ampRuntimeUtf16CharOffsets =
+        metaDataObj['ampRuntimeUtf16CharOffsets'];
+      if (!isValidOffsetArray(ampRuntimeUtf16CharOffsets)) {
+        throw new Error('Invalid runtime offsets');
       }
-    }
-    if (metaDataObj['customElementExtensions']) {
-      metaData.customElementExtensions = metaDataObj['customElementExtensions'];
-      if (!isArray(metaData.customElementExtensions)) {
-        throw new Error('Invalid extensions');
-      }
-    }
-    if (metaDataObj['customStylesheets']) {
-      // Expect array of objects with at least one key being 'href' whose value
-      // is URL.
-      metaData.customStylesheets = metaDataObj['customStylesheets'];
-      const errorMsg = 'Invalid custom stylesheets';
-      if (!isArray(metaData.customStylesheets)) {
-        throw new Error(errorMsg);
+      const metaData = {};
+      if (metaDataObj['customElementExtensions']) {
+        metaData.customElementExtensions =
+          metaDataObj['customElementExtensions'];
+        if (!isArray(metaData.customElementExtensions)) {
+          throw new Error('Invalid extensions');
+        }
       }
-      metaData.customStylesheets.forEach(stylesheet => {
-        if (!isObject(stylesheet) || !stylesheet['href'] ||
-            typeof stylesheet['href'] !== 'string' ||
-            !/^https:\/\//i.test(stylesheet['href'])) {
+      if (metaDataObj['customStylesheets']) {
+        // Expect array of objects with at least one key being 'href' whose value
+        // is URL.
+        metaData.customStylesheets = metaDataObj['customStylesheets'];
+        const errorMsg = 'Invalid custom stylesheets';
+        if (!isArray(metaData.customStylesheets)) {
           throw new Error(errorMsg);
         }
-      });
+        metaData.customStylesheets.forEach(stylesheet => {
+          if (!isObject(stylesheet) || !stylesheet['href'] ||
+              typeof stylesheet['href'] !== 'string' ||
+              !/^https:\/\//i.test(stylesheet['href'])) {
+            throw new Error(errorMsg);
+          }
+        });
+      }
+      // TODO(keithwrightbos): OK to assume ampRuntimeUtf16CharOffsets is before
+      // metadata as its in the head?
+      metaData.minifiedCreative =
+        creative.slice(0, ampRuntimeUtf16CharOffsets[0]) +
+        creative.slice(ampRuntimeUtf16CharOffsets[1], metadataStart) +
+        creative.slice(metadataEnd, creative.length);
+      return metaData;
+    } catch (err) {
+      dev().warn('A4A', 'Invalid amp metadata: %s',
+        creative.slice(metadataStart + METADATA_STRING.length, metadataEnd));
+      return null;
     }
-    return metaData;
   }
 
- /**
-  * Extracts the body portion of the creative, according to directions in the
-  * metaData, and formats it for insertion into Shadow DOM.
-  * @param {string} creative from which CSS is extracted
-  * @param {!CreativeMetaDataDef} metaData Metadata object extracted from the
-  *    reserialized creative.
-  * @returns {string}  Body of AMP creative, surrounded by {@code
-  *     <amp-ad-body>} tags, and suitable for injection into Shadow DOM.
-  * @private
-  */
- formatBody_(creative, metaData) {
-   return creative.substring(metaData.bodyUtf16CharOffsets[0],
-       metaData.bodyUtf16CharOffsets[1]);
- }
-
- /**
-  * Note: destructively reverses the {@code offsets} list as a side effect.
-  * @param {string} creative from which CSS is extracted
-  * @param {!CreativeMetaDataDef} metaData from creative.
-  * @returns {string} CSS to be added to page.
-  */
- formatCSSBlock_(creative, metaData) {
-   if (!metaData.cssUtf16CharOffsets) {
-     return '';
-   }
-   return creative.substring(
-       metaData.cssUtf16CharOffsets[0],
-       metaData.cssUtf16CharOffsets[1]);
- }
-
   /**
    * Registers a click handler for "A2A" (AMP-to-AMP navigation where the AMP
    * viewer navigates to an AMP destination on our behalf.

diff --git a/extensions/amp-a4a/0.1/test/testdata/regexps.original.html b/extensions/amp-a4a/0.1/test/testdata/regexps.original.html
@@ -14,12 +14,11 @@
   limitations under the license.
 -->
 <!doctype html>
-<html ⚡>
+<html ⚡4ads>
 <head>
   <meta charset="utf-8">
-  <link rel="canonical" href="./regular-html-version.html" />
   <meta name="viewport" content="width=device-width,minimum-scale=1">
-  <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
+  <style amp4ads-boilerplate>body{visibility:hidden}</style>
   <script async src="https://cdn.ampproject.org/v0.js"></script>
   <script async custom-element="amp-vine" src="https://cdn.ampproject.org/v0/amp-vine-latest.js"></script>
   <script async custom-element="amp-audio" src="https://cdn.ampproject.org/v0/amp-audio-0.1.js"></script>
@@ -34,8 +33,8 @@
 </head>
 <body>
 
-  <amp-audio src="https://example.com/audio" layout="fixed" autoplay="">
-  <amp-audio src="https://example.com/audio" layout="fixed" autoplay="desktop">
+  <amp-audio src="https://example.com/audio" layout="fixed">
+  <amp-audio src="https://example.com/audio" layout="fixed">
 
 </body>
 </html>