Skip to content

Commit

Permalink
Validator rollup (#24000)
Browse files Browse the repository at this point in the history
* cl/262591581 Revision bump for #23840

* cl/262631527 Revision bump for #23765

* cl/263233041 data-ampdevmode. Avoid using non-data attributes on non-custom elements.

* cl/263556980 Allow nonce on `<link rel=stylesheet>`.

* cl/263618983 Revision bump for #23775

* cl/263623541 Revision bump for #23873

* cl/263628558 Revision bump for #23866

* cl/263637721 Revision bump for #23867

* cl/263656022 Revision bump for #23967

* fix reverse sync

* fix reverse sync
  • Loading branch information
Greg Grothaus committed Aug 16, 2019
1 parent 7b2162a commit dcaf2ce
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 51 deletions.
Expand Up @@ -188,4 +188,4 @@ amp-date-display/0.1/test/validator-amp-date-display.html:169:2 The attribute 'd
| </amp-date-display>
| </body>
|
| </html>
| </html>
2 changes: 1 addition & 1 deletion extensions/amp-script/0.1/test/validator-amp-script.out
Expand Up @@ -112,4 +112,4 @@ amp-script/0.1/test/validator-amp-script.html:84:2 Custom JavaScript is not allo
amp-script/0.1/test/validator-amp-script.html:89:2 Custom JavaScript is not allowed. (see https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml#html-tags) [CUSTOM_JAVASCRIPT_DISALLOWED]
| document.body.textContent = "Hello World!";
| </script>
| </body>
| </body>
32 changes: 16 additions & 16 deletions validator/engine/validator.js
Expand Up @@ -1518,7 +1518,7 @@ class TagStack {
}

/**
* Records that tag currently on the stack is using ampdevmode.
* Records that tag currently on the stack is using data-ampdevmode.
*/
setDevMode() {
this.back_().devMode = true;
Expand Down Expand Up @@ -2741,11 +2741,11 @@ class Context {
}

/**
* Returns true iff "ampdevmode" is a type identifier in this document.
* Returns true iff "data-ampdevmode" is a type identifier in this document.
* @return {boolean}
*/
isDevMode() {
return this.typeIdentifiers_.includes('ampdevmode');
return this.typeIdentifiers_.includes('data-ampdevmode');
}

/**
Expand Down Expand Up @@ -4252,19 +4252,19 @@ function validateAttrDeclaration(

/**
* Returns true if errors reported on this tag should be suppressed, due to
* ampdevmode annotations.
* data-ampdevmode annotations.
* @param {!amp.htmlparser.ParsedHtmlTag} encounteredTag
* @param {!Context} context
* @return {boolean}
*/
function ShouldSuppressDevModeErrors(encounteredTag, context) {
if (!context.isDevMode()) return false;
// Cannot suppress errors on HTML tag. The "ampdevmode" here is a
// Cannot suppress errors on HTML tag. The "data-ampdevmode" here is a
// type identifier. Suppressing errors here would suppress all errors since
// HTML is the root of the document.
if (encounteredTag.upperName() === 'HTML') return false;
for (const attr of encounteredTag.attrs()) {
if (attr.name === 'ampdevmode') return true;
if (attr.name === 'data-ampdevmode') return true;
}
return context.getTagStack().isDevMode();
}
Expand Down Expand Up @@ -4979,7 +4979,7 @@ class ParsedValidatorRules {
this.typeIdentifiers_['amp4email'] = 0;
this.typeIdentifiers_['actions'] = 0;
this.typeIdentifiers_['transformed'] = 0;
this.typeIdentifiers_['ampdevmode'] = 0;
this.typeIdentifiers_['data-ampdevmode'] = 0;

/**
* @type {function(!amp.validator.TagSpec) : boolean}
Expand Down Expand Up @@ -5191,7 +5191,7 @@ class ParsedValidatorRules {
// mandatory unlike other type identifiers.
if (typeIdentifier !== 'actions' &&
typeIdentifier !== 'transformed' &&
typeIdentifier !== 'ampdevmode') {
typeIdentifier !== 'data-ampdevmode') {
hasMandatoryTypeIdentifier = true;
}
// The type identifier "transformed" has restrictions on it's value.
Expand All @@ -5209,7 +5209,7 @@ class ParsedValidatorRules {
validationResult);
}
}
if (typeIdentifier === 'ampdevmode') {
if (typeIdentifier === 'data-ampdevmode') {
// https://github.com/ampproject/amphtml/issues/20974
// We always emit an error for this type identifier, but it
// suppresses other errors later in the document.
Expand Down Expand Up @@ -5248,23 +5248,23 @@ class ParsedValidatorRules {
switch (this.htmlFormat_) {
case 'AMP':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['⚡', 'amp', 'transformed', 'ampdevmode'], context,
validationResult);
htmlTag.attrs(), ['⚡', 'amp', 'transformed', 'data-ampdevmode'],
context, validationResult);
break;
case 'AMP4ADS':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['⚡4ads', 'amp4ads', 'ampdevmode'], context,
htmlTag.attrs(), ['⚡4ads', 'amp4ads', 'data-ampdevmode'], context,
validationResult);
break;
case 'AMP4EMAIL':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['⚡4email', 'amp4email', 'ampdevmode'], context,
validationResult);
htmlTag.attrs(), ['⚡4email', 'amp4email', 'data-ampdevmode'],
context, validationResult);
break;
case 'ACTIONS':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['⚡', 'amp', 'actions', 'ampdevmode'], context,
validationResult);
htmlTag.attrs(), ['⚡', 'amp', 'actions', 'data-ampdevmode'],
context, validationResult);
if (validationResult.typeIdentifier.indexOf('actions') === -1) {
context.addError(
amp.validator.ValidationError.Code.MANDATORY_ATTR_MISSING,
Expand Down
14 changes: 7 additions & 7 deletions validator/testdata/feature_tests/dev_mode.html
Expand Up @@ -15,15 +15,15 @@
-->
<!--
Test Description:
This tests the logic for ampdevmode attributes.
This tests the logic for data-ampdevmode attributes.
See https://github.com/ampproject/amphtml/issues/20974 for the
original motivation. The idea is that an author can tag a document with
the ampdevmode type identifier, which will emit an error and make
the data-ampdevmode type identifier, which will emit an error and make
the document invalid. However, any other tags marked with the same
identifier will no longer emit errors, regardless of the error type.
-->
<!doctype html>
<html ampdevmode>
<html data-ampdevmode>
<head>
<meta charset="utf-8">
<link rel="canonical" href="./regular-html-version.html">
Expand All @@ -38,17 +38,17 @@
<faketag></faketag>
<anotherfaketag></anotherfaketag>

<!-- Tagged with ampdevmode, so errors are suppressed. -->
<faketag ampdevmode></faketag>
<!-- Tagged with data-ampdevmode, so errors are suppressed. -->
<faketag data-ampdevmode></faketag>

<!-- Also suppresses the entire tree. -->
<faketag ampdevmode>
<faketag data-ampdevmode>
<anotherfaketag></anotherfaketag>
</faketag>

<!-- Yet, it does not prevent the tag from satisfying conditions, such as
the amp-autocomplete-0.1.js being used -->
<amp-autocomplete ampdevmode></amp-autocomplete>
<amp-autocomplete data-ampdevmode></amp-autocomplete>

</body>
</html>
16 changes: 8 additions & 8 deletions validator/testdata/feature_tests/dev_mode.out
Expand Up @@ -16,17 +16,17 @@ FAIL
| -->
| <!--
| Test Description:
| This tests the logic for ampdevmode attributes.
| This tests the logic for data-ampdevmode attributes.
| See https://github.com/ampproject/amphtml/issues/20974 for the
| original motivation. The idea is that an author can tag a document with
| the ampdevmode type identifier, which will emit an error and make
| the data-ampdevmode type identifier, which will emit an error and make
| the document invalid. However, any other tags marked with the same
| identifier will no longer emit errors, regardless of the error type.
| -->
| <!doctype html>
| <html ⚡ ampdevmode>
| <html ⚡ data-ampdevmode>
>> ^~~~~~~~~
feature_tests/dev_mode.html:26:0 Tag 'html' marked with attribute 'ampdevmode'. Validator will suppress errors regarding any other tag with this attribute. [GENERIC]
feature_tests/dev_mode.html:26:0 Tag 'html' marked with attribute 'data-ampdevmode'. Validator will suppress errors regarding any other tag with this attribute. [GENERIC]
| <head>
| <meta charset="utf-8">
| <link rel="canonical" href="./regular-html-version.html">
Expand All @@ -45,17 +45,17 @@ feature_tests/dev_mode.html:38:2 The tag 'faketag' is disallowed. [DISALLOWED_HT
>> ^~~~~~~~~
feature_tests/dev_mode.html:39:2 The tag 'anotherfaketag' is disallowed. [DISALLOWED_HTML]
|
| <!-- Tagged with ampdevmode, so errors are suppressed. -->
| <faketag ampdevmode></faketag>
| <!-- Tagged with data-ampdevmode, so errors are suppressed. -->
| <faketag data-ampdevmode></faketag>
|
| <!-- Also suppresses the entire tree. -->
| <faketag ampdevmode>
| <faketag data-ampdevmode>
| <anotherfaketag></anotherfaketag>
| </faketag>
|
| <!-- Yet, it does not prevent the tag from satisfying conditions, such as
| the amp-autocomplete-0.1.js being used -->
| <amp-autocomplete ampdevmode></amp-autocomplete>
| <amp-autocomplete data-ampdevmode></amp-autocomplete>
|
| </body>
| </html>
14 changes: 7 additions & 7 deletions validator/testdata/feature_tests/no_dev_mode.html
Expand Up @@ -15,15 +15,15 @@
-->
<!--
Test Description:
This tests the logic for ampdevmode attributes.
This tests the logic for data-ampdevmode attributes.
See https://github.com/ampproject/amphtml/issues/20974 for the
original motivation. The idea is that an author can tag a document with
the ampdevmode type identifier, which will emit an error and make
the data-ampdevmode type identifier, which will emit an error and make
the document invalid. However, any other tags marked with the same
identifier will no longer emit errors, regardless of the error type.
Unlike dev_mode.html, this test does not enable dev mode, and should emit
full errors. Other than the lack of `ampdevmode` in the <html> tag,
full errors. Other than the lack of `data-ampdevmode` in the <html> tag,
and some comments, this is the same as dev_mode.html.
-->
<!doctype html>
Expand All @@ -42,14 +42,14 @@
<faketag></faketag>
<anotherfaketag></anotherfaketag>

<!-- Tagged with ampdevmode, but not dev mode in html tag. -->
<faketag ampdevmode></faketag>
<!-- Tagged with data-ampdevmode, but not dev mode in html tag. -->
<faketag data-ampdevmode></faketag>

<faketag ampdevmode>
<faketag data-ampdevmode>
<anotherfaketag></anotherfaketag>
</faketag>

<amp-autocomplete ampdevmode></amp-autocomplete>
<amp-autocomplete data-ampdevmode></amp-autocomplete>

</body>
</html>
16 changes: 7 additions & 9 deletions validator/testdata/feature_tests/no_dev_mode.out
Expand Up @@ -16,15 +16,15 @@ FAIL
| -->
| <!--
| Test Description:
| This tests the logic for ampdevmode attributes.
| This tests the logic for data-ampdevmode attributes.
| See https://github.com/ampproject/amphtml/issues/20974 for the
| original motivation. The idea is that an author can tag a document with
| the ampdevmode type identifier, which will emit an error and make
| the data-ampdevmode type identifier, which will emit an error and make
| the document invalid. However, any other tags marked with the same
| identifier will no longer emit errors, regardless of the error type.
|
| Unlike dev_mode.html, this test does not enable dev mode, and should emit
| full errors. Other than the lack of `ampdevmode` in the <html> tag,
| full errors. Other than the lack of `data-ampdevmode` in the <html> tag,
| and some comments, this is the same as dev_mode.html.
| -->
| <!doctype html>
Expand All @@ -47,22 +47,20 @@ feature_tests/no_dev_mode.html:42:2 The tag 'faketag' is disallowed. [DISALLOWED
>> ^~~~~~~~~
feature_tests/no_dev_mode.html:43:2 The tag 'anotherfaketag' is disallowed. [DISALLOWED_HTML]
|
| <!-- Tagged with ampdevmode, but not dev mode in html tag. -->
| <faketag ampdevmode></faketag>
| <!-- Tagged with data-ampdevmode, but not dev mode in html tag. -->
| <faketag data-ampdevmode></faketag>
>> ^~~~~~~~~
feature_tests/no_dev_mode.html:46:2 The tag 'faketag' is disallowed. [DISALLOWED_HTML]
|
| <faketag ampdevmode>
| <faketag data-ampdevmode>
>> ^~~~~~~~~
feature_tests/no_dev_mode.html:48:2 The tag 'faketag' is disallowed. [DISALLOWED_HTML]
| <anotherfaketag></anotherfaketag>
>> ^~~~~~~~~
feature_tests/no_dev_mode.html:49:4 The tag 'anotherfaketag' is disallowed. [DISALLOWED_HTML]
| </faketag>
|
| <amp-autocomplete ampdevmode></amp-autocomplete>
>> ^~~~~~~~~
feature_tests/no_dev_mode.html:52:2 The attribute 'ampdevmode' may not appear in tag 'amp-autocomplete'. (see https://amp.dev/documentation/components/amp-autocomplete) [DISALLOWED_HTML]
| <amp-autocomplete data-ampdevmode></amp-autocomplete>
|
| </body>
| </html>
5 changes: 3 additions & 2 deletions validator/validator-main.protoascii
Expand Up @@ -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: 936
spec_file_revision: 945

styles_spec_url: "https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/style_pages"
script_spec_url: "https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml#html-tags"
Expand Down Expand Up @@ -310,6 +310,7 @@ tags: {
spec_name: "link rel=stylesheet for fonts"
named_id: LINK_FONT_STYLESHEET
mandatory_parent: "HEAD"
attr_lists: "nonce-attr"
attrs: { name: "async" }
attrs: { name: "crossorigin" } # SRI attribute (https://www.w3.org/TR/SRI/)
attrs: {
Expand Down Expand Up @@ -7348,6 +7349,6 @@ error_formats {
}
error_formats {
code: DEV_MODE_ONLY
format: "Tag 'html' marked with attribute 'ampdevmode'. Validator "
format: "Tag 'html' marked with attribute 'data-ampdevmode'. Validator "
"will suppress errors regarding any other tag with this attribute."
}

0 comments on commit dcaf2ce

Please sign in to comment.