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

Validator fails to detect lack of required amp-access-scroll extension script #33801

Open
schlessera opened this issue Apr 13, 2021 · 2 comments

Comments

@schlessera
Copy link
Contributor

What's the issue?

If a site using amp-access adds an integration to the Scroll network, it is required to load the amp-access-scroll extension script (see docs).

However, the validator does not detect this issue as it is missing a rule to recognize this use case. This is clearly an invalid AMP page, though, as the functionality it is trying to use is broken if that dependency is missing.

This also means that the auto-extensions transformer in the NodeJS and PHP toolbox packages will not properly add the required amp-access-scroll extension script automatically, leading to a broken page.

Related issue: https://github.com/ampproject/amp-wp/issues/5999

How do we reproduce the issue?

Here's a playground example which should throw a validation error, as the required amp-access-scroll extension script is not loaded even though it is being used by the markup: Playground example

@alanorozco
Copy link
Member

alanorozco commented Apr 13, 2021

Requiring the extension would be a breaking change at this point, since it would invalidate pages that already lack the extension and kick them off-cache. But it's up to @ampproject/wg-caching to say.

@Gregable
Copy link
Member

@schlessera As Alan pointed out, this risks breaking pages using amp-access-scroll. As a result of the extra effort required to assess the risk, I'm bumping the priority down. One thing that would help as a first step would be to add a test case for this that should return a validator error. Would you be interested in making that PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants