Validator rollup (ampproject#20961)

- cl/234811475 Revision bump for ampproject#20913 - cl/234805704 Revision bump for ampproject#20911 - cl/234655757 allow render_only_if_payment_method_present for amp-payment-google-button - cl/234162273 Update comment regarding id/name attribute lists - cl/234053286 Update tagspecs using attribute `name` to include blacklist - cl/234038899 Update tagspecs using attribute `id` to include blacklist - cl/234023532 Revision bump for ampproject#20772
noranazmy · Mar 22, 2019 · abeb563 · abeb563
1 parent 7d989f0
commit abeb563
Show file tree

Hide file tree

Showing 15 changed files with 465 additions and 202 deletions.
diff --git a/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii b/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii
@@ -29,11 +29,8 @@ tags: {  # <amp-ad-exit>
   tag_name: "AMP-AD-EXIT"
   requires_extension: "amp-ad-exit"
   requires: "amp-ad-exit configuration JSON"
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
   attr_lists: "extended-amp-global"
+  attr_lists: "mandatory-id-attr"
   child_tags: {
     mandatory_num_child_tags: 1
     child_tag_name_oneof: "SCRIPT"

diff --git a/extensions/amp-app-banner/validator-amp-app-banner.protoascii b/extensions/amp-app-banner/validator-amp-app-banner.protoascii
@@ -34,11 +34,8 @@ tags: {  # <amp-app-banner>
   # validator-main.protoascii and in turn has an "AMP-APP-BANNER"
   # mandatory_parent.
   requires: "amp-app-banner button[open-button]"
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
   attr_lists: "extended-amp-global"
+  attr_lists: "mandatory-id-attr"
   unique: true
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-app-banner"
   amp_layout: {

diff --git a/extensions/amp-bind/validator-amp-bind.protoascii b/extensions/amp-bind/validator-amp-bind.protoascii
@@ -67,10 +67,6 @@ tags: {  # <amp-state>
   attrs: {
     name: "credentials"
   }
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
   attrs: {
     name: "overridable"
   }
@@ -82,6 +78,7 @@ tags: {  # <amp-state>
     }
     blacklisted_value_regex: "__amp_source_origin"
   }
+  attr_lists: "mandatory-id-attr"
   # <amp-bind>
   attrs: { name: "[src]" }
   child_tags: {
@@ -101,11 +98,8 @@ tags: {  # <amp-state>
   disallowed_ancestor: "AMP-LIST"
   disallowed_ancestor: "AMP-STATE"
   disallowed_ancestor: "TEMPLATE"
+  attr_lists: "mandatory-id-attr"
   attr_lists: "optional-src-amp4email"
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
   child_tags: {
     first_child_tag_name_oneof: "SCRIPT"
   }
@@ -124,10 +118,6 @@ tags: { # <amp-state>
     name: "cross-origin"
     value: "amp-viewer-auth-token-via-post"
   }
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
   attrs: {
     name: "overridable"
   }
@@ -139,6 +129,7 @@ tags: { # <amp-state>
     }
     blacklisted_value_regex: "__amp_source_origin"
   }
+  attr_lists: "mandatory-id-attr"
   # <amp-bind>
   attrs: { name: "[src]" }
   child_tags: {
@@ -158,9 +149,6 @@ tags: {  # <amp-bind-macro>
     name: "expression"
     mandatory: true
   }
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
+  attr_lists: "mandatory-id-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-bind"
 }
diff --git a/extensions/amp-inputmask/validator-amp-inputmask.protoascii b/extensions/amp-inputmask/validator-amp-inputmask.protoascii
@@ -47,7 +47,7 @@ tags: {
   }
   attr_lists: "amp-inputmask-common-attr"
   attr_lists: "input-common-attr"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   attrs: { name: "[type]" }
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-inputmask"
 }
@@ -65,7 +65,7 @@ tags: {
   }
   attr_lists: "amp-inputmask-common-attr"
   attr_lists: "input-common-attr"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-inputmask"
 }
 
@@ -82,7 +82,7 @@ tags: {
   }
   attr_lists: "amp-inputmask-common-attr"
   attr_lists: "input-common-attr"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-inputmask"
 }
 
@@ -99,7 +99,7 @@ tags: {
   }
   attr_lists: "amp-inputmask-common-attr"
   attr_lists: "input-common-attr"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-inputmask"
 }
 
@@ -116,7 +116,7 @@ tags: {
   }
   attr_lists: "amp-inputmask-common-attr"
   attr_lists: "input-common-attr"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-inputmask"
 }
 
@@ -133,7 +133,7 @@ tags: {
   }
   attr_lists: "amp-inputmask-common-attr"
   attr_lists: "input-common-attr"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-inputmask"
 }
 

diff --git a/extensions/amp-live-list/validator-amp-live-list.protoascii b/extensions/amp-live-list/validator-amp-live-list.protoascii
@@ -45,14 +45,11 @@ tags: {  # <amp-live-list>
     name: "disabled"
     value: ""
   }
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
   attrs: {
     name: "sort"
     value: "ascending"
   }
+  attr_lists: "mandatory-id-attr"
   reference_points: {
     tag_spec_name: "AMP-LIVE-LIST [update]"
     mandatory: true
@@ -119,9 +116,6 @@ tags: {
   }
   attrs: { name: "data-tombstone" }
   attrs: { name: "data-update-time" }
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
+  attr_lists: "mandatory-id-attr"
   spec_url: "https://www.ampproject.org/docs/reference/components/amp-live-list#items"
 }
diff --git a/...sions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.html b/...sions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.html
@@ -79,6 +79,15 @@
     }
     </script>
   </amp-payment-google-button>
+  <!-- Valid: with render_only_if_payment_method_present -->
+  <amp-payment-google-button
+    render_only_if_payment_method_present="true">
+    <script type="application/json">
+      {
+        "config": "example"
+      }
+    </script>
+  </amp-payment-google-button>
   <!-- Invalid: missing mandatory child script tag -->
   <amp-payment-google-button></amp-payment-google-button>
 </body>

diff --git a/...nsions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out b/...nsions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out
@@ -80,9 +80,18 @@ FAIL
 |      }
 |      </script>
 |    </amp-payment-google-button>
+|    <!-- Valid: with render_only_if_payment_method_present -->
+|    <amp-payment-google-button
+|      render_only_if_payment_method_present="true">
+|      <script type="application/json">
+|        {
+|          "config": "example"
+|        }
+|      </script>
+|    </amp-payment-google-button>
 |    <!-- Invalid: missing mandatory child script tag -->
 |    <amp-payment-google-button></amp-payment-google-button>
 >>   ^~~~~~~~~
-amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.html:83:2 Tag 'amp-payment-google-button' must have a minimum of 1 child tags - saw 0 child tags. (see https://www.ampproject.org/docs/reference/components/amp-payment-google-button) [GENERIC]
+amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.html:92:2 Tag 'amp-payment-google-button' must have a minimum of 1 child tags - saw 0 child tags. (see https://www.ampproject.org/docs/reference/components/amp-payment-google-button) [GENERIC]
 |  </body>
 |  </html>
diff --git a/extensions/amp-payment-google-button/validator-amp-payment-google-button.protoascii b/extensions/amp-payment-google-button/validator-amp-payment-google-button.protoascii
@@ -29,7 +29,6 @@ tags: {  # <amp-payment-google-button> (json)
   spec_name: "amp-payment-google-button extension .json script"
   requires_extension: "amp-payment-google-button"
   mandatory_parent: "AMP-PAYMENT-GOOGLE-BUTTON"
-  attrs: { name: "name" }
   attrs: {
     disabled_by: "transformed"
     name: "nonce"
@@ -40,6 +39,7 @@ tags: {  # <amp-payment-google-button> (json)
     value_casei: "application/json"
     dispatch_key: NAME_VALUE_PARENT_DISPATCH
   }
+  attr_lists: "name-attr"
   cdata: {
     blacklisted_cdata_regex: {
       regex: "<!--"
@@ -56,6 +56,11 @@ tags: {  # <amp-payment-google-button>
     value_casei: "false"
     value_casei: "true"
   }
+  attrs: {
+    name: "render_only_if_payment_method_present"
+    value_casei: "false"
+    value_casei: "true"
+  }
   child_tags: {
     mandatory_min_num_child_tags: 1
     child_tag_name_oneof: "SCRIPT"

diff --git a/...ions/amp-payment-google-inline-async/validator-amp-payment-google-inline-async.protoascii b/...ions/amp-payment-google-inline-async/validator-amp-payment-google-inline-async.protoascii
@@ -29,7 +29,6 @@ tags: {  # amp-payment-google-inline-async (json)
   spec_name: "amp-payment-google-inline-async extension .json script"
   requires_extension: "amp-payment-google-inline-async"
   mandatory_parent: "AMP-PAYMENT-GOOGLE-INLINE-ASYNC"
-  attrs: { name: "name" }
   attrs: {
     disabled_by: "transformed"
     name: "nonce"
@@ -40,6 +39,7 @@ tags: {  # amp-payment-google-inline-async (json)
     value_casei: "application/json"
     dispatch_key: NAME_VALUE_PARENT_DISPATCH
   }
+  attr_lists: "name-attr"
   cdata: {
     blacklisted_cdata_regex: {
       regex: "<!--"
@@ -57,10 +57,7 @@ tags: {  # <amp-payment-google-inline-async>
     value_casei: "false"
     value_casei: "true"
   }
-  attrs: {
-    name: "name"
-    mandatory: true
-  }
+  attr_lists: "mandatory-name-attr"
   child_tags: {
     mandatory_min_num_child_tags: 1
     child_tag_name_oneof: "SCRIPT"

diff --git a/extensions/amp-payment-google-inline/validator-amp-payment-google-inline.protoascii b/extensions/amp-payment-google-inline/validator-amp-payment-google-inline.protoascii
@@ -29,7 +29,6 @@ tags: {  # <amp-payment-google-inline> (json)
   spec_name: "amp-payment-google-inline extension .json script"
   requires_extension: "amp-payment-google-inline"
   mandatory_parent: "AMP-PAYMENT-GOOGLE-INLINE"
-  attrs: { name: "name" }
   attrs: {
     disabled_by: "transformed"
     name: "nonce"
@@ -40,6 +39,7 @@ tags: {  # <amp-payment-google-inline> (json)
     value_casei: "application/json"
     dispatch_key: NAME_VALUE_PARENT_DISPATCH
   }
+  attr_lists: "name-attr"
   cdata: {
     blacklisted_cdata_regex: {
       regex: "<!--"

diff --git a/extensions/amp-recaptcha-input/validator-amp-recaptcha-input.protoascii b/extensions/amp-recaptcha-input/validator-amp-recaptcha-input.protoascii
@@ -29,10 +29,6 @@ tags: {  # <amp-form> <amp-recaptcha-input>
   mandatory_ancestor: "FORM"
   requires_extension: "amp-form"
   requires_extension: "amp-recaptcha-input"
-  attrs: {
-    name: "name"
-    mandatory: true
-  }
   attrs: {
     name: "data-sitekey"
     mandatory: true
@@ -41,6 +37,7 @@ tags: {  # <amp-form> <amp-recaptcha-input>
     name: "data-action"
     mandatory: true
   }
+  attr_lists: "mandatory-name-attr"
   amp_layout: {
     supported_layouts: NODISPLAY
   }

diff --git a/extensions/amp-selector/validator-amp-selector.protoascii b/extensions/amp-selector/validator-amp-selector.protoascii
@@ -70,7 +70,7 @@ tags: {  # <amp-selector> for AMP (autoplay attribute allowed)
     tag_spec_name: "AMP-SELECTOR child"
   }
   attr_lists: "extended-amp-global"
-  attr_lists: "input-name-attr"
+  attr_lists: "name-attr"
   amp_layout {
     supported_layouts: FILL
     supported_layouts: FIXED

diff --git a/extensions/amp-story/validator-amp-story.protoascii b/extensions/amp-story/validator-amp-story.protoascii
@@ -163,10 +163,7 @@ tags: {  # <amp-story-page>
       protocol: "https"
     }
   }
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
+  attr_lists: "mandatory-id-attr"
   child_tags: {
     child_tag_name_oneof: "AMP-ANALYTICS"
     child_tag_name_oneof: "AMP-PIXEL"
@@ -421,10 +418,7 @@ tags: {  # <amp-story-consent>
   requires_extension: "amp-story"
   mandatory_parent: "AMP-CONSENT"
   requires: "amp-story-consent extension .json script"
-  attrs: {
-    name: "id"
-    mandatory: true
-  }
+  attr_lists: "mandatory-id-attr"
   child_tags: {
     mandatory_num_child_tags: 1
     child_tag_name_oneof: "SCRIPT"

diff --git a/validator/engine/validator_test.js b/validator/engine/validator_test.js
@@ -922,6 +922,19 @@ function attrRuleShouldMakeSense(attrSpec, tagSpec, rules) {
   it('attr_spec only has one value set', () => {
     expect(numValues).toBeLessThan(2);
   });
+  // `id` attribute must have blacklisted_value_regex set if no explicit values.
+  if ((attrSpec.name === 'id') && (numValues === 0)) {
+    it('"id" attribute must have blacklisted_value_regex set', () => {
+      expect(attrSpec.blacklistedValueRegex !== null).toBe(true);
+    });
+  }
+  // `name` attribute must have blacklisted_value_regex set if no explicit
+  // values.
+  if ((attrSpec.name === 'name') && (numValues === 0)) {
+    it('"name" attribute must have blacklisted_value_regex set', () => {
+      expect(attrSpec.blacklistedValueRegex !== null).toBe(true);
+    });
+  }
   // deprecation
   if ((attrSpec.deprecation !== null) || (attrSpec.deprecationUrl !== null)) {
     it('deprecation and deprecation_url must both be defined if one is defined',