search

ampproject / amp-toolbox-php

AMP Optimizer PHP library
Apache License 2.0
73 stars 25 forks source link

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

Closed swissspidy closed 1 year ago

swissspidy commented 1 year ago

This new amp-story-audio-sticker extension has been added recently, see https://github.com/ampproject/amphtml/pull/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 commented 1 year ago

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 (https://github.com/ampproject/amphtml/issues/38715) so we'll want to update the spec for that as well.

swissspidy commented 1 year ago

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

westonruter commented 1 year ago

Note: While https://github.com/ampproject/amp-wp/pull/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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/ampproject/amp-wp/pull/7615.