-
Notifications
You must be signed in to change notification settings - Fork 39
URL with ?amp query var unexpectedly fails AMP URL test #113
Comments
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: tl;dr everything works correctly in AMP Bench |
@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. |
@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? |
Uh yes you're right, the validator was stale. Fixing that problem is #97… |
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
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
What's the difference? This is of concern for the AMP WordPress plugin because we are moving to
?amp
for all paired AMP URLs.The text was updated successfully, but these errors were encountered: