ampproject / ampbench

AMPBench: AMP URL validation and troubleshooting tools (DEPRECATED)
Apache License 2.0
66 stars 39 forks source link

URL with ?amp query var unexpectedly fails AMP URL test #113

Closed westonruter closed 5 years ago

westonruter commented 5 years ago

This URL is failing the AMP URL test: https://www.history101.com/vikingfacts/?amp

Results: https://ampbench.appspot.com/validate?url=https%3A%2F%2Fwww.history101.com%2Fvikingfacts%2F%3Famp

image

Nevertheless, this AMP URL also with ?amp is passing the AMP URL test:

https://make.xwp.co/2019/03/15/7-experiences-that-create-team/?amp

Results: https://ampbench.appspot.com/validate?url=https%3A%2F%2Fmake.xwp.co%2F2019%2F03%2F15%2F7-experiences-that-create-team%2F%3Famp

image

What's the difference? This is of concern for the AMP WordPress plugin because we are moving to ?amp for all paired AMP URLs.

swissspidy commented 5 years ago

I think the issue is that the "AMP URL" label is misleading. This is not validating the format of the URL, but actually checks whether that page is valid AMP.

The "FAIL" comes from the AMP validation output that you can see when scrolling down on https://ampbench.appspot.com/validate?url=https%3A%2F%2Fwww.history101.com%2Fvikingfacts%2F%3Famp:

Screenshot 2019-05-04 at 12 10 28

tl;dr everything works correctly in AMP Bench

ithinkihaveacat commented 5 years ago

@swissspidy is correct: the "AMP URL" status is misleading. It doesn't do anything to validating the URL itself (AMP has no requirements here) but whether the document itself is valid. I guess it should read "AMP valid?" or similar. Will create a PR to fix this.

westonruter commented 5 years ago

@ithinkihaveacat Humm. The main validator doesn't report an error:

https://validator.ampproject.org/#url=https%3A%2F%2Fwww.history101.com%2Fvikingfacts%2F%3Famp

Is AMP Bench's validator stale?

ithinkihaveacat commented 5 years ago

Uh yes you're right, the validator was stale. Fixing that problem is #97…