Modify A4A AMP Creative to use ampRuntimeUtf16CharOffsets from valida…

…tion rewrite (ampproject#5982)

* Update amp-ad-network-doubleclick-impl.md

* Reverting doc changes

The previous change was made in error (wrong branch).

* Initial changes, need to update tests

* Start fixing tests

* Fix unit test coverage

* revert a4a examples

Introduce amp-auto-id attribute to AMP element. (ampproject#5973)

* Introduce amp-auto-id attribute to AMP element.

* Fix tests

* Address comments

* Address comments

* revert

amp-live-list.md polling clarification (ampproject#5991)

Only report 1% of errors if a page has non-AMP JS (ampproject#5994)

Fixes ampproject#5732

Allow for hosted testing to override where third party frame is retrieved from (ampproject#5890)

* Allow for hosted testing to override where third party frame is retrieved from

* test my own site

* fix ava tests

Remove unsupported query selector feature (ampproject#5999)

Workaround for misbehaving webview viewer (ampproject#6001)

A webview viewer from Google is seing origins of the form `www.google.com`.

For the time being we accept these as they trigger errors in our error reporting that aren't useful. As this form of origins can only occur with webviews that have full control over the page via JS injection anyway, this change should make no difference for normal web usage.

A4A integration tests (ampproject#5812)

* First draft of full integration tests for A4A.

* First draft of full integration tests for A4A.

* Got tests of exception conditions working.  Refactored some common stuff to utils.js.

* Refactored XDom and friendly iframe tests to helpers.

* Added tests.

* Resolved merge conflicts.

* Centralized visibility check and updated visibility test.

* Moved all A4A tests to using central visibility test.

* Lint fixes.

* Changed custom expectations to local functions.

* Replaced stringToArrayBuffer with utf8Encode.

* Fixed visibility assertion.

* Lint fixes.

* Added a comment (partly in order to get Travis to re-run).

* Changes in response to reviews.

* Rebased and fixed small rebase error.

Fixes an issue where ads are not correctly centered on certain platforms. (ampproject#6003)

* Centralized creative centering in doubleclick.js, and changed css 'transform' to a more commonly supported property.

* Added check for existence and changed to plain old 'transform'.

* Now asserting the existence of the container.

amp-sticky-ad close button new style (ampproject#5979)

* unit style

* fix css

* fix test

* rebase

* fix z-index

Other JS errors: Use startsWith (ampproject#6006)

Fixes ampproject#5994 (comment).

Fix log calls without TAGNAME (ampproject#6005)

My presubmit was a bit too lenient, and too strict at the same time.

refactor amp-ad.css (ampproject#5992)

Combine amp-analytics var docs with var substitutions doc re: ampproject#1302 (ampproject#5576)

* Combine amp-analytics var docs with var substitutions doc re: ampproject#1302

* Updated non-supported vars + format changes for amp-analytics vars

* Updates per Avi's feedback

* Updated descriptions for amp-carousel vars

add version parameter to AMP.extension signature (ampproject#5989)

* add version parameter to AMP.extension signature

* apply recommendations

* remove TODO

Improve error reporting (ampproject#6019)

- Include the current `location.hash`. We are missing it, because it is not included in the referrer.
- Include previously recorded errors, so it is easier to identify follow on errors.
- Kill the 2000 char limit as it isn't important for AMP's target browsers.

Include the originalHash (ampproject#6020)

We remove the hash in embedding mode, so the original code doesn't actually work.

Fix error: "Can't find variable: TextDecoder"  (ampproject#6011)

* Update amp-ad-network-doubleclick-impl.md

* Reverting doc changes

The previous change was made in error (wrong branch).

* Initial changes, need to update tests

* Start fixing tests

* Fix unit test coverage

* revert a4a examples

* Include self (window) when verifying TextDecoder/TextEncoder existence

* fix upstream conflict

* PR feedback

* PR feedback

DEVELOPING.md

@@ -51,6 +51,7 @@ If you have any questions, feel free to ask on the issue or join us on [Slack](h
 | `gulp lint --watch`                                                     | Watches for changes in files, Validates against Google Closure Linter.|
 | `gulp lint --fix`                                                       | Fixes simple lint warnings/errors automatically.                      |
 | `gulp build`<sup>[[1]](#footnote-1)</sup>                               | Builds the AMP library.                                               |
+| `gulp build --fortesting`<sup>[[1]](#footnote-1)</sup>                  | Builds the AMP library and will read the AMP_TESTING_HOST environment variable to write out an override AMP_CONFIG. |
 | `gulp build --css-only`<sup>[[1]](#footnote-1)</sup>                    | Builds only the embedded css into js files for the AMP library.       |
 | `gulp clean`                                                            | Removes build output.                                                 |
 | `gulp css`                                                              | Recompile css to build directory.                                     |
@@ -127,6 +128,11 @@ For deploying and testing local AMP builds on [HEROKU](https://www.heroku.com/)
 
 Meantime, you can also use our automatic build on Heroku [link](http://amphtml-nightly.herokuapp.com/), which is normally built with latest head on master branch (please allow delay). The first time load is normally slow due to Heroku's free account throttling policy.
 
+To correctly get ads and third party working when testing on hosted services
+you will need set the `AMP_TESTING_HOST` environment variable. (On heroku this
+is done through
+`heroku config:set AMP_TESTING_HOST=my-heroku-subdomain.herokuapp.com`)
+
 ## Repository Layout
 <pre>
   3p/             - Implementation of third party sandbox iframes.

ads/_ping_.js

@@ -49,7 +49,7 @@ export function _ping_(global, data) {
     }
     global.context.observeIntersection(function(changes) {
       changes.forEach(function(c) {
-        dev().info('Intersection: (WxH)' +
+        dev().info('AMP-AD', 'Intersection: (WxH)' +
             `${c.intersectionRect.width}x${c.intersectionRect.height}`);
       });
     });

ads/google/doubleclick.js

@@ -16,7 +16,8 @@
 
 import {makeCorrelator} from './correlator';
 import {validateData, loadScript} from '../../3p/3p';
-import {user} from '../../src/log';
+import {dev, user} from '../../src/log';
+import {setStyles} from '../../src/style';
 
 /**
  * @enum {number}
@@ -53,6 +54,16 @@ export function doubleclick(global, data) {
     };
   }
 
+  // Center the ad in the container.
+  const container = global.document.querySelector('#c');
+  setStyles(dev().assertElement(container), {
+    top: '50%',
+    left: '50%',
+    bottom: '',
+    right: '',
+    transform: 'translate(-50%, -50%)',
+  });
+
   if (data.useSameDomainRenderingUntilDeprecated != undefined ||
       data.multiSize) {
     doubleClickWithGpt(global, data, GladeExperiment.GLADE_OPT_OUT);
@@ -127,14 +138,6 @@ function doubleClickWithGpt(global, data, gladeExperiment) {
     parseInt(data.overrideHeight || data.height, 10),
   ]];
 
-  // Center the ad in the container.
-  const container = global.document.querySelector('#c');
-  container.style.top = '50%';
-  container.style.left = '50%';
-  container.style.bottom = '';
-  container.style.right = '';
-  container.style.transform = 'translate(-50%, -50%)';
-
   // Get multi-size ad request data, if any, and validate it in the following
   // ways:
   // 1) Ensure that the data string is a comma-separated list of sizes of the
@@ -346,11 +349,6 @@ function doubleClickWithGlade(global, data, gladeExperiment) {
   // Center the ad in the container.
   slot.setAttribute('height', requestHeight);
   slot.setAttribute('width', requestWidth);
-  slot.style.top = '50%';
-  slot.style.left = '50%';
-  slot.style.bottom = '';
-  slot.style.right = '';
-  slot.style.transform = 'translate(-50%, -50%)';
 
   slot.addEventListener('gladeAdFetched', event => {
     if (event.detail.empty) {

build-system/SERVING.md

@@ -52,6 +52,7 @@ rm /path/to/cdn/production/alp.js.bak
 # make sure and prepend the global production config to main binaries
 gulp prepend-global --target /path/to/cdn/production/v0.js --prod
 gulp prepend-global --target /path/to/cdn/production/alp.js --prod
+gulp prepend-global --target /path/to/3p/cdn/production/f.js --prod
 
 # The following commands below are optional if you want to host a similar
 # experiments page like https://cdn.ampproject.org/experiments.html

build-system/dep-check-config.js

@@ -86,6 +86,7 @@ exports.rules = [
       'ads/**->src/mode.js',
       'ads/**->src/url.js',
       'ads/**->src/types.js',
+      'ads/**->src/style.js',
       // ads/google/a4a doesn't contain 3P ad code and should probably move
       // somewhere else at some point
       'ads/google/a4a/**->src/ad-cid.js',

build-system/runner/src/org/ampproject/AmpPass.java

@@ -99,12 +99,12 @@ public AmpPass(AbstractCompiler compiler, boolean isProd,
    *         NAME self 3 [length: 4] [source_file: input0]
    *         STRING someproperty 3 [length: 15] [source_file: input0]
    *       STRING AMP 3 [length: 3] [source_file: input0]
-   *     STRING etension 3 [length: 12] [source_file: input0]
+   *     STRING extension 3 [length: 12] [source_file: input0]
    *   STRING some-string 3 [length: 9] [source_file: input0]
    *   FUNCTION  3 [length: 46] [source_file: input0]
    */
   private boolean isAmpExtensionCall(Node n) {
-    if (n != null && n.isCall() && n.getChildCount() == 3) {
+    if (n != null && n.isCall()) {
       Node getprop = n.getFirstChild();
 
       // The AST has the last getprop higher in the hierarchy.
@@ -115,7 +115,7 @@ private boolean isAmpExtensionCall(Node n) {
                firstChild.getString() == "AMP") ||
             isGetPropName(firstChild, "AMP")) {
           // Child at index 1 should be the "string" value (first argument)
-          Node func = n.getChildAtIndex(2);
+          Node func = getAmpExtensionCallback(n);
           return func != null && func.isFunction();
         }
       }
@@ -136,7 +136,7 @@ private boolean isGetPropName(Node n, String name) {
    * This operation should be guarded stringently by `isAmpExtensionCall`
    * predicate.
    *
-   * AMP.extension('some-name', function(AMP) {
+   * AMP.extension('some-name', '0.1', function(AMP) {
    *   // BODY...
    * });
    *
@@ -149,7 +149,7 @@ private void inlineAmpExtensionCall(Node n, Node expr) {
     if (expr == null || !expr.isExprResult()) {
       return;
     }
-    Node func = n.getChildAtIndex(2);
+    Node func = getAmpExtensionCallback(n);
     func.detachFromParent();
     Node arg1 = IR.getprop(IR.name("self"), IR.string("AMP"));
     arg1.setLength("self.AMP".length());
@@ -161,6 +161,10 @@ private void inlineAmpExtensionCall(Node n, Node expr) {
     compiler.reportCodeChange();
   }
 
+  private Node getAmpExtensionCallback(Node n) {
+    return n.getLastChild();
+  }
+
   /**
    * For a function that looks like:
    * function fun(val) {

build-system/runner/test/org/ampproject/AmpPassTest.java

@@ -324,7 +324,7 @@ public void testRemoveAmpAddExtensionCallWithExplicitContext() throws Exception
     testEs6(
         LINE_JOINER.join(
             "var a = 'hello';",
-            "self.AMP.extension('hello', function(AMP) {",
+            "self.AMP.extension('hello', '0.1', function(AMP) {",
             "  var a = 'world';",
             "  console.log(a);",
             "});",
@@ -342,7 +342,7 @@ public void testRemoveAmpAddExtensionCallWithNoContext() throws Exception {
     testEs6(
         LINE_JOINER.join(
             "var a = 'hello';",
-            "AMP.extension('hello', function(AMP) {",
+            "AMP.extension('hello', '0.1', function(AMP) {",
             "  var a = 'world';",
             "  console.log(a);",
             "});",

build-system/tasks/prepend-global/index.js

@@ -42,6 +42,9 @@ function sanityCheck(str) {
  * @return {!Promise}
  */
 function checkoutBranchConfigs(filename, opt_branch) {
+  if (argv.local) {
+    return Promise.resolve();
+  }
   var branch = opt_branch || 'origin/master';
   // One bad path here will fail the whole operation.
   return exec(`git checkout ${branch} ${filename}`)
@@ -61,7 +64,7 @@ function checkoutBranchConfigs(filename, opt_branch) {
  * @return {string}
  */
 function prependConfig(configString, fileString) {
-  return `window.AMP_CONFIG||(window.AMP_CONFIG=${configString});` +
+  return `self.AMP_CONFIG||(self.AMP_CONFIG=${configString});` +
     `/*AMP_CONFIG*/${fileString}`;
 }
 
@@ -93,7 +96,10 @@ function valueOrDefault(value, defaultValue) {
 }
 
 function main() {
-  if (!argv.target) {
+  var TESTING_HOST = process.env.AMP_TESTING_HOST;
+  var target = argv.target || TESTING_HOST;
+
+  if (!target) {
     util.log(util.colors.red('Missing --target.'));
     return;
   }
@@ -105,7 +111,6 @@ function main() {
 
   var globs = [].concat(argv.files).filter(x => typeof x == 'string');
   var branch = argv.branch;
-  var target = argv.target;
   var filename = '';
 
   // Prod by default.
@@ -152,6 +157,7 @@ gulp.task('prepend-global', 'Prepends a json config to a target file', main, {
         'Takes in an optional value for a custom prod config source.',
     'branch': '  Switch to a git branch to get config source from. ' +
         'Uses master by default.',
+    'local': '  Don\'t switch branches and use local config',
   }
 });
 

build-system/tasks/prepend-global/test.js

@@ -25,7 +25,7 @@ var targetFile = 'target-file.js';
 test('sync - prepends global config', t => {
   t.plan(1);
   var res = m.prependConfig('{"hello":"world"}', 'var x = 1 + 1;');
-  t.is(res, 'window.AMP_CONFIG||(window.AMP_CONFIG={"hello":"world"});' +
+  t.is(res, 'self.AMP_CONFIG||(self.AMP_CONFIG={"hello":"world"});' +
       '/*AMP_CONFIG*/var x = 1 + 1;');
 });
 
@@ -39,13 +39,13 @@ test('sync - valueOrDefault', t => {
 
 test('sync - sanityCheck', t => {
   t.plan(3);
-  var badStr = 'window.AMP_CONFIG||(window.AMP_CONFIG={"hello":"world"})' +
+  var badStr = 'self.AMP_CONFIG||(self.AMP_CONFIG={"hello":"world"})' +
       '/*AMP_CONFIG*/' +
-      'window.AMP_CONFIG||(window.AMP_CONFIG={"hello":"world"})' +
+      'self.AMP_CONFIG||(self.AMP_CONFIG={"hello":"world"})' +
       '/*AMP_CONFIG*/' +
       'var x = 1 + 1;';
   var badStr2 = 'var x = 1 + 1;';
-  var goodStr = 'window.AMP_CONFIG||(window.AMP_CONFIG={"hello":"world"})' +
+  var goodStr = 'self.AMP_CONFIG||(self.AMP_CONFIG={"hello":"world"})' +
       '/*AMP_CONFIG*/' +
       'var x = 1 + 1;';
   t.false(m.sanityCheck(badStr));

build-system/tasks/presubmit-checks.js

@@ -438,7 +438,7 @@ var forbiddenTerms = {
       'src/log.js',
     ],
   },
-  '(dev|user)\\(\\)\\.(fine|info|warn|error)\\((?!\\s*([\'"`])?[A-Z-]+([\'"`])?)[^)]*\\)': {
+  '(dev|user)\\(\\)\\.(fine|info|warn|error)\\((?!\\s*([A-Z0-9-]+|[\'"`][A-Z0-9-]+[\'"`]))[^,)\n]*': {
     message: 'Logging message require explicitly `TAG`, or an all uppercase' +
         ' string as the first parameter',
   },

builtins/amp-img.js

@@ -76,9 +76,9 @@ export class AmpImg extends BaseElement {
     // only read "Graphic" when using only 'alt'.
     if (this.element.getAttribute('role') == 'img') {
       this.element.removeAttribute('role');
-      user().error('Setting role=img on amp-img elements breaks screen ' +
-          'readers please just set alt or ARIA attributes, they will be ' +
-          'correctly propagated for the underlying <img> element.');
+      user().error('AMP-IMG', 'Setting role=img on amp-img elements breaks ' +
+        'screen readers please just set alt or ARIA attributes, they will ' +
+        'be correctly propagated for the underlying <img> element.');
     }
 
     this.propagateAttributes(['alt', 'referrerpolicy', 'aria-label',

css/Z_INDEX.md

@@ -9,10 +9,11 @@ selector                                    |   z-index   |   file
 .-amp-element > [overflow]                  |   2         |   css/amp.css
 .-amp-image-lightbox-caption                |   2         |   extensions/amp-image-lightbox/0.1/amp-image-lightbox.css
 .amp-carousel-button                        |   10        |   extensions/amp-carousel/0.1/amp-carousel.css
-amp-sticky-ad                               |   11         |   extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
-amp-app-banner                              |   12        |   extensions/amp-app-banner/0.1/amp-app-banner.css
-.amp-app-banner-dismiss-button              |   13        |   extensions/amp-app-banner/0.1/amp-app-banner.css
-i-amp-app-banner-top-padding                |   14        |   extensions/amp-app-banner/0.1/amp-app-banner.css
+amp-sticky-ad                               |   11        |   extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
+i-amp-sticky-ad-top-padding                 |   12        |   extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
+amp-app-banner                              |   13        |   extensions/amp-app-banner/0.1/amp-app-banner.css
+.amp-app-banner-dismiss-button              |   14        |   extensions/amp-app-banner/0.1/amp-app-banner.css
+i-amp-app-banner-top-padding                |   15        |   extensions/amp-app-banner/0.1/amp-app-banner.css
 amp-image-lightbox                          |   1000      |   extensions/amp-image-lightbox/0.1/amp-image-lightbox.css
 amp-live-list > [update]                    |   1000      |   extensions/amp-live-list/0.1/amp-live-list.css
 amp-user-notification                       |   1000      |   extensions/amp-user-notification/0.1/amp-user-notification.css

css/amp.css

@@ -408,16 +408,6 @@ amp-pixel {
   visibility: hidden;
 }
 
-/**
- * Force the layout box of the ad iframe to be exactly as big as the actual
- * iframe. The `amp-ad` tag itself can be freely styled.
- */
-amp-ad iframe {
-  border: 0 !important;
-  margin: 0 !important;
-  padding: 0 !important;
-}
-
 /**
  * Instagram wraps the standard image into a fixed size container.
  * With these offsets, users can simply specify the the size of the