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

✨ FR Validator should reject invalid <script type="application/json"> #15888

Closed
jpettitt opened this issue Jun 6, 2018 · 6 comments
Closed

✨ FR Validator should reject invalid <script type="application/json"> #15888

jpettitt opened this issue Jun 6, 2018 · 6 comments
Assignees
@honeybadgerdontcare
Labels
P3: When Possible Type: Feature Request WG: caching
Milestone
New FRs

Comments

@jpettitt
Copy link
Contributor

jpettitt commented Jun 6, 2018 •
edited
Loading

Invalid JSON in amp component makes pages behave in unexpected and invalid ways. This is a feature request to add JSON validation to the validator and to treat as invalid any AMP page with bad <script type="application/json"> tags.
@jpettitt jpettitt added this to the New FRs milestone Jun 6, 2018
@jpettitt
Copy link
Contributor Author

jpettitt commented Jun 6, 2018

Related to #15885

@newmuis
Copy link
Contributor

newmuis commented Jun 7, 2018

amp-consent and amp-story would benefit from this too.

@jpettitt
Copy link
Contributor Author

jpettitt commented Jun 7, 2018

and amp-analytics

@jpettitt jpettitt mentioned this issue Jun 7, 2018
Closed
@aghassemi
Copy link
Contributor

+1

@honeybadgerdontcare
Copy link
Contributor

We'll start with emitting a warning in the validator when JSON can't be parsed. We can change it to an error when we're comfortable we've given developers enough time to update their JSON.

@Gregable Gregable mentioned this issue Aug 6, 2018
Merged
Gregable added a commit that referenced this issue Aug 6, 2018
@Gregable
Validator Rollup (#17313)
8edcb22 
- Emits a warning if JSON cannot be parsed #15888 
- Add `amp-pan-zoom` to experimental format #17150 
- Clarify `INLINE_STYLE_TOO_LONG` error message.
- Enable the `amp-date-picker` `minimum-nights` attribute #17201, #17220
- `amp-list`: Require `reset-on-refresh="always"` for local data #17221 
- `amp-3d-glt`: add attribute `clearColor` #17085
@Gregable
Copy link
Member

Gregable commented Aug 8, 2019

This was implemented a while ago by @honeybadgerdontcare

See example output in https://github.com/ampproject/amphtml/blob/master/extensions/amp-geo/0.1/test/validator-amp-geo.out

@Gregable Gregable closed this as completed Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@honeybadgerdontcare honeybadgerdontcare

Labels
P3: When Possible Type: Feature Request WG: caching
Projects
None yet
Milestone
New FRs
Development

No branches or pull requests
6 participants
@Gregable @jpettitt @honeybadgerdontcare @newmuis @aghassemi @dreamofabear