Skip to content

Commit

Permalink
Merge pull request #1194 from Automattic/query-var
Browse files Browse the repository at this point in the history 
Let query var always be used and always be 'amp' when theme support is added
  • Loading branch information
@westonruter
westonruter authored Jun 3, 2018
2 parents 90acca4 + ab99cd7 commit c8d56ed
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 16 deletions.
24 changes: 21 additions & 3 deletions amp.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ function amp_deactivate() {
function amp_after_setup_theme() {
amp_get_slug(); // Ensure AMP_QUERY_VAR is set.

/**
* Filters whether AMP is enabled on the current site.
*
* Useful if the plugin is network activated and you want to turn it off on select sites.
*
* @since 0.2
*/
if ( false === apply_filters( 'amp_is_enabled', true ) ) {
return;
}
Expand Down Expand Up @@ -183,6 +190,7 @@ function amp_force_query_var_value( $query_vars ) {
* If the request is for an AMP page and this is in 'canonical mode,' redirect to the non-AMP page.
* It won't need this plugin's template system, nor the frontend actions like the 'rel' link.
*
* @deprecated This function is not used when 'amp' theme support is added.
* @global WP_Query $wp_query
* @since 0.2
* @return void
Expand Down Expand Up @@ -311,6 +319,7 @@ function amp_add_frontend_actions() {
* Add post template actions.
*
* @since 0.2
* @deprecated This function is not used when 'amp' theme support is added.
*/
function amp_add_post_template_actions() {
require_once AMP__DIR__ . '/includes/amp-post-template-actions.php';
Expand All @@ -322,15 +331,18 @@ function amp_add_post_template_actions() {
* Add action to do post template rendering at template_redirect action.
*
* @since 0.2
* @since 1.0 The amp_render() function is called at template_redirect action priority 11 instead of priority 10.
* @deprecated This function is not used when 'amp' theme support is added.
*/
function amp_prepare_render() {
add_action( 'template_redirect', 'amp_render' );
add_action( 'template_redirect', 'amp_render', 11 );
}

/**
* Render AMP for queried post.
*
* @since 0.1
* @deprecated This function is not used when 'amp' theme support is added.
*/
function amp_render() {
// Note that queried object is used instead of the ID so that the_preview for the queried post can apply.
Expand All @@ -345,6 +357,8 @@ function amp_render() {
* Render AMP post template.
*
* @since 0.5
* @deprecated This function is not used when 'amp' theme support is added.
*
* @param WP_Post|int $post Post.
* @global WP_Query $wp_query
*/
Expand Down Expand Up @@ -377,6 +391,8 @@ function amp_render_post( $post ) {
/**
* Fires before rendering a post in AMP.
*
* This action is not triggered when 'amp' theme support is present. Instead, you should use 'template_redirect' action and check if `is_amp_endpoint()`.
*
* @since 0.2
*
* @param int $post_id Post ID.
Expand Down Expand Up @@ -412,9 +428,11 @@ function _amp_bootstrap_customizer() {
* Redirects the old AMP URL to the new AMP URL.
* If post slug is updated the amp page with old post slug will be redirected to the updated url.
*
* @param string $link New URL of the post.
* @since 0.5
* @deprecated This function is irrelevant when 'amp' theme support is added.
*
* @return string $link URL to be redirected.
* @param string $link New URL of the post.
* @return string URL to be redirected.
*/
function amp_redirect_old_slug_to_new_url( $link ) {

Expand Down
68 changes: 56 additions & 12 deletions includes/amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,26 @@
/**
* Get the slug used in AMP for the query var, endpoint, and post type support.
*
* The return value can be overridden by previously defining a AMP_QUERY_VAR
* This function always returns 'amp' when 'amp' theme support is present. Otherwise,
* the return value can be overridden by previously defining a AMP_QUERY_VAR
* constant or by adding a 'amp_query_var' filter, but *warning* this ability
* may be deprecated in the future. Normally the slug should be just 'amp'.
*
* @since 0.7
* @since 1.0 The return value is always 'amp' when 'amp' theme support is present, and the 'amp_query_var' filter no longer applies.
*
* @return string Slug used for query var, endpoint, and post type support.
*/
function amp_get_slug() {
if ( current_theme_supports( 'amp' ) ) {
if ( ! defined( 'AMP_QUERY_VAR' ) ) {
define( 'AMP_QUERY_VAR', 'amp' );
} elseif ( 'amp' !== AMP_QUERY_VAR ) {
_doing_it_wrong( __FUNCTION__, esc_html__( 'The AMP_QUERY_VAR constant should only be defined as "amp" when "amp" theme support is present.', 'amp' ), '1.0' );
}
return 'amp';
}

if ( defined( 'AMP_QUERY_VAR' ) ) {
return AMP_QUERY_VAR;
}
Expand All @@ -26,6 +38,8 @@ function amp_get_slug() {
* Warning: This filter may become deprecated.
*
* @since 0.3.2
* @since 1.0 This filter does not apply when 'amp' theme support is present.
*
* @param string $query_var The AMP query variable.
*/
$query_var = apply_filters( 'amp_query_var', 'amp' );
Expand All @@ -39,19 +53,30 @@ function amp_get_slug() {
* Retrieves the full AMP-specific permalink for the given post ID.
*
* @since 0.1
* @since 1.0 The query var 'amp' is always used exclusively when 'amp' theme support is present; the 'amp_pre_get_permalink' and 'amp_get_permalink' filters do not apply.
*
* @param int $post_id Post ID.
*
* @return string AMP permalink.
*/
function amp_get_permalink( $post_id ) {

// When theme support is present, the plain query var should always be used.
if ( current_theme_supports( 'amp' ) ) {
$permalink = get_permalink( $post_id );
if ( ! amp_is_canonical() ) {
$permalink = add_query_arg( 'amp', '', $permalink );
}
return $permalink;
}

/**
* Filters the AMP permalink to short-circuit normal generation.
*
* Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply.
*
* @since 0.4
* @since 1.0 This filter does not apply when 'amp' theme support is present.
*
* @param false $url Short-circuited URL.
* @param int $post_id Post ID.
Expand All @@ -62,22 +87,39 @@ function amp_get_permalink( $post_id ) {
return $pre_url;
}

$permalink = get_permalink( $post_id );

if ( amp_is_canonical() ) {
$amp_url = get_permalink( $post_id );
$amp_url = $permalink;
} else {
$parsed_url = wp_parse_url( get_permalink( $post_id ) );
$structure = get_option( 'permalink_structure' );
if ( empty( $structure ) || ! empty( $parsed_url['query'] ) || is_post_type_hierarchical( get_post_type( $post_id ) ) ) {
$amp_url = add_query_arg( amp_get_slug(), '', get_permalink( $post_id ) );
$parsed_url = wp_parse_url( get_permalink( $post_id ) );
$structure = get_option( 'permalink_structure' );
$use_query_var = (
// If pretty permalinks aren't available, then query var must be used.
empty( $structure )
||
// If there are existing query vars, then always use the amp query var as well.
! empty( $parsed_url['query'] )
||
// If the post type is hierarchical then the /amp/ endpoint isn't available.
is_post_type_hierarchical( get_post_type( $post_id ) )
);
if ( $use_query_var ) {
$amp_url = add_query_arg( amp_get_slug(), '', $permalink );
} else {
$amp_url = trailingslashit( get_permalink( $post_id ) ) . user_trailingslashit( amp_get_slug(), 'single_amp' );
$amp_url = preg_replace( '/#.*/', '', $permalink );
$amp_url = trailingslashit( $amp_url ) . user_trailingslashit( amp_get_slug(), 'single_amp' );
if ( ! empty( $parsed_url['fragment'] ) ) {
$amp_url .= '#' . $parsed_url['fragment'];
}
}
}

/**
* Filters AMP permalink.
*
* @since 0.2
* @since 1.0 This filter does not apply when 'amp' theme support is present.
*
* @param false $amp_url AMP URL.
* @param int $post_id Post ID.
Expand Down Expand Up @@ -166,7 +208,7 @@ function post_supports_amp( $post ) {
/**
* Are we currently on an AMP URL?
*
* Note: will always return `false` if called before the `parse_query` hook.
* @since 1.0 This function can be called before the `parse_query` action because the 'amp' query var is specifically and exclusively used when 'amp' theme support is added.
*
* @return bool Whether it is the AMP endpoint.
*/
Expand All @@ -175,15 +217,17 @@ function is_amp_endpoint() {
return false;
}

if ( amp_is_canonical() ) {
// When 'amp' theme support is (or will be added) then these are the conditions that are key to be checked.
if ( amp_is_canonical() || isset( $_GET[ amp_get_slug() ] ) ) { // WPCS: CSRF OK.
return true;
}

if ( 0 === did_action( 'parse_query' ) ) {
_doing_it_wrong( __FUNCTION__, sprintf( esc_html__( "is_amp_endpoint() was called before the 'parse_query' hook was called. This function will always return 'false' before the 'parse_query' hook is called.", 'amp' ) ), '0.4.2' );
// Condition for non-theme support when /amp/ endpoint is used.
if ( false !== get_query_var( amp_get_slug(), false ) ) {
return true;
}

return false !== get_query_var( amp_get_slug(), false );
return false;
}

/**
Expand Down
62 changes: 61 additions & 1 deletion tests/test-amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ public function test_amp_get_permalink_without_pretty_permalinks() {
remove_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10 );
$url = amp_get_permalink( $published_post );
$this->assertContains( 'current_filter=amp_get_permalink', $url );
remove_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ) );

// Now check with theme support added (in paired mode).
add_theme_support( 'amp', array( 'template_dir' => './' ) );
$this->assertStringEndsWith( '&amp', amp_get_permalink( $published_post ) );
$this->assertStringEndsWith( '&amp', amp_get_permalink( $drafted_post ) );
$this->assertStringEndsWith( '&amp', amp_get_permalink( $published_page ) );
add_filter( 'amp_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$this->assertNotContains( 'current_filter=amp_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply.
add_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$this->assertNotContains( 'current_filter=amp_pre_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply.
}

/**
Expand All @@ -101,6 +112,10 @@ public function test_amp_get_permalink_with_pretty_permalinks() {
$wp_rewrite->init();
$wp_rewrite->flush_rules();

$add_anchor_fragment = function( $url ) {
return $url . '#anchor';
};

$drafted_post = $this->factory()->post->create( array(
'post_name' => 'draft',
'post_status' => 'draft',
Expand All @@ -114,11 +129,14 @@ public function test_amp_get_permalink_with_pretty_permalinks() {
'post_status' => 'publish',
'post_type' => 'page',
) );

$this->assertStringEndsWith( '&amp', amp_get_permalink( $drafted_post ) );
$this->assertStringEndsWith( '/amp/', amp_get_permalink( $published_post ) );
$this->assertStringEndsWith( '?amp', amp_get_permalink( $published_page ) );

add_filter( 'post_link', $add_anchor_fragment );
$this->assertStringEndsWith( '/amp/#anchor', amp_get_permalink( $published_post ) );
remove_filter( 'post_link', $add_anchor_fragment );

add_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
add_filter( 'amp_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$url = amp_get_permalink( $published_post );
Expand All @@ -128,6 +146,21 @@ public function test_amp_get_permalink_with_pretty_permalinks() {
remove_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10 );
$url = amp_get_permalink( $published_post );
$this->assertContains( 'current_filter=amp_get_permalink', $url );
remove_filter( 'amp_get_permalink', array( $this, 'return_example_url' ), 10 );

// Now check with theme support added (in paired mode).
add_theme_support( 'amp', array( 'template_dir' => './' ) );
$this->assertStringEndsWith( '&amp', amp_get_permalink( $drafted_post ) );
$this->assertStringEndsWith( '?amp', amp_get_permalink( $published_post ) );
$this->assertStringEndsWith( '?amp', amp_get_permalink( $published_page ) );
add_filter( 'amp_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$this->assertNotContains( 'current_filter=amp_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply.
add_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$this->assertNotContains( 'current_filter=amp_pre_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply.

// Make sure that if permalink has anchor that it is persists.
add_filter( 'post_link', $add_anchor_fragment );
$this->assertStringEndsWith( '/?amp#anchor', amp_get_permalink( $published_post ) );
}

/**
Expand Down Expand Up @@ -164,6 +197,33 @@ public function test_amp_remove_endpoint() {
$this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/?blaz' ) );
}

/**
* Test is_amp_endpoint() function.
*
* @covers \is_amp_endpoint()
*/
public function test_is_amp_endpoint() {
$this->assertFalse( is_amp_endpoint() );

// Legacy query var.
set_query_var( amp_get_slug(), '' );
$this->assertTrue( is_amp_endpoint() );
unset( $GLOBALS['wp_query']->query_vars[ amp_get_slug() ] );
$this->assertFalse( is_amp_endpoint() );

// Paired theme support.
add_theme_support( 'amp', array( 'template_dir' => './' ) );
$_GET['amp'] = '';
$this->assertTrue( is_amp_endpoint() );
unset( $_GET['amp'] );
$this->assertFalse( is_amp_endpoint() );
remove_theme_support( 'amp' );

// Native theme support.
add_theme_support( 'amp' );
$this->assertTrue( is_amp_endpoint() );
}

/**
* Filter calls.
*
Expand Down

0 comments on commit c8d56ed

Please sign in to comment.