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.

Sign up for GitHub

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

Jump to bottom

Check for /amp/ suffix when determining is_amp_request #9588

Closed
gravityrail opened this issue May 20, 2018 · 2 comments
Closed

Check for /amp/ suffix when determining is_amp_request #9588

gravityrail opened this issue May 20, 2018 · 2 comments
Labels
AMP [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Comments

@gravityrail
Copy link
Contributor

As per @westonruter's comment on the AMP support patch.

Until ampproject/amp-wp#1148 is implemented, this should also check if the URL path ends in /amp/. Otherwise, AMP URLs on sites with amp theme support like https://make.xwp.co/2018/05/03/wordpress-amp-plugin-0-7-release/amp/ won't get recognized as an AMP request here.
@gravityrail gravityrail mentioned this issue May 20, 2018
Merged
@jeherve jeherve added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label May 21, 2018
@westonruter
Copy link
Contributor

Note that ampproject/amp-wp#1148 has been resolved. Only the ?amp URL param will be used when amp theme support is present.

@stale
Copy link

stale bot commented Dec 8, 2018

This issue has been marked as stale. This happened because:

  • It has been inactive in the past 6 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@stale stale bot added the [Status] Stale label Dec 8, 2018
@westonruter westonruter mentioned this issue Dec 10, 2018
Closed
@gravityrail gravityrail mentioned this issue Dec 11, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
AMP [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix checks for serving AMP response and improve AMP compatibility westonruter/jetpack
4 participants
@gravityrail @westonruter @jeherve @anomiex