diff --git a/extensions/amp-accordion/validator-amp-accordion.protoascii b/extensions/amp-accordion/validator-amp-accordion.protoascii index 3d35a015c418..ef60a92c3781 100644 --- a/extensions/amp-accordion/validator-amp-accordion.protoascii +++ b/extensions/amp-accordion/validator-amp-accordion.protoascii @@ -34,6 +34,8 @@ tags: { # amp-accordion name: "amp-accordion" # AMP4EMAIL doesn't allow version: "latest". version: "0.1" + requires_usage: EXEMPTED + deprecated_allow_duplicates: true } attr_lists: "common-extension-attrs" } diff --git a/extensions/amp-autocomplete/validator-amp-autocomplete.protoascii b/extensions/amp-autocomplete/validator-amp-autocomplete.protoascii index d408242391f1..4f4cbdcfb421 100644 --- a/extensions/amp-autocomplete/validator-amp-autocomplete.protoascii +++ b/extensions/amp-autocomplete/validator-amp-autocomplete.protoascii @@ -105,7 +105,6 @@ tags: { # attrs: { name: "highlight-user-entry" } attrs: { name: "inline" } attrs: { name: "items" } - attrs: { name: "max-entries" } attrs: { name: "max-items" } attrs: { name: "min-characters" } attrs: { diff --git a/extensions/amp-fit-text/validator-amp-fit-text.protoascii b/extensions/amp-fit-text/validator-amp-fit-text.protoascii index 4b0f8703e75c..f4b0f12191e7 100644 --- a/extensions/amp-fit-text/validator-amp-fit-text.protoascii +++ b/extensions/amp-fit-text/validator-amp-fit-text.protoascii @@ -35,6 +35,8 @@ tags: { # amp-fit-text (AMP4EMAIL) name: "amp-fit-text" # AMP4EMAIL doesn't allow version: "latest". version: "0.1" + requires_usage: EXEMPTED + deprecated_allow_duplicates: true } attr_lists: "common-extension-attrs" } diff --git a/extensions/amp-form/validator-amp-form.protoascii b/extensions/amp-form/validator-amp-form.protoascii index 6487139d6a68..a34d05681815 100644 --- a/extensions/amp-form/validator-amp-form.protoascii +++ b/extensions/amp-form/validator-amp-form.protoascii @@ -36,6 +36,8 @@ tags: { # amp-form name: "amp-form" # AMP4EMAIL doesn't allow version: "latest". version: "0.1" + deprecated_allow_duplicates: true + requires_usage: EXEMPTED } attr_lists: "common-extension-attrs" } diff --git a/validator/js/engine/parse-css.js b/validator/js/engine/parse-css.js index 8a8c9a0f1c29..8675513a548e 100644 --- a/validator/js/engine/parse-css.js +++ b/validator/js/engine/parse-css.js @@ -353,7 +353,8 @@ const Declaration = class extends Rule { /** * For a declaration, if the first non-whitespace token is an identifier, - * returns its string value. Otherwise, returns the empty string. + * (including a number token type), returns its string value. Otherwise, + * returns the empty string. * @return {string} */ firstIdent() { @@ -364,11 +365,18 @@ const Declaration = class extends Rule { return /** @type {!tokenize_css.StringValuedToken} */ (this.value[0]) .value; } + if (this.value[0].tokenType === tokenize_css.TokenType.NUMBER) { + return /** @type {!tokenize_css.NumberToken} */ (this.value[0]).repr; + } if (this.value.length >= 2 && - (this.value[0].tokenType === tokenize_css.TokenType.WHITESPACE) && - this.value[1].tokenType === tokenize_css.TokenType.IDENT) { - return /** @type {!tokenize_css.StringValuedToken} */ (this.value[1]) - .value; + (this.value[0].tokenType === tokenize_css.TokenType.WHITESPACE)) { + if (this.value[1].tokenType === tokenize_css.TokenType.IDENT) { + return /** @type {!tokenize_css.StringValuedToken} */ (this.value[1]) + .value; + } + if (this.value[1].tokenType === tokenize_css.TokenType.NUMBER) { + return /** @type {!tokenize_css.NumberToken} */ (this.value[1]).repr; + } } return ''; } diff --git a/validator/js/engine/validator.js b/validator/js/engine/validator.js index 8b0d0348b832..4a8cedeb6b22 100644 --- a/validator/js/engine/validator.js +++ b/validator/js/engine/validator.js @@ -2087,28 +2087,62 @@ class InvalidRuleVisitor extends parse_css.RuleVisitor { /** @private */ class InvalidDeclVisitor extends parse_css.RuleVisitor { /** - * @param {!ParsedDocCssSpec} spec + * @param {!ParsedDocCssSpec} cssSpec * @param {!Context} context + * @param {string} tagDescriptiveName * @param {!generated.ValidationResult} result */ - constructor(spec, context, result) { + constructor(cssSpec, context, tagDescriptiveName, result) { super(); /** @type {!ParsedDocCssSpec} */ - this.spec = spec; + this.cssSpec = cssSpec; /** @type {!Context} */ this.context = context; + /** @type {string} */ + this.tagDescriptiveName = tagDescriptiveName; /** @type {!generated.ValidationResult} */ this.result = result; } /** @inheritDoc */ visitDeclaration(declaration) { - if (!this.spec.cssDeclarationByName(declaration.name)) { + const cssDeclaration = this.cssSpec.cssDeclarationByName(declaration.name); + const firstIdent = declaration.firstIdent(); + const lineCol = new LineCol(declaration.line, declaration.col); + if (!cssDeclaration) { this.context.addError( generated.ValidationError.Code.CSS_SYNTAX_INVALID_PROPERTY_NOLIST, - new LineCol(declaration.line, declaration.col), - ['style amp-custom', declaration.name], this.spec.spec().specUrl, - this.result); + lineCol, [this.tagDescriptiveName, declaration.name], + this.cssSpec.spec().specUrl, this.result); + // Don't emit additional errors for this declaration + return; + } + if (cssDeclaration.valueCasei.length > 0) { + let hasValidValue = false; + for (const value of cssDeclaration.valueCasei) { + if (firstIdent.toLowerCase() == value) { + hasValidValue = true; + break; + } + } + if (!hasValidValue) { + // Declaration value not allowed. + this.context.addError( + generated.ValidationError.Code.CSS_SYNTAX_DISALLOWED_PROPERTY_VALUE, + lineCol, /* params */ + [this.tagDescriptiveName, declaration.name, firstIdent], + this.cssSpec.spec().specUrl, this.result); + } + } else if (cssDeclaration.valueRegexCasei != null) { + const valueRegex = this.context.getRules().getFullMatchCaseiRegex( + /** @type {number} */ (cssDeclaration.valueRegexCasei)); + if (!valueRegex.test(firstIdent)) { + this.context.addError( + generated.ValidationError.Code.CSS_SYNTAX_DISALLOWED_PROPERTY_VALUE, + lineCol, /* params */ + [this.tagDescriptiveName, declaration.name, firstIdent], + this.cssSpec.spec().specUrl, this.result); + } } } } @@ -2477,11 +2511,10 @@ class CdataMatcher { parse_css.extractImportantDeclarations(stylesheet, important); for (const decl of important) { context.addError( - generated.ValidationError.Code.CDATA_VIOLATES_DENYLIST, + generated.ValidationError.Code.CSS_SYNTAX_DISALLOWED_IMPORTANT, new LineCol(decl.important_line, decl.important_col), - /* params */ - [getTagDescriptiveName(this.tagSpec_), 'CSS !important'], - getTagSpecUrl(this.tagSpec_), validationResult); + /* params */[], context.getRules().getStylesSpecUrl(), + validationResult); } } @@ -2511,8 +2544,9 @@ class CdataMatcher { // Validate the allowed CSS declarations (eg: `background-color`) if (maybeDocCssSpec !== null && !maybeDocCssSpec.spec().allowAllDeclarationInStyleTag) { - const invalidDeclVisitor = - new InvalidDeclVisitor(maybeDocCssSpec, context, validationResult); + const invalidDeclVisitor = new InvalidDeclVisitor( + maybeDocCssSpec, context, getTagDescriptiveName(this.tagSpec_), + validationResult); stylesheet.accept(invalidDeclVisitor); } @@ -3493,7 +3527,8 @@ class UrlErrorInAttrAdapter { context.addError( generated.ValidationError.Code.INVALID_URL_PROTOCOL, context.getLineCol(), - /* params */[this.attrName_, getTagDescriptiveName(tagSpec), protocol], + /* params */ + [this.attrName_, getTagDescriptiveName(tagSpec), protocol], getTagSpecUrl(tagSpec), result); } @@ -3547,7 +3582,8 @@ function validateAttrValueUrl(parsedAttrSpec, context, attr, tagSpec, result) { } else { context.addError( parseResult.errorCode, context.getLineCol(), - /* params */[attr.name, getTagDescriptiveName(tagSpec), attr.value], + /* params */ + [attr.name, getTagDescriptiveName(tagSpec), attr.value], getTagSpecUrl(tagSpec), result); } return; @@ -4655,12 +4691,14 @@ function validateAttrNotFoundInSpec(parsedTagSpec, context, attrName, result) { context.addError( generated.ValidationError.Code.TEMPLATE_IN_ATTR_NAME, context.getLineCol(), - /* params */[attrName, getTagDescriptiveName(parsedTagSpec.getSpec())], + /* params */ + [attrName, getTagDescriptiveName(parsedTagSpec.getSpec())], context.getRules().getTemplateSpecUrl(), result); } else { context.addError( generated.ValidationError.Code.DISALLOWED_ATTR, context.getLineCol(), - /* params */[attrName, getTagDescriptiveName(parsedTagSpec.getSpec())], + /* params */ + [attrName, getTagDescriptiveName(parsedTagSpec.getSpec())], getTagSpecUrl(parsedTagSpec), result); } } @@ -4902,6 +4940,7 @@ function validateAttrCss( // in the allowed list for this DocCssSpec, and have allowed values if // relevant. for (const declaration of declarations) { + const firstIdent = declaration.firstIdent(); // Allowed declarations vary by context. SVG has its own set of CSS // declarations not supported generally in HTML. const cssDeclaration = parsedAttrSpec.getSpec().valueDocSvgCss === true ? @@ -4919,7 +4958,6 @@ function validateAttrCss( continue; } else if (cssDeclaration.valueCasei.length > 0) { let hasValidValue = false; - const firstIdent = declaration.firstIdent(); for (const value of cssDeclaration.valueCasei) { if (firstIdent.toLowerCase() == value) { hasValidValue = true; @@ -4935,16 +4973,23 @@ function validateAttrCss( [getTagDescriptiveName(tagSpec), declaration.name, firstIdent], context.getRules().getStylesSpecUrl(), result.validationResult); } + } else if (cssDeclaration.valueRegexCasei != null) { + const valueRegex = context.getRules().getFullMatchCaseiRegex( + /** @type {number} */ (cssDeclaration.valueRegexCasei)); + if (!valueRegex.test(firstIdent)) { + context.addError( + generated.ValidationError.Code + .CSS_SYNTAX_DISALLOWED_PROPERTY_VALUE, + context.getLineCol(), /* params */ + [getTagDescriptiveName(tagSpec), declaration.name, firstIdent], + context.getRules().getStylesSpecUrl(), result.validationResult); + } } if (!maybeSpec.spec().allowImportant) { if (declaration.important) - // TODO(gregable): Use a more specific error message for - // `!important` errors. context.addError( - generated.ValidationError.Code.INVALID_ATTR_VALUE, - context.getLineCol(), - /* params */ - [attrName, getTagDescriptiveName(tagSpec), 'CSS !important'], + generated.ValidationError.Code.CSS_SYNTAX_DISALLOWED_IMPORTANT, + context.getLineCol(), /* params */[], context.getRules().getStylesSpecUrl(), result.validationResult); } /** @type {!Array} */ @@ -5030,6 +5075,7 @@ function validateAttrDeclaration( const cssDeclarationByName = parsedAttrSpec.getCssDeclarationByName(); for (const declaration of declarations) { + const firstIdent = declaration.firstIdent(); const declarationName = parse_css.stripVendorPrefix(declaration.name.toLowerCase()); if (!(declarationName in cssDeclarationByName)) { @@ -5043,7 +5089,6 @@ function validateAttrDeclaration( const cssDeclaration = cssDeclarationByName[declarationName]; if (cssDeclaration.valueCasei.length > 0) { let has_valid_value = false; - const firstIdent = declaration.firstIdent(); for (const value of cssDeclaration.valueCasei) { if (firstIdent.toLowerCase() === value) { has_valid_value = true; @@ -5059,6 +5104,17 @@ function validateAttrDeclaration( /* params */[tagSpecName, declaration.name, firstIdent], context.getRules().getStylesSpecUrl(), validationResult); } + } else if (cssDeclaration.valueRegexCasei != null) { + const valueRegex = context.getRules().getFullMatchCaseiRegex( + /** @type {number} */ (cssDeclaration.valueRegexCasei)); + if (!valueRegex.test(firstIdent)) { + context.addError( + generated.ValidationError.Code + .CSS_SYNTAX_DISALLOWED_PROPERTY_VALUE, + context.getLineCol(), /* params */ + /* params */[tagSpecName, declaration.name, firstIdent], + context.getRules().getStylesSpecUrl(), validationResult); + } } } } @@ -5134,7 +5190,8 @@ function validateAttributes( continue; } } else if (attr.name === 'class') { - // For non-transformed AMP, `class` must not contain 'i-amphtml-' prefix. + // For non-transformed AMP, `class` must not contain 'i-amphtml-' + // prefix. validateClassAttr(attr, spec, context, result.validationResult); } @@ -6773,6 +6830,46 @@ const ValidationHandler = } } + /** + * Currently, the Javascript HTML parser considers Doctype to be another HTML + * tag, which is not technically accurate. We have special handling for + * doctype in Javascript which applies to all AMP formats, as this is strict + * handling for all HTML in general. Specifically "attributes" are not + * allowed, even things like `data-foo`. + * @param {!parserInterface.ParsedHtmlTag} encounteredTag + * @param {!Context} context + * @param {!generated.ValidationResult} validationResult + */ + validateDocType(encounteredTag, context, validationResult) { + // - OK + if (encounteredTag.attrs().length === 1 && + encounteredTag.attrs()[0].name === 'html') + return; + // OK + // This is technically invalid. The 'correct' way to do this is to emit the + // lang attribute on the `` tag. However, we observe a number of + // websites incorrectly emitting `lang` as part of doctype, so this specific + // attribute is allowed to avoid breaking existing pages. + if (encounteredTag.attrs().length === 2) { + if (encounteredTag.attrs()[0].name === 'html' && + encounteredTag.attrs()[1].name === 'lang') + return; + if (encounteredTag.attrs()[0].name === 'lang' && + encounteredTag.attrs()[1].name === 'html') + return; + } + if (encounteredTag.attrs().length !== 1 || + encounteredTag.attrs()[0].name !== 'html') { + this.context_.addError( + generated.ValidationError.Code.INVALID_DOCTYPE_HTML, + this.context_.getLineCol(), + /* params */[], + /* specUrl */ 'https://amp.dev/documentation/' + + 'guides-and-tutorials/start/create/basic_markup/', + this.validationResult_); + } + } + /** * Callback for a start HTML tag. * @param {!parserInterface.ParsedHtmlTag} encounteredTag @@ -6783,6 +6880,14 @@ const ValidationHandler = this.context_.getRules().validateHtmlTag( encounteredTag, this.context_, this.validationResult_); } + if (encounteredTag.upperName() === '!DOCTYPE') { + this.validateDocType( + encounteredTag, this.context_, this.validationResult_); + // Even though validateDocType emits all necessary errors about the tag, + // we continue to process it further (validateTag and such) so that we can + // record the tag was present and record it as the root pseudo element for + // the document. + } /** @type {?string} */ const maybeDuplicateAttrName = encounteredTag.hasDuplicateAttrs(); if (maybeDuplicateAttrName !== null) { diff --git a/validator/testdata/amp4ads_feature_tests/doctype.out b/validator/testdata/amp4ads_feature_tests/doctype.out index d70c1127d01b..e952d232dac6 100644 --- a/validator/testdata/amp4ads_feature_tests/doctype.out +++ b/validator/testdata/amp4ads_feature_tests/doctype.out @@ -1,4 +1,4 @@ -PASS +FAIL | | +>> ^~~~~~~~~ +amp4ads_feature_tests/doctype.html:20:0 Invalid or missing doctype declaration. Should be '!doctype html'. (see https://amp.dev/documentation/guides-and-tutorials/start/create/basic_markup/) | | | diff --git a/validator/testdata/amp4ads_feature_tests/doctype.out.cpponly b/validator/testdata/amp4ads_feature_tests/doctype.out.cpponly index dacd8fd3096c..3738b75bfc1e 100644 --- a/validator/testdata/amp4ads_feature_tests/doctype.out.cpponly +++ b/validator/testdata/amp4ads_feature_tests/doctype.out.cpponly @@ -20,7 +20,7 @@ FAIL | --> | >> ^~~~~~~~~ -amp4ads_feature_tests/doctype.html:20:12 Invalid or missing doctype declaration. Should be . See https://amp.dev/documentation/guides-and-tutorials/start/create/basic_markup/ +amp4ads_feature_tests/doctype.html:20:12 Invalid or missing doctype declaration. Should be '!doctype html'. (see https://amp.dev/documentation/guides-and-tutorials/start/create/basic_markup/) | | | diff --git a/validator/testdata/amp4email_feature_tests/amp-autocomplete.html b/validator/testdata/amp4email_feature_tests/amp-autocomplete.html index fd373a928a95..9bd3b9c36c12 100644 --- a/validator/testdata/amp4email_feature_tests/amp-autocomplete.html +++ b/validator/testdata/amp4email_feature_tests/amp-autocomplete.html @@ -74,7 +74,6 @@ highlight-user-entry inline min-characters="0" - max-entries="3" max-items="3" submit-on-enter suggest-first diff --git a/validator/testdata/amp4email_feature_tests/amp-autocomplete.out b/validator/testdata/amp4email_feature_tests/amp-autocomplete.out index a8ae5e5dbba7..ae6b72588ce6 100644 --- a/validator/testdata/amp4email_feature_tests/amp-autocomplete.out +++ b/validator/testdata/amp4email_feature_tests/amp-autocomplete.out @@ -77,7 +77,6 @@ amp4email_feature_tests/amp-autocomplete.html:22:0 The attribute 'data-amp-autoc | highlight-user-entry | inline | min-characters="0" -| max-entries="3" | max-items="3" | submit-on-enter | suggest-first @@ -88,11 +87,11 @@ amp4email_feature_tests/amp-autocomplete.html:22:0 The attribute 'data-amp-autoc | | >> ^~~~~~~~~ -amp4email_feature_tests/amp-autocomplete.html:86:4 The mandatory attribute 'src' is missing in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) +amp4email_feature_tests/amp-autocomplete.html:85:4 The mandatory attribute 'src' is missing in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) | | | @@ -100,7 +99,7 @@ amp4email_feature_tests/amp-autocomplete.html:88:6 Custom JavaScript is not allo | | > ^~~~~~~~~ -amp4email_feature_tests/amp-autocomplete.html:94:4 The attribute 'filter' may not appear in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) +amp4email_feature_tests/amp-autocomplete.html:93:4 The attribute 'filter' may not appear in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) | filter="prefix" | src="https://data.com/articles.json?ref=CANONICAL_URL" | > @@ -110,7 +109,7 @@ amp4email_feature_tests/amp-autocomplete.html:94:4 The attribute 'filter' may no | | > ^~~~~~~~~ -amp4email_feature_tests/amp-autocomplete.html:102:4 The attribute 'filter-value' may not appear in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) +amp4email_feature_tests/amp-autocomplete.html:101:4 The attribute 'filter-value' may not appear in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) | filter-value="property" | src="https://data.com/articles.json?ref=CANONICAL_URL" | > @@ -124,12 +123,12 @@ amp4email_feature_tests/amp-autocomplete.html:102:4 The attribute 'filter-value' |
| > ^~~~~~~~~ -amp4email_feature_tests/amp-autocomplete.html:114:10 The tag 'amp-autocomplete' may not appear as a descendant of tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) +amp4email_feature_tests/amp-autocomplete.html:113:10 The tag 'amp-autocomplete' may not appear as a descendant of tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) | src="https://data.com/articles.json?ref=CANONICAL_URL" | > |