From 50cd66e9757d203f604e83a204b4600f281b4eb1 Mon Sep 17 00:00:00 2001 From: Amaltas Bohra Date: Wed, 20 Feb 2019 10:57:27 -0800 Subject: [PATCH 1/7] cl/234023532 Revision bump for #20772 --- validator/validator-main.protoascii | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii index e66967effca7..8ba9accf0d45 100644 --- a/validator/validator-main.protoascii +++ b/validator/validator-main.protoascii @@ -26,7 +26,7 @@ min_validator_revision_required: 375 # 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: 822 +spec_file_revision: 823 styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages" script_spec_url: "https://www.ampproject.org/docs/reference/spec#html-tags" From 8223b637de9a37646bbefc3112f297f63873efe4 Mon Sep 17 00:00:00 2001 From: honeybadgerdontcare Date: Wed, 20 Feb 2019 11:00:56 -0800 Subject: [PATCH 2/7] cl/234038899 Update tagspecs using attribute `id` to include blacklist --- .../validator-amp-ad-exit.protoascii | 5 +- .../validator-amp-app-banner.protoascii | 5 +- .../amp-bind/validator-amp-bind.protoascii | 20 +-- .../validator-amp-live-list.protoascii | 10 +- .../amp-story/validator-amp-story.protoascii | 10 +- validator/engine/validator_test.js | 6 + validator/validator-main.protoascii | 134 +++++++++++++++++- 7 files changed, 149 insertions(+), 41 deletions(-) diff --git a/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii b/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii index 2b9552d595af..e9560c2ab68c 100644 --- a/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii +++ b/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii @@ -29,10 +29,7 @@ tags: { # tag_name: "AMP-AD-EXIT" requires_extension: "amp-ad-exit" requires: "amp-ad-exit configuration JSON" - attrs: { - name: "id" - mandatory: true - } + attr_lists: "mandatory-id" attr_lists: "extended-amp-global" child_tags: { mandatory_num_child_tags: 1 diff --git a/extensions/amp-app-banner/validator-amp-app-banner.protoascii b/extensions/amp-app-banner/validator-amp-app-banner.protoascii index 3c3f4f989798..401e3b48ad22 100644 --- a/extensions/amp-app-banner/validator-amp-app-banner.protoascii +++ b/extensions/amp-app-banner/validator-amp-app-banner.protoascii @@ -34,10 +34,7 @@ tags: { # # 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: "mandatory-id" attr_lists: "extended-amp-global" unique: true spec_url: "https://www.ampproject.org/docs/reference/components/amp-app-banner" diff --git a/extensions/amp-bind/validator-amp-bind.protoascii b/extensions/amp-bind/validator-amp-bind.protoascii index df4377842509..5be2cbe08cd9 100644 --- a/extensions/amp-bind/validator-amp-bind.protoascii +++ b/extensions/amp-bind/validator-amp-bind.protoascii @@ -67,10 +67,6 @@ tags: { # attrs: { name: "credentials" } - attrs: { - name: "id" - mandatory: true - } attrs: { name: "overridable" } @@ -82,6 +78,7 @@ tags: { # } blacklisted_value_regex: "__amp_source_origin" } + attr_lists: "mandatory-id" # attrs: { name: "[src]" } child_tags: { @@ -101,11 +98,8 @@ tags: { # disallowed_ancestor: "AMP-LIST" disallowed_ancestor: "AMP-STATE" disallowed_ancestor: "TEMPLATE" + attr_lists: "mandatory-id" attr_lists: "optional-src-amp4email" - attrs: { - name: "id" - mandatory: true - } child_tags: { first_child_tag_name_oneof: "SCRIPT" } @@ -124,10 +118,6 @@ tags: { # name: "cross-origin" value: "amp-viewer-auth-token-via-post" } - attrs: { - name: "id" - mandatory: true - } attrs: { name: "overridable" } @@ -139,6 +129,7 @@ tags: { # } blacklisted_value_regex: "__amp_source_origin" } + attr_lists: "mandatory-id" # attrs: { name: "[src]" } child_tags: { @@ -158,9 +149,6 @@ tags: { # name: "expression" mandatory: true } - attrs: { - name: "id" - mandatory: true - } + attr_lists: "mandatory-id" spec_url: "https://www.ampproject.org/docs/reference/components/amp-bind" } diff --git a/extensions/amp-live-list/validator-amp-live-list.protoascii b/extensions/amp-live-list/validator-amp-live-list.protoascii index 7db714e1df00..68840ae83593 100644 --- 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: { # name: "disabled" value: "" } - attrs: { - name: "id" - mandatory: true - } attrs: { name: "sort" value: "ascending" } + attr_lists: "mandatory-id" 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" spec_url: "https://www.ampproject.org/docs/reference/components/amp-live-list#items" } diff --git a/extensions/amp-story/validator-amp-story.protoascii b/extensions/amp-story/validator-amp-story.protoascii index 56f853ac50f1..a55814f80238 100644 --- a/extensions/amp-story/validator-amp-story.protoascii +++ b/extensions/amp-story/validator-amp-story.protoascii @@ -163,10 +163,7 @@ tags: { # protocol: "https" } } - attrs: { - name: "id" - mandatory: true - } + attr_lists: "mandatory-id" child_tags: { child_tag_name_oneof: "AMP-ANALYTICS" child_tag_name_oneof: "AMP-PIXEL" @@ -421,10 +418,7 @@ tags: { # requires_extension: "amp-story" mandatory_parent: "AMP-CONSENT" requires: "amp-story-consent extension .json script" - attrs: { - name: "id" - mandatory: true - } + attr_lists: "mandatory-id" 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 index f75be99228cc..0daf1d462f49 100644 --- a/validator/engine/validator_test.js +++ b/validator/engine/validator_test.js @@ -922,6 +922,12 @@ 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); + }); + } // deprecation if ((attrSpec.deprecation !== null) || (attrSpec.deprecationUrl !== null)) { it('deprecation and deprecation_url must both be defined if one is defined', diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii index 8ba9accf0d45..346fa53a460b 100644 --- a/validator/validator-main.protoascii +++ b/validator/validator-main.protoascii @@ -26,7 +26,7 @@ min_validator_revision_required: 375 # 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: 823 +spec_file_revision: 824 styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages" script_spec_url: "https://www.ampproject.org/docs/reference/spec#html-tags" @@ -5267,6 +5267,136 @@ attr_lists: { } } +# Mandatory attribute "id". +attr_lists: { + name: "mandatory-id" + attrs: { + name: "id" + mandatory: true + # When updating this blacklisted_value_regex, also update the list in + # $GLOBAL_ATTRS for "id". + blacklisted_value_regex: "(^|\\s)(" # Values are space separated + "__amp_\\S*|" + "__count__|" + "__defineGetter__|" + "__defineSetter__|" + "__lookupGetter__|" + "__lookupSetter__|" + "__noSuchMethod__|" + "__parent__|" + "__proto__|" + "__AMP_\\S*|" + "\\$p|" + "\\$proxy|" + "acceptCharset|" + "addEventListener|" + "appendChild|" + "assignedSlot|" + "attachShadow|" + "AMP|" + "baseURI|" + "checkValidity|" + "childElementCount|" + "childNodes|" + "classList|" + "className|" + "clientHeight|" + "clientLeft|" + "clientTop|" + "clientWidth|" + "compareDocumentPosition|" + "computedName|" + "computedRole|" + "contentEditable|" + "createShadowRoot|" + "enqueAction|" + "firstChild|" + "firstElementChild|" + "getAnimations|" + "getAttribute|" + "getAttributeNS|" + "getAttributeNode|" + "getAttributeNodeNS|" + "getBoundingClientRect|" + "getClientRects|" + "getDestinationInsertionPoints|" + "getElementsByClassName|" + "getElementsByTagName|" + "getElementsByTagNameNS|" + "getRootNode|" + "hasAttribute|" + "hasAttributeNS|" + "hasAttributes|" + "hasChildNodes|" + "hasPointerCapture|" + "i-amphtml-\\S*|" + "innerHTML|" + "innerText|" + "inputMode|" + "insertAdjacentElement|" + "insertAdjacentHTML|" + "insertAdjacentText|" + "isContentEditable|" + "isDefaultNamespace|" + "isEqualNode|" + "isSameNode|" + "lastChild|" + "lastElementChild|" + "lookupNamespaceURI|" + "namespaceURI|" + "nextElementSibling|" + "nextSibling|" + "nodeName|" + "nodeType|" + "nodeValue|" + "offsetHeight|" + "offsetLeft|" + "offsetParent|" + "offsetTop|" + "offsetWidth|" + "outerHTML|" + "outerText|" + "ownerDocument|" + "parentElement|" + "parentNode|" + "previousElementSibling|" + "previousSibling|" + "querySelector|" + "querySelectorAll|" + "releasePointerCapture|" + "removeAttribute|" + "removeAttributeNS|" + "removeAttributeNode|" + "removeChild|" + "removeEventListener|" + "replaceChild|" + "reportValidity|" + "requestPointerLock|" + "scrollHeight|" + "scrollIntoView|" + "scrollIntoViewIfNeeded|" + "scrollLeft|" + "scrollWidth|" + "setAttribute|" + "setAttributeNS|" + "setAttributeNode|" + "setAttributeNodeNS|" + "setPointerCapture|" + "shadowRoot|" + "styleMap|" + "tabIndex|" + "tagName|" + "textContent|" + "toString|" + "valueOf|" + "(webkit|ms|moz|o)dropzone|" + "(webkit|moz|ms|o)MatchesSelector|" + "(webkit|moz|ms|o)RequestFullScreen|" + "(webkit|moz|ms|o)RequestFullscreen" + ")(\\s|$)" + } +} + # Global attributes: These can be used in all tags. attr_lists: { name: "$GLOBAL_ATTRS" @@ -5560,6 +5690,8 @@ attr_lists: { } attrs: { name: "id" + # When updating this blacklisted_value_regex, also update the list in + # attr_lists: { name: "mandatory-id" }. blacklisted_value_regex: "(^|\\s)(" # Values are space separated "__amp_\\S*|" "__count__|" From 7750e5908c0613dbc6fd76a2c507918b3d04193d Mon Sep 17 00:00:00 2001 From: honeybadgerdontcare Date: Wed, 20 Feb 2019 11:02:59 -0800 Subject: [PATCH 3/7] cl/234053286 Update tagspecs using attribute `name` to include blacklist --- .../validator-amp-ad-exit.protoascii | 2 +- .../validator-amp-app-banner.protoascii | 2 +- .../amp-bind/validator-amp-bind.protoascii | 8 +- .../validator-amp-inputmask.protoascii | 12 +- .../validator-amp-live-list.protoascii | 4 +- ...dator-amp-payment-google-button.protoascii | 2 +- ...amp-payment-google-inline-async.protoascii | 7 +- ...dator-amp-payment-google-inline.protoascii | 2 +- .../validator-amp-recaptcha-input.protoascii | 5 +- .../validator-amp-selector.protoascii | 2 +- .../amp-story/validator-amp-story.protoascii | 4 +- validator/engine/validator_test.js | 9 +- validator/validator-main.protoascii | 430 ++++++++++++------ 13 files changed, 311 insertions(+), 178 deletions(-) diff --git a/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii b/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii index e9560c2ab68c..b6276e307320 100644 --- a/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii +++ b/extensions/amp-ad-exit/validator-amp-ad-exit.protoascii @@ -29,8 +29,8 @@ tags: { # tag_name: "AMP-AD-EXIT" requires_extension: "amp-ad-exit" requires: "amp-ad-exit configuration JSON" - attr_lists: "mandatory-id" 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 index 401e3b48ad22..419616e6d206 100644 --- a/extensions/amp-app-banner/validator-amp-app-banner.protoascii +++ b/extensions/amp-app-banner/validator-amp-app-banner.protoascii @@ -34,8 +34,8 @@ tags: { # # validator-main.protoascii and in turn has an "AMP-APP-BANNER" # mandatory_parent. requires: "amp-app-banner button[open-button]" - attr_lists: "mandatory-id" 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 index 5be2cbe08cd9..becba8f404f3 100644 --- a/extensions/amp-bind/validator-amp-bind.protoascii +++ b/extensions/amp-bind/validator-amp-bind.protoascii @@ -78,7 +78,7 @@ tags: { # } blacklisted_value_regex: "__amp_source_origin" } - attr_lists: "mandatory-id" + attr_lists: "mandatory-id-attr" # attrs: { name: "[src]" } child_tags: { @@ -98,7 +98,7 @@ tags: { # disallowed_ancestor: "AMP-LIST" disallowed_ancestor: "AMP-STATE" disallowed_ancestor: "TEMPLATE" - attr_lists: "mandatory-id" + attr_lists: "mandatory-id-attr" attr_lists: "optional-src-amp4email" child_tags: { first_child_tag_name_oneof: "SCRIPT" @@ -129,7 +129,7 @@ tags: { # } blacklisted_value_regex: "__amp_source_origin" } - attr_lists: "mandatory-id" + attr_lists: "mandatory-id-attr" # attrs: { name: "[src]" } child_tags: { @@ -149,6 +149,6 @@ tags: { # name: "expression" mandatory: true } - attr_lists: "mandatory-id" + 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 index a1b30a5be636..225a4cef1b8a 100644 --- 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 index 68840ae83593..30e4951cf4e5 100644 --- a/extensions/amp-live-list/validator-amp-live-list.protoascii +++ b/extensions/amp-live-list/validator-amp-live-list.protoascii @@ -49,7 +49,7 @@ tags: { # name: "sort" value: "ascending" } - attr_lists: "mandatory-id" + attr_lists: "mandatory-id-attr" reference_points: { tag_spec_name: "AMP-LIVE-LIST [update]" mandatory: true @@ -116,6 +116,6 @@ tags: { } attrs: { name: "data-tombstone" } attrs: { name: "data-update-time" } - attr_lists: "mandatory-id" + attr_lists: "mandatory-id-attr" spec_url: "https://www.ampproject.org/docs/reference/components/amp-live-list#items" } 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 index cb54efbfe49d..c28ecd3720f8 100644 --- 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: { # (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: { # (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-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out b/extensions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out index eb915711b21f..31ab9aba9bc2 100644 --- a/extensions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out +++ b/extensions/amp-payment-google-button/0.1/test/validator-actions-amp-payment-google-button.out @@ -80,9 +80,18 @@ FAIL | } | | +| +| +| +| | | >> ^~~~~~~~~ -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] | | 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 index c28ecd3720f8..f089d98d1d56 100644 --- a/extensions/amp-payment-google-button/validator-amp-payment-google-button.protoascii +++ b/extensions/amp-payment-google-button/validator-amp-payment-google-button.protoascii @@ -56,6 +56,11 @@ tags: { # 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/validator/validator-main.protoascii b/validator/validator-main.protoascii index 4cee384930eb..e95e16096c4c 100644 --- a/validator/validator-main.protoascii +++ b/validator/validator-main.protoascii @@ -26,7 +26,7 @@ min_validator_revision_required: 375 # 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: 826 +spec_file_revision: 827 styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages" script_spec_url: "https://www.ampproject.org/docs/reference/spec#html-tags" From fba01bdfcd7cbf5d6a8353d2b338d814b95205d0 Mon Sep 17 00:00:00 2001 From: Amaltas Bohra Date: Wed, 20 Feb 2019 11:10:30 -0800 Subject: [PATCH 6/7] cl/234805704 Revision bump for #20911 --- validator/validator-main.protoascii | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii index e95e16096c4c..dde10ac4f80f 100644 --- a/validator/validator-main.protoascii +++ b/validator/validator-main.protoascii @@ -26,7 +26,7 @@ min_validator_revision_required: 375 # 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: 827 +spec_file_revision: 828 styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages" script_spec_url: "https://www.ampproject.org/docs/reference/spec#html-tags" From 641c9ea76d8a2cb623aa0297a6dab6920ddedfdc Mon Sep 17 00:00:00 2001 From: Amaltas Bohra Date: Wed, 20 Feb 2019 11:12:41 -0800 Subject: [PATCH 7/7] cl/234811475 Revision bump for #20913 --- validator/validator-main.protoascii | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii index dde10ac4f80f..bc75562a8a34 100644 --- a/validator/validator-main.protoascii +++ b/validator/validator-main.protoascii @@ -26,7 +26,7 @@ min_validator_revision_required: 375 # 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: 828 +spec_file_revision: 829 styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages" script_spec_url: "https://www.ampproject.org/docs/reference/spec#html-tags"