Amp-wp: AMP Helper Function Chooses Not To Append Query Value - ($use_query_var) Evaluates To False

[ospira]

Bug Description

On amp-helper-functions.php in the amp_get_permalink() function the $use_query_var conditional evaluates to false where I would expect it should evaluate to true. I have a custom permalink structure using the permalink settings in WordPress - /%category%/%postname%/. I don't understand why the $use_query_var conditional is structured in this way, as using /amp instead of /?amp can cause WordPress redirection issues - it thinks amp is part of the post slug and redirects it to a post (the wrong post - and not AMP). Oddly, this bug only occurred for me on a multisite, where the multisite was appended to the url as a directory, so like this:

/my-multisite-identifier/%category%/%postname%/amp - causes redirect issue

This broke the amp link and caused redirection issues as described. To be clear, however - the /amp was occurring on my main site too - it just only caused a redirect issue on the site that has its multisite identifier (url string specific to that multisite) as part of the url. I manually removed the $use_query_var conditional to make it always use a query var and this was always reliable and works for me across all my sites in the network - but this modification will be lost on plugin update. So my question is why don't we always use the query var or can we do it in a way that still fires for my use case, as it seems appropriate (and working) there.

Expected Behaviour

I expected the plugin to use a query var not append /amp without the query string character ?.

Steps to reproduce

Issue can't be seen anymore because I fixed it as it was breaking my site and causing search console issues. But if you run a multisite (not a subdomain) with my url scheme it should reproduce for you on the multisite.

Screenshots

Additional context

Thank you very much for all that you do and for your consideration.

• WordPress version: Version 5.3.2
• Plugin version: Version 1.4.4
• Gutenberg plugin version (if applicable): N/A
• AMP plugin template mode: Reader
• PHP version: PHP 7.2.11
• OS: Windows and Amazon AMI
• Browser: All (Server-side Issue)
• Device: All (Server-side Issue)

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[westonruter]

You can force a query var to be used instead of /amp/ by adding this code to a custom plugin or your custom theme's functions.php:

add_filter( 'amp_pre_get_permalink', function( $pre, $post_id ) {
    return add_query_arg( amp_get_slug(), '1', get_permalink( $post_id ) );
}, 10, 2 );

You can then freely update the plugin without the change being reverted.

Note that the plugin is going to move to ?amp=1 being the default instead of /amp/ as of https://github.com/ampproject/amp-wp/issues/2204.

[ospira]

Hi @westonruter - will do! Thanks so much for your answer and for supporting this plugin!

[westonruter]

I think the issue is that the permalinks were not flushed in the endpoint for the multisite instance.

This needs to be accounted for in #2204. Namely, ?amp=1 will become the default and this issue will no longer happen.

[westonruter]

Actually, since we're going to still support /amp/ for legacy installs and as an option for users who want it (as opposed to ?amp=1), then I think this still needs to be addressed. It isn't accounted for completely in #4312 .

[westonruter]

The code in question (which has since been removed) is the following:

https://github.com/ampproject/amp-wp/blob/7fcdabd53ae0ae7d86a5a93b446dd4bfb5fa4046/includes/amp-helper-functions.php#L736-L748

Nevertheless, I can't seem to be able to reproduce this issue. I supplied the custom structure of /%category%/%postname%/:

I did this on a subdirectory multisite install "foo" and I can successfully go to both http://subdirectory-multisite-test.lndo.site/foo/uncategorized/hello-world/amp/ and http://subdirectory-multisite-test.lndo.site/foo/uncategorized/hello-world/?amp. Both URLs work as expected and I get the AMP page.

In any case, when there are issues with rewrite rules, then the recommendation is to opt for the query var(e.g. ?amp=1) which is now the plugin's default behavior.