Skip to content

Commit

Permalink
Validator rollup (#20961)
Browse files Browse the repository at this point in the history 
 - cl/234811475 Revision bump for #20913
 - cl/234805704 Revision bump for #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 #20772
  • Loading branch information
@amaltas @honeybadgerdontcare
amaltas authored and honeybadgerdontcare committed Feb 20, 2019
1 parent 2277223 commit 6f6d263
Show file tree
Hide file tree
Showing 15 changed files with 465 additions and 202 deletions.
5 changes: 1 addition & 4 deletions extensions/amp-ad-exit/validator-amp-ad-exit.protoascii
View file
Expand Up @@ -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"
Expand Down
5 changes: 1 addition & 4 deletions extensions/amp-app-banner/validator-amp-app-banner.protoascii
View file
Expand Up @@ -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: {
Expand Down
20 changes: 4 additions & 16 deletions extensions/amp-bind/validator-amp-bind.protoascii
View file
Expand Up @@ -67,10 +67,6 @@ tags: { # <amp-state>
attrs: {
name: "credentials"
}
attrs: {
name: "id"
mandatory: true
}
attrs: {
name: "overridable"
}
Expand All @@ -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: {
Expand All @@ -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"
}
Expand All @@ -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"
}
Expand All @@ -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: {
Expand All @@ -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"
}
12 changes: 6 additions & 6 deletions extensions/amp-inputmask/validator-amp-inputmask.protoascii
View file
Expand Up @@ -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"
}
Expand All @@ -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"
}

Expand All @@ -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"
}

Expand All @@ -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"
}

Expand All @@ -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"
}

Expand All @@ -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"
}

Expand Down
10 changes: 2 additions & 8 deletions extensions/amp-live-list/validator-amp-live-list.protoascii
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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"
}
9 changes: 9 additions & 0 deletions ...sions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.html
View file
Expand Up @@ -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>
Expand Down
11 changes: 10 additions & 1 deletion ...nsions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out
View file
Expand Up @@ -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>
7 changes: 6 additions & 1 deletion extensions/amp-payment-google-button/validator-amp-payment-google-button.protoascii
View file
Expand Up @@ -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"
Expand All @@ -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: "<!--"
Expand All @@ -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"
Expand Down
7 changes: 2 additions & 5 deletions ...ions/amp-payment-google-inline-async/validator-amp-payment-google-inline-async.protoascii
View file
Expand Up @@ -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"
Expand All @@ -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: "<!--"
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-payment-google-inline/validator-amp-payment-google-inline.protoascii
View file
Expand Up @@ -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"
Expand All @@ -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: "<!--"
Expand Down
5 changes: 1 addition & 4 deletions extensions/amp-recaptcha-input/validator-amp-recaptcha-input.protoascii
View file
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-selector/validator-amp-selector.protoascii
View file
Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions extensions/amp-story/validator-amp-story.protoascii
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions validator/engine/validator_test.js
View file
Expand Up @@ -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',
Expand Down

0 comments on commit 6f6d263

Please sign in to comment.