From 7d89b5da0e3e347f453f6a42bb8932a7ad18b04a Mon Sep 17 00:00:00 2001 From: Greg Grothaus Date: Mon, 5 Jun 2017 16:13:52 -0700 Subject: [PATCH] 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 --- .../0.1/validator-amp-ad-exit.protoascii | 6 +- .../amp-ad/0.1/test/validator-amp-ad.html | 16 +++- .../amp-ad/0.1/test/validator-amp-ad.out | 4 +- .../amp-ad/0.1/validator-amp-ad.protoascii | 91 +++++++++++++++++++ validator/engine/validator.js | 6 +- validator/validator-main.protoascii | 2 +- 6 files changed, 115 insertions(+), 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 index 8853b0f0e908..541d6cdb7fac 100644 --- 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 index 8667ca688e11..236bd087c92e 100644 --- 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 @@ + - + + + + + + + + + + + + + + diff --git a/extensions/amp-ad/0.1/test/validator-amp-ad.out b/extensions/amp-ad/0.1/test/validator-amp-ad.out index 7ef22e9a431a..0ba4da420e87 100644 --- 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 index 687a7efc6e62..ec6a6400685d 100644 --- 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: { # supported_layouts: RESPONSIVE } } +tags: { # + 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: { # html_format: AMP tag_name: "AMP-EMBED" @@ -97,3 +143,48 @@ tags: { # supported_layouts: RESPONSIVE } } +tags: { # + 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 index 7a1b05ba7b70..12fb79e76966 100644 --- 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 index ce7544a1fda4..7001f6739715 100644 --- 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"