Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AMP Validator: no longer detecting missing mandatory tags #36040

Closed
samhutchins opened this issue Sep 13, 2021 · 5 comments · Fixed by #36054
Closed

AMP Validator: no longer detecting missing mandatory tags #36040

samhutchins opened this issue Sep 13, 2021 · 5 comments · Fixed by #36054
Assignees
@honeybadgerdontcare
Labels
Type: Bug WG: caching

Comments

@samhutchins
Copy link

Description

The AMP validator seems to have stopped detecting missing mandatory tags in some cases, such as the <script async src="https://cdn.ampproject.org/v0.js"></script> tag.

Reproduction Steps

Paste the following HTML into the online validator:

<!--
     This is the minimum valid AMP HTML document. Type away
     here and the AMP Validator will re-check your document on the fly.
-->
<!doctype html>
<html ⚡>
<head>
  <meta charset="utf-8">
  <link rel="canonical" href="self.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>
  -->
</head>
<body>Hello, AMP world.</body>
</html>

Note the commented out <script async src="https://cdn.ampproject.org/v0.js"></script> line. I'd expect this to fail, but it passes

Relevant Logs

No response

Browser(s) Affected

No response

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

No response
@honeybadgerdontcare
Copy link
Contributor

This is intended behavior. It was decided with issue #24232 to make documents without any JavaScript as Valid AMP. This was rolled out in PR #35885.

@honeybadgerdontcare
Copy link
Contributor

@samhutchins reopening as the comment should be updated from:
minimum valid AMP HTML document
to
minimum valid AMP HTML document using JavaScript

@honeybadgerdontcare honeybadgerdontcare self-assigned this Sep 13, 2021
@honeybadgerdontcare honeybadgerdontcare mentioned this issue Sep 13, 2021
Merged
@samhutchins
Copy link
Author

@honeybadgerdontcare The documentation here still lists the amp script as mandatory, could that be updated as well?

@honeybadgerdontcare
Copy link
Contributor

@honeybadgerdontcare The documentation here still lists the amp script as mandatory, could that be updated as well?

I'm not confident that we would want to update that documentation as most developers reading it will want to use the AMP Runtime and that script is required for AMP Runtime. Could you file a separate issue to amp.dev repo for it? This issue will cover the validator related texts.

@honeybadgerdontcare
Copy link
Contributor

https://validator.amp.dev/ now reflects the change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@honeybadgerdontcare honeybadgerdontcare

Labels
Type: Bug WG: caching
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update text to minimum valid JavaScript AMP honeybadgerdontcare/amphtml
2 participants
@honeybadgerdontcare @samhutchins