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.

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

Update validator spec for <amp-story-audio-sticker> #542

Closed
swissspidy opened this issue Aug 2, 2023 · 5 comments
Closed

Update validator spec for <amp-story-audio-sticker> #542

swissspidy opened this issue Aug 2, 2023 · 5 comments

Comments

@swissspidy
Copy link
Contributor

This new amp-story-audio-sticker extension has been added recently, see ampproject/amphtml#39184

It doesn't show up on https://cdn.ampproject.org/v0/validator.json yet, but once it does it would be nice to update the spec in this repo so that we could use this new element in the Web Stories for WordPress project.

@westonruter
Copy link
Member

The spec in amp-toolbox-php isn't actually used by Web Stories, is it? Isn't it the spec that is consumed by the AMP plugin for the AMP_Tag_And_Attribute_Sanitizer using the data in AMP_Allowed_Tags_Generated?

Note that fetchpriority is also soon to be allowed by the validator (ampproject/amphtml#38715) so we'll want to update the spec for that as well.

@swissspidy
Copy link
Contributor Author

Ah right. Okay, so in that case the AMP plugin needs to be updated as well.

@westonruter
Copy link
Member

Note: While ampproject/amp-wp#7597 updated the spec in the plugin, the latest AMP release doesn't yet include amp-story-audio-sticker. Just a note to self.

@swissspidy
Copy link
Contributor Author

https://cdn.ampproject.org/v0/validator.json hs now been updated to include this.

Please correct me if I'm wrong, but I think some update in this repo is still required because of the optimizer, which relies on the spec. And the Web Stories plugin uses the optimizer.

@westonruter
Copy link
Member

The AMP plugin actually doesn't use the Optimizer for sanitization or validation (which would depend on #195). It's all still handled internally in the plugin. Unless the Optimizer needs to be updated with special handling to SSR the amp-story-audio-sticker, I don't think the spec has to be updated in this repo for the sake of Web Stories. So this should be addressed by ampproject/amp-wp#7615.

@swissspidy swissspidy closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants