Skip to content
Permalink
Browse files

Validator rollup (ampproject#23835)

* cl/262363476 Add width and height attributes to the svg SYMBOL tag validation rules.

* cl/262384419 Revision bump for ampproject#23406

* cl/262398573 Revision bump for ampproject#23393

* cl/262415464 Add devmode validation logic.

* cl/262420139 s/amphtml-dev-mode/ampdevmode/ in our error strings.

* cl/262430869 Convert python scripts to be python3 compatible.

* cl/262440817 Revision bump for ampproject#23800

* cl/262442134 Revision bump for ampproject#23826
  • Loading branch information...
Gregable authored and erwinmombay committed Aug 8, 2019
1 parent 2ea05fb commit 786cf5b460b2e18ca500c7bc2f2d176648ab1838
@@ -1,4 +1,4 @@
#!/usr/bin/env python2.7
#!/usr/bin/env python
#
# Copyright 2015 The AMP HTML Authors. All Rights Reserved.
#
@@ -43,7 +43,7 @@ def EnsureNodeJsIsInstalled():

try:
output = subprocess.check_output(['node', '--eval', 'console.log("42")'])
if output.strip() == '42':
if b'42' in output.strip():
return
except (subprocess.CalledProcessError, OSError):
pass
@@ -76,8 +76,8 @@ def CheckPrereqs():
Die('Protobuf compiler not found. Try "apt-get install protobuf-compiler" or follow the install instructions at https://github.com/ampproject/amphtml/blob/master/validator/README.md#installation.')

# Ensure 'libprotoc 2.5.0' or newer.
m = re.search('^(\\w+) (\\d+)\\.(\\d+)\\.(\\d+)', libprotoc_version)
if (m.group(1) != 'libprotoc' or
m = re.search(b'^(\\w+) (\\d+)\\.(\\d+)\\.(\\d+)', libprotoc_version)
if (m.group(1) != b'libprotoc' or
(int(m.group(2)), int(m.group(3)), int(m.group(4))) < (2, 5, 0)):
Die('Expected libprotoc 2.5.0 or newer, saw: %s' % libprotoc_version)

@@ -87,7 +87,15 @@ def CheckPrereqs():
try:
__import__(module)
except ImportError:
Die('%s not found. Try "pip install protobuf" or follow the install instructions at https://github.com/ampproject/amphtml/blob/master/validator/README.md#installation' % module)
# Python3 needs pip3. Python 2 needs pip.
if sys.version_info < (3, 0):
Die('%s not found. Try "pip install protobuf" or follow the install '
'instructions at https://github.com/ampproject/amphtml/blob/master/'
'validator/README.md#installation' % module)
else:
Die('%s not found. Try "pip3 install protobuf" or follow the install '
'instructions at https://github.com/ampproject/amphtml/blob/master/'
'validator/README.md#installation' % module)

# Ensure that yarn is installed.
try:
@@ -337,8 +345,8 @@ def RunSmokeTest(out_dir):
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
(stdout, stderr) = p.communicate()
if ('testdata/feature_tests/minimum_valid_amp.html: PASS\n', '', p.returncode
) != (stdout, stderr, 0):
if (b'testdata/feature_tests/minimum_valid_amp.html: PASS\n', b'',
p.returncode) != (stdout, stderr, 0):
Die('Smoke test failed. returncode=%d stdout="%s" stderr="%s"' %
(p.returncode, stdout, stderr))

@@ -354,8 +362,8 @@ def RunSmokeTest(out_dir):
(stdout, stderr) = p.communicate()
if p.returncode != 1:
Die('smoke test failed. Expected p.returncode==1, saw: %s' % p.returncode)
if not stderr.startswith('testdata/feature_tests/empty.html:1:0 '
'The mandatory tag \'html'):
if not stderr.startswith(b'testdata/feature_tests/empty.html:1:0 '
b'The mandatory tag \'html'):
Die('smoke test failed; stderr was: "%s"' % stderr)
logging.info('... done')

@@ -1295,7 +1295,8 @@ let DescendantConstraints;
* lastChildErrorLineCol: LineCol,
* cdataMatcher: ?CdataMatcher,
* childTagMatcher: ?ChildTagMatcher,
* referencePointMatcher: ?ReferencePointMatcher }}
* referencePointMatcher: ?ReferencePointMatcher,
* devMode: boolean }}
*/
let TagStackEntry;

@@ -1351,6 +1352,7 @@ class TagStack {
cdataMatcher: null,
childTagMatcher: null,
referencePointMatcher: null,
devMode: false,
};
}

@@ -1398,6 +1400,7 @@ class TagStack {
* @private
*/
updateStackEntryFromTagResult_(result, parsedRules, lineCol) {
if (result.devModeSuppress) this.setDevMode();
if (result.bestMatchTagSpec === null) {return;}
const parsedTagSpec = result.bestMatchTagSpec;

@@ -1514,6 +1517,13 @@ class TagStack {
if (matcher !== null) {this.back_().referencePointMatcher = matcher;}
}

/**
* Records that tag currently on the stack is using ampdevmode.
*/
setDevMode() {
this.back_().devMode = true;
}

/**
* @return {?ReferencePointMatcher}
*/
@@ -1562,6 +1572,14 @@ class TagStack {
return this.parentStackEntry_().numChildren;
}

/**
* True if the current stack leaf has dev mode set.
* @return {boolean}
*/
isDevMode() {
return this.parentStackEntry_().devMode;
}

/**
* Tells the parent of the current stack entry that it can only have 1 child
* and that child must be me (the current stack entry).
@@ -2722,6 +2740,14 @@ class Context {
return this.typeIdentifiers_.includes('transformed');
}

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

/**
* Record that this document contains a tag which is a member of a list
* of mandatory alternatives.
@@ -4224,6 +4250,25 @@ function validateAttrDeclaration(
}
}

/**
* Returns true if errors reported on this tag should be suppressed, due to
* 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
// 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;
}
return context.getTagStack().isDevMode();
}

/**
* Validates whether the attributes set on |encountered_tag| conform to this
* tag specification. All mandatory attributes must appear. Only attributes
@@ -4934,6 +4979,7 @@ class ParsedValidatorRules {
this.typeIdentifiers_['amp4email'] = 0;
this.typeIdentifiers_['actions'] = 0;
this.typeIdentifiers_['transformed'] = 0;
this.typeIdentifiers_['ampdevmode'] = 0;

/**
* @type {function(!amp.validator.TagSpec) : boolean}
@@ -5144,7 +5190,8 @@ class ParsedValidatorRules {
// The type identifier "actions" and "transformed" are not considered
// mandatory unlike other type identifiers.
if (typeIdentifier !== 'actions' &&
typeIdentifier !== 'transformed') {
typeIdentifier !== 'transformed' &&
typeIdentifier !== 'ampdevmode') {
hasMandatoryTypeIdentifier = true;
}
// The type identifier "transformed" has restrictions on it's value.
@@ -5162,6 +5209,15 @@ class ParsedValidatorRules {
validationResult);
}
}
if (typeIdentifier === '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.
context.addError(
amp.validator.ValidationError.Code.DEV_MODE_ONLY,
context.getLineCol(), /*params=*/[], /*url*/ '',
validationResult);
}
} else {
context.addError(
amp.validator.ValidationError.Code.DISALLOWED_ATTR,
@@ -5192,21 +5248,22 @@ class ParsedValidatorRules {
switch (this.htmlFormat_) {
case 'AMP':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['', 'amp', 'transformed'], context,
htmlTag.attrs(), ['', 'amp', 'transformed', 'ampdevmode'], context,
validationResult);
break;
case 'AMP4ADS':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['⚡4ads', 'amp4ads'], context, validationResult);
htmlTag.attrs(), ['⚡4ads', 'amp4ads', 'ampdevmode'], context,
validationResult);
break;
case 'AMP4EMAIL':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['⚡4email', 'amp4email'], context,
htmlTag.attrs(), ['⚡4email', 'amp4email', 'ampdevmode'], context,
validationResult);
break;
case 'ACTIONS':
this.validateTypeIdentifiers(
htmlTag.attrs(), ['', 'amp', 'actions'], context,
htmlTag.attrs(), ['', 'amp', 'actions', 'ampdevmode'], context,
validationResult);
if (validationResult.typeIdentifier.indexOf('actions') === -1) {
context.addError(
@@ -5796,6 +5853,7 @@ amp.validator.ValidationHandler =
let resultForReferencePoint = {
bestMatchTagSpec: null,
validationResult: new amp.validator.ValidationResult(),
devModeSuppress: false,
};
resultForReferencePoint.validationResult.status =
amp.validator.ValidationResult.Status.UNKNOWN;
@@ -5812,12 +5870,15 @@ amp.validator.ValidationHandler =
const resultForTag = validateTag(
encounteredTag, resultForReferencePoint.bestMatchTagSpec,
this.context_);
resultForTag.devModeSuppress =
ShouldSuppressDevModeErrors(encounteredTag, this.context_);
// Only merge in the reference point errors into the final result if the
// tag otherwise passes one of the TagSpecs. Otherwise, we end up with
// unnecessarily verbose errors.
if (referencePointMatcher !== null &&
resultForTag.validationResult.status ===
amp.validator.ValidationResult.Status.PASS) {
amp.validator.ValidationResult.Status.PASS &&
!resultForTag.devModeSuppress) {
this.validationResult_.mergeFrom(
resultForReferencePoint.validationResult);
}
@@ -5827,7 +5888,8 @@ amp.validator.ValidationHandler =
resultForReferencePoint.bestMatchTagSpec,
resultForTag.bestMatchTagSpec,
this.context_, resultForTag.validationResult);
this.validationResult_.mergeFrom(resultForTag.validationResult);
if (!resultForTag.devModeSuppress)
this.validationResult_.mergeFrom(resultForTag.validationResult);

this.context_.updateFromTagResults(
encounteredTag, resultForReferencePoint, resultForTag);
@@ -0,0 +1,54 @@
<!--
Copyright 2019 The AMP HTML Authors. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS-IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the license.
-->
<!--
Test Description:
This tests the logic for 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 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>
<head>
<meta charset="utf-8">
<link rel="canonical" href="./regular-html-version.html">
<meta name="viewport" content="width=device-width,minimum-scale=1">
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-autocomplete" src="https://cdn.ampproject.org/v0/amp-autocomplete-0.1.js"></script>
</head>
<body>

<!-- Still produces an error -->
<faketag></faketag>
<anotherfaketag></anotherfaketag>

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

<!-- Also suppresses the entire tree. -->
<faketag 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>

</body>
</html>
@@ -0,0 +1,61 @@
FAIL
| <!--
| Copyright 2019 The AMP HTML Authors. All Rights Reserved.
|
| Licensed under the Apache License, Version 2.0 (the "License");
| you may not use this file except in compliance with the License.
| You may obtain a copy of the License at
|
| http://www.apache.org/licenses/LICENSE-2.0
|
| Unless required by applicable law or agreed to in writing, software
| distributed under the License is distributed on an "AS-IS" BASIS,
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
| See the License for the specific language governing permissions and
| limitations under the license.
| -->
| <!--
| Test Description:
| This tests the logic for 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 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>
>> ^~~~~~~~~
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]
| <head>
| <meta charset="utf-8">
| <link rel="canonical" href="./regular-html-version.html">
| <meta name="viewport" content="width=device-width,minimum-scale=1">
| <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
| <script async src="https://cdn.ampproject.org/v0.js"></script>
| <script async custom-element="amp-autocomplete" src="https://cdn.ampproject.org/v0/amp-autocomplete-0.1.js"></script>
| </head>
| <body>
|
| <!-- Still produces an error -->
| <faketag></faketag>
>> ^~~~~~~~~
feature_tests/dev_mode.html:38:2 The tag 'faketag' is disallowed. [DISALLOWED_HTML]
| <anotherfaketag></anotherfaketag>
>> ^~~~~~~~~
feature_tests/dev_mode.html:39:2 The tag 'anotherfaketag' is disallowed. [DISALLOWED_HTML]
|
| <!-- Tagged with ampdevmode, so errors are suppressed. -->
| <faketag ampdevmode></faketag>
|
| <!-- Also suppresses the entire tree. -->
| <faketag 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>
|
| </body>
| </html>

0 comments on commit 786cf5b

Please sign in to comment.
You can’t perform that action at this time.