Validator Rollup (#9727)

* Minor cleanup of amp-ad-exit * Disallow amp-ad/amp-embed as children of amp ad containers when data-multi-size is present. * Revision bump
ampproject · Jun 5, 2017 · 7d89b5d · 7d89b5d
1 parent 16d86f7
commit 7d89b5d
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 10 deletions.
diff --git a/extensions/amp-ad-exit/0.1/validator-amp-ad-exit.protoascii b/extensions/amp-ad-exit/0.1/validator-amp-ad-exit.protoascii
@@ -17,11 +17,9 @@
 tags: {  # amp-ad-exit
   html_format: AMP4ADS
   tag_name: "SCRIPT"
-  spec_name: "amp-ad-exit extension .js script"
   satisfies: "amp-ad-exit extension .js script"
   requires: "amp-ad-exit"
   mandatory_parent: "HEAD"
-  unique: true
   extension_spec {
     name: "amp-ad-exit"
     allowed_versions: "0.1"
@@ -47,14 +45,14 @@ tags: {  # amp-ad-exit config JSON
   tag_name: "SCRIPT"
   spec_name: "amp-ad-exit configuration JSON"
   satisfies: "amp-ad-exit configuration JSON"
+  requires: "amp-ad-exit extension .js script"
   mandatory_parent: "AMP-AD-EXIT"
   attrs: { name: "nonce" }
   attrs: {
     name: "type"
-    value: "application/json"
     mandatory: true
+    value: "application/json"
     dispatch_key: true
   }
-  requires: "amp-ad-exit extension .js script"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-ad-exit"
 }
diff --git a/extensions/amp-ad/0.1/test/validator-amp-ad.html b/extensions/amp-ad/0.1/test/validator-amp-ad.html
@@ -26,14 +26,28 @@
   <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>
   <script async src="https://cdn.ampproject.org/v0.js"></script>
   <script async custom-element="amp-ad" src="https://cdn.ampproject.org/v0/amp-ad-0.1.js"></script>
+  <script async custom-element="amp-fx-flying-carpet" src="https://cdn.ampproject.org/v0/amp-fx-flying-carpet-0.1.js"></script>
 </head>
 <body>
-  <!-- Example of a valid amp-ad -->
+  <!-- Valid -->
   <amp-ad width=300 height=250
           type="a9"
           data-aax_size="300x250"
           data-aax_pubname="test123"
           data-aax_src="302">
   </amp-ad>
+  <!-- Valid -->
+  <amp-fx-flying-carpet height="300">
+    <amp-ad height="300" type="foo"></amp-ad>
+  </amp-fx-flying-carpet>
+  <!-- Invalid: amp-ad in an amp ad container with data-multi-size attr -->
+  <amp-fx-flying-carpet height="300">
+    <amp-ad data-multi-size="" height="300" type="foo"></amp-ad>
+  </amp-fx-flying-carpet>
+  <!-- Invalid: amp-ad in an amp ad container with data-multi-size attr -->
+  <amp-fx-flying-carpet height="300">
+    <amp-embed data-multi-size="" height="300" type="foo"></amp-ad>
+  </amp-fx-flying-carpet>
+
 </body>
 </html>
diff --git a/extensions/amp-ad/0.1/test/validator-amp-ad.out b/extensions/amp-ad/0.1/test/validator-amp-ad.out
@@ -1 +1,3 @@
-PASS
+FAIL
+amp-ad/0.1/test/validator-amp-ad.html:45:4 The tag 'amp-ad with data-multi-size attribute' may not appear as a descendant of tag 'amp-fx-flying-carpet'. (see https://www.ampproject.org/docs/reference/components/amp-ad) [AMP_TAG_PROBLEM]
+amp-ad/0.1/test/validator-amp-ad.html:49:4 The tag 'amp-embed with data-multi-size attribute' may not appear as a descendant of tag 'amp-fx-flying-carpet'. (see https://www.ampproject.org/docs/reference/components/amp-ad) [AMP_TAG_PROBLEM]
diff --git a/extensions/amp-ad/0.1/validator-amp-ad.protoascii b/extensions/amp-ad/0.1/validator-amp-ad.protoascii
@@ -66,6 +66,52 @@ tags: {  # <amp-ad>
     supported_layouts: RESPONSIVE
   }
 }
+tags: {  # <amp-ad data-multi-size>
+  html_format: AMP  # Ads are not allowed inside ads
+  tag_name: "AMP-AD"
+  spec_name: "amp-ad with data-multi-size attribute"
+  # Typically we'd require the extension j/s, but for historical reasons
+  # many pages don't have it, so it ends up late loaded and we warn, instead
+  # of making this a validation error.
+  also_requires_tag_warning: "amp-ad extension .js script"
+  # If modifying disallowed_ancestors, then please also edit
+  # extensions/amp-auto-ads/*/placement.js
+  disallowed_ancestor: "AMP-APP-BANNER"
+  disallowed_ancestor: "AMP-SIDEBAR"
+  # amp-ad elements cannot be both children of an amp ad container and have
+  # data-multi-size attribute.
+  disallowed_ancestor: "AMP-CAROUSEL"
+  disallowed_ancestor: "AMP-FX-FLYING-CARPET"
+  disallowed_ancestor: "AMP-LIGHTBOX"
+  disallowed_ancestor: "AMP-STICKY-AD"
+  attrs: { name: "alt" }
+  attrs: {
+    name: "data-multi-size"
+    mandatory: true
+    value: ""
+    dispatch_key: true
+  }
+  attrs: { name: "json" }
+  attrs: {
+    name: "src"
+    value_url: {
+      allowed_protocol: "https"
+      allow_relative: true  # Will be set to false at a future date.
+    }
+    blacklisted_value_regex: "__amp_source_origin"
+  }
+  attrs: { name: "type" mandatory: true }
+  attr_lists: "extended-amp-global"
+  spec_url: "https://www.ampproject.org/docs/reference/components/amp-ad"
+  amp_layout: {
+    supported_layouts: FILL
+    supported_layouts: FIXED
+    supported_layouts: FIXED_HEIGHT
+    supported_layouts: FLEX_ITEM
+    supported_layouts: NODISPLAY
+    supported_layouts: RESPONSIVE
+  }
+}
 tags: {  # <amp-embed>
   html_format: AMP
   tag_name: "AMP-EMBED"
@@ -97,3 +143,48 @@ tags: {  # <amp-embed>
     supported_layouts: RESPONSIVE
   }
 }
+tags: {  # <amp-embed data-multi-size>
+  html_format: AMP
+  tag_name: "AMP-EMBED"
+  spec_name: "amp-embed with data-multi-size attribute"
+  # Typically we'd require the extension j/s, but for historical reasons
+  # many pages don't have it, so it ends up late loaded and we warn, instead
+  # of making this a validation error.
+  also_requires_tag_warning: "amp-ad extension .js script"
+  disallowed_ancestor: "AMP-APP-BANNER"
+  disallowed_ancestor: "AMP-SIDEBAR"
+  # amp-embed elements cannot be both children of an amp ad container and have
+  # data-multi-size attribute.
+  disallowed_ancestor: "AMP-CAROUSEL"
+  disallowed_ancestor: "AMP-FX-FLYING-CARPET"
+  disallowed_ancestor: "AMP-LIGHTBOX"
+  disallowed_ancestor: "AMP-STICKY-AD"
+  attrs: { name: "alt" }
+  attrs: {
+    name: "data-multi-size"
+    mandatory: true
+    value: ""
+    dispatch_key: true
+  }
+  attrs: { name: "json" }
+  attrs: {
+    name: "src"
+    value_url: {
+      allowed_protocol: "https"
+      allow_relative: true  # Will be set to false at a future date.
+    }
+    blacklisted_value_regex: "__amp_source_origin"
+  }
+  attrs: { name: "type" mandatory: true }
+  attr_lists: "extended-amp-global"
+  spec_url: "https://www.ampproject.org/docs/reference/components/amp-ad"
+  amp_layout: {
+    supported_layouts: FILL
+    supported_layouts: FIXED
+    supported_layouts: FIXED_HEIGHT
+    supported_layouts: FLEX_ITEM
+    supported_layouts: NODISPLAY
+    supported_layouts: RESPONSIVE
+  }
+}
+
diff --git a/validator/engine/validator.js b/validator/engine/validator.js
@@ -2676,7 +2676,7 @@ function validateAncestorTags(parsedTagSpec, context, validationResult) {
               context.getDocLocator(),
               /* params */
               [
-                spec.tagName.toLowerCase(), mandatoryAncestor.toLowerCase(),
+                getTagSpecName(spec), mandatoryAncestor.toLowerCase(),
                 spec.mandatoryAncestorSuggestedAlternative.toLowerCase()
               ],
               spec.specUrl, validationResult);
@@ -2686,7 +2686,7 @@ function validateAncestorTags(parsedTagSpec, context, validationResult) {
               amp.validator.ValidationError.Code.MANDATORY_TAG_ANCESTOR,
               context.getDocLocator(),
               /* params */
-              [spec.tagName.toLowerCase(), mandatoryAncestor.toLowerCase()],
+              [getTagSpecName(spec), mandatoryAncestor.toLowerCase()],
               spec.specUrl, validationResult);
         }
       }
@@ -2703,7 +2703,7 @@ function validateAncestorTags(parsedTagSpec, context, validationResult) {
             amp.validator.ValidationError.Code.DISALLOWED_TAG_ANCESTOR,
             context.getDocLocator(),
             /* params */
-            [spec.tagName.toLowerCase(), disallowedAncestor.toLowerCase()],
+            [getTagSpecName(spec), disallowedAncestor.toLowerCase()],
             spec.specUrl, validationResult);
       }
       return;

diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii
@@ -25,7 +25,7 @@ min_validator_revision_required: 232
 # newer versions of the spec file. This is currently a Google internal
 # mechanism, validator.js does not use this facility. However, any
 # change to this file (validator-main.js) requires updating this revision id.
-spec_file_revision: 427
+spec_file_revision: 430
 
 styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages"