Default to ?amp=1 query parameter #4312
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -612,7 +612,7 @@ function amp_redirect_old_slug_to_new_url( $link ) { | |
|
|
||
| if ( amp_is_request() && ! amp_is_canonical() ) { | ||
| if ( ! amp_is_legacy() ) { | ||
| $link = amp_get_url( $link ); | ||
| } else { | ||
| $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); | ||
| } | ||
|
|
@@ -699,16 +699,6 @@ function amp_get_current_url() { | |
| * @return string AMP permalink. | ||
| */ | ||
| function amp_get_permalink( $post_id ) { | ||
| /** | ||
| * Filters the AMP permalink to short-circuit normal generation. | ||
| * | ||
|
|
@@ -727,35 +717,7 @@ function amp_get_permalink( $post_id ) { | |
| } | ||
|
|
||
| $permalink = get_permalink( $post_id ); | ||
| $amp_url = amp_is_canonical() ? $permalink : amp_get_url( $permalink ); | ||
|
|
||
| /** | ||
| * Filters AMP permalink. | ||
|
|
@@ -778,12 +740,20 @@ function amp_get_permalink( $post_id ) { | |
| * @return string URL with AMP stripped. | ||
| */ | ||
| function amp_remove_endpoint( $url ) { | ||
| $slug = amp_get_slug(); | ||
|
|
||
| // Strip endpoint, including /amp/, /amp/amp/, /amp/foo/. | ||
| $url = preg_replace( | ||
| sprintf( | ||
| ':(/%s(/[^/?#]+)?)+(?=/?(\?|#|$)):', | ||
| preg_quote( $slug, ':' ) | ||
| ), | ||
| '', | ||
| $url | ||
| ); | ||
|
|
||
| // Strip query var, including ?amp, ?amp=1, etc. | ||
| $url = remove_query_arg( $slug, $url ); | ||
|
|
||
| return $url; | ||
| } | ||
|
|
@@ -836,7 +806,7 @@ function amp_add_amphtml_link() { | |
| } | ||
|
|
||
| if ( AMP_Theme_Support::is_paired_available() ) { | ||
| $amp_url = amp_get_url( amp_get_current_url() ); | ||
| } else { | ||
| $amp_url = amp_get_permalink( get_queried_object_id() ); | ||
| } | ||
|
|
@@ -899,13 +869,7 @@ function amp_is_request() { | |
| $is_amp_url = ( | ||
| amp_is_canonical() | ||
| || | ||
| amp_has_query_var() | ||
| ); | ||
|
|
||
| // If AMP is not available, then it's definitely not an AMP endpoint. | ||
|
|
@@ -1914,7 +1878,7 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { | |
| } elseif ( is_singular() ) { | ||
| $href = amp_get_permalink( get_queried_object_id() ); // For sake of Reader mode. | ||
| } else { | ||
| $href = amp_get_url( amp_get_current_url() ); | ||
| } | ||
|
|
||
| $href = remove_query_arg( QueryVar::NOAMP, $href ); | ||
|
|
@@ -1949,7 +1913,7 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { | |
| if ( amp_is_legacy() ) { | ||
| $args['href'] = add_query_arg( 'autofocus[panel]', AMP_Template_Customizer::PANEL_ID, $args['href'] ); | ||
| } else { | ||
| $args['href'] = amp_get_url( $args['href'] ); | ||
| } | ||
| $wp_admin_bar->add_node( $args ); | ||
| } | ||
|
|
@@ -1983,3 +1947,39 @@ function amp_generate_script_hash( $script ) { | |
| ); | ||
| return 'sha384-' . $hash; | ||
| } | ||
|
|
||
| /** | ||
| * Get the AMP version of a URL. | ||
| * | ||
| * @since 2.1 | ||
| * @todo Rename this to clear up confusion with amp_get_current_url(). | ||
| * | ||
| * @param string $url URL. | ||
| * @return string AMP URL. | ||
| */ | ||
| function amp_get_url( $url ) { | ||
| return add_query_arg( amp_get_slug(), '1', $url ); | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the current request has the AMP query parameter set. | ||
| * | ||
| * @since 2.1 | ||
| * @todo Rename this to be more agnostic to what the URL contains. | ||
| * | ||
| * @return bool True if the AMP query parameter is set with the required value, false if not. | ||
| * @global WP_Query $wp_query | ||
| */ | ||
| function amp_has_query_var() { | ||
|
There was a problem hiding this comment. Given that in the future a paired AMP URL will be able to be indicated arbitrarily, such as via a subdomain, I think we can anticipate that by making We have Maybe There was a problem hiding this comment. Thinking about this some more… what about While in a WordPress context, an “endpoint” refers to the end of the URL, in reality an API endpoint doesn't necessarily refer to any physical part of the URL. Normally the endpoint will still be at the end ( There was a problem hiding this comment. I suppose then we'd rename There was a problem hiding this comment. We basically need CRUD functions for manipulating paired AMP URLs:
There was a problem hiding this comment. So:
Thoughts? There was a problem hiding this comment.
That reasoning makes sense. I'm also in support of renaming the functions to represent the CRUD methods of manipulating the paired URL 👍. Sidenote: That means There was a problem hiding this comment. Here, check this out: d5610f2 |
||
| global $wp_query; | ||
| $slug = amp_get_slug(); | ||
| return ( | ||
| isset( $_GET[ $slug ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended | ||
| || | ||
| ( | ||
| $wp_query instanceof WP_Query | ||
| && | ||
| false !== $wp_query->get( $slug, false ) | ||
| ) | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have
amp_get_current_url()andamp_get_asset_url()andamp_get_customizer_url(). These being here, I thinkamp_get_url()is too ambiguous.How about we call it
amp_get_paired_url()since its purpose is to get the paired AMP URL for the current page.We also have a
\AMP_Theme_Support::get_current_canonical_url()which could be extracted into a separate global function calledamp_get_canonical_url().There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in b66c233