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.

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

Set AMP_QUERY_VAR fallback when AMP is inactive #986

Merged
merged 2 commits into from
Mar 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function amp_deactivate() {
// We need to manually remove the amp endpoint
global $wp_rewrite;
foreach ( $wp_rewrite->endpoints as $index => $endpoint ) {
if ( AMP_QUERY_VAR === $endpoint[1] ) {
if ( amp_get_slug() === $endpoint[1] ) {
unset( $wp_rewrite->endpoints[ $index ] );
break;
}
Expand All @@ -73,20 +73,12 @@ function amp_deactivate() {
* @since 0.6
*/
function amp_after_setup_theme() {
amp_get_slug(); // Ensure AMP_QUERY_VAR is set.

if ( false === apply_filters( 'amp_is_enabled', true ) ) {
return;
}

if ( ! defined( 'AMP_QUERY_VAR' ) ) {
/**
* Filter the AMP query variable.
*
* @since 0.3.2
* @param string $query_var The AMP query variable.
*/
define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) );
}

add_action( 'init', 'amp_init', 0 ); // Must be 0 because widgets_init happens at init priority 1.
}
add_action( 'after_setup_theme', 'amp_after_setup_theme', 5 );
Expand All @@ -107,7 +99,7 @@ function amp_init() {

load_plugin_textdomain( 'amp', false, plugin_basename( AMP__DIR__ ) . '/languages' );

add_rewrite_endpoint( AMP_QUERY_VAR, EP_PERMALINK );
add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );

AMP_Validation_Utils::init();
AMP_Theme_Support::init();
Expand All @@ -132,8 +124,8 @@ function amp_init() {
// Make sure the `amp` query var has an explicit value.
// Avoids issues when filtering the deprecated `query_string` hook.
function amp_force_query_var_value( $query_vars ) {
if ( isset( $query_vars[ AMP_QUERY_VAR ] ) && '' === $query_vars[ AMP_QUERY_VAR ] ) {
$query_vars[ AMP_QUERY_VAR ] = 1;
if ( isset( $query_vars[ amp_get_slug() ] ) && '' === $query_vars[ amp_get_slug() ] ) {
$query_vars[ amp_get_slug() ] = 1;
}
return $query_vars;
}
Expand Down Expand Up @@ -202,7 +194,7 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
$query->is_home()
&&
// Is AMP endpoint.
false !== $query->get( AMP_QUERY_VAR, false )
false !== $query->get( amp_get_slug(), false )
&&
// Is query not yet fixed uo up to be front page.
! $query->is_front_page()
Expand All @@ -214,7 +206,7 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
get_option( 'page_on_front' )
&&
// See line in WP_Query::parse_query() at <https://github.com/WordPress/wordpress-develop/blob/0baa8ae/src/wp-includes/class-wp-query.php#L961>.
0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), array( AMP_QUERY_VAR, 'preview', 'page', 'paged', 'cpage' ) ) )
0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), array( amp_get_slug(), 'preview', 'page', 'paged', 'cpage' ) ) )
);
if ( $is_front_page_query ) {
$query->is_home = false;
Expand Down Expand Up @@ -304,9 +296,9 @@ function amp_render_post( $post ) {
* which is not ideal for any code that expects to run in an AMP context.
* Let's force the value to be true while we render AMP.
*/
$was_set = isset( $wp_query->query_vars[ AMP_QUERY_VAR ] );
$was_set = isset( $wp_query->query_vars[ amp_get_slug() ] );
if ( ! $was_set ) {
$wp_query->query_vars[ AMP_QUERY_VAR ] = true;
$wp_query->query_vars[ amp_get_slug() ] = true;
}

// Prevent New Relic from causing invalid AMP responses due the NREUM script it injects after the meta charset.
Expand All @@ -328,7 +320,7 @@ function amp_render_post( $post ) {
$template->load();

if ( ! $was_set ) {
unset( $wp_query->query_vars[ AMP_QUERY_VAR ] );
unset( $wp_query->query_vars[ amp_get_slug() ] );
}
}

Expand Down Expand Up @@ -359,7 +351,7 @@ function _amp_bootstrap_customizer() {
function amp_redirect_old_slug_to_new_url( $link ) {

if ( is_amp_endpoint() ) {
$link = trailingslashit( trailingslashit( $link ) . AMP_QUERY_VAR );
$link = trailingslashit( trailingslashit( $link ) . amp_get_slug() );
}

return $link;
Expand Down
2 changes: 1 addition & 1 deletion includes/admin/class-amp-customizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public function add_customizer_scripts() {

wp_add_inline_script( 'amp-customize-controls', sprintf( 'ampCustomizeControls.boot( %s );',
wp_json_encode( array(
'queryVar' => AMP_QUERY_VAR,
'queryVar' => amp_get_slug(),
'panelId' => self::PANEL_ID,
'ampUrl' => amp_admin_get_preview_permalink(),
'l10n' => array(
Expand Down
4 changes: 2 additions & 2 deletions includes/admin/class-amp-post-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public function enqueue_admin_assets() {
);
wp_add_inline_script( self::ASSETS_HANDLE, sprintf( 'ampPostMetaBox.boot( %s );',
wp_json_encode( array(
'previewLink' => esc_url_raw( add_query_arg( AMP_QUERY_VAR, '', get_preview_post_link( $post ) ) ),
'previewLink' => esc_url_raw( add_query_arg( amp_get_slug(), '', get_preview_post_link( $post ) ) ),
'canonical' => amp_is_canonical(),
'enabled' => post_supports_amp( $post ),
'canSupport' => count( AMP_Post_Type_Support::get_support_errors( $post ) ) === 0,
Expand Down Expand Up @@ -237,7 +237,7 @@ public function preview_post_link( $link ) {
);

if ( $is_amp ) {
$link = add_query_arg( AMP_QUERY_VAR, true, $link );
$link = add_query_arg( amp_get_slug(), true, $link );
}

return $link;
Expand Down
2 changes: 1 addition & 1 deletion includes/admin/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function amp_admin_get_preview_permalink() {
*/
$post_type = (string) apply_filters( 'amp_customizer_post_type', 'post' );

if ( ! post_type_supports( $post_type, AMP_QUERY_VAR ) ) {
if ( ! post_type_supports( $post_type, amp_get_slug() ) ) {
return null;
}

Expand Down
40 changes: 35 additions & 5 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,36 @@
* @package AMP
*/

/**
* 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
* 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
* @return string Slug used for query var, endpoint, and post type support.
*/
function amp_get_slug() {
if ( defined( 'AMP_QUERY_VAR' ) ) {
return AMP_QUERY_VAR;
}

/**
* Filter the AMP query variable.
*
* Warning: This filter may become deprecated.
*
* @since 0.3.2
* @param string $query_var The AMP query variable.
*/
$query_var = apply_filters( 'amp_query_var', 'amp' );

define( 'AMP_QUERY_VAR', $query_var );

return $query_var;
}

/**
* Retrieves the full AMP-specific permalink for the given post ID.
*
Expand Down Expand Up @@ -38,9 +68,9 @@ function amp_get_permalink( $post_id ) {
$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_QUERY_VAR, '', get_permalink( $post_id ) );
$amp_url = add_query_arg( amp_get_slug(), '', get_permalink( $post_id ) );
} else {
$amp_url = trailingslashit( get_permalink( $post_id ) ) . user_trailingslashit( AMP_QUERY_VAR, 'single_amp' );
$amp_url = trailingslashit( get_permalink( $post_id ) ) . user_trailingslashit( amp_get_slug(), 'single_amp' );
}
}

Expand All @@ -66,10 +96,10 @@ function amp_get_permalink( $post_id ) {
function amp_remove_endpoint( $url ) {

// Strip endpoint.
$url = preg_replace( ':/' . preg_quote( AMP_QUERY_VAR, ':' ) . '(?=/?(\?|#|$)):', '', $url );
$url = preg_replace( ':/' . preg_quote( amp_get_slug(), ':' ) . '(?=/?(\?|#|$)):', '', $url );

// Strip query var.
$url = remove_query_arg( AMP_QUERY_VAR, $url );
$url = remove_query_arg( amp_get_slug(), $url );

return $url;
}
Expand Down Expand Up @@ -149,7 +179,7 @@ function is_amp_endpoint() {
_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' );
}

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

/**
Expand Down
4 changes: 2 additions & 2 deletions includes/class-amp-post-type-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static function add_post_type_support() {
AMP_Options_Manager::get_option( 'supported_post_types', array() )
);
foreach ( $post_types as $post_type ) {
add_post_type_support( $post_type, AMP_QUERY_VAR );
add_post_type_support( $post_type, amp_get_slug() );
}
}

Expand All @@ -73,7 +73,7 @@ public static function get_support_errors( $post ) {
$errors = array();

// Because `add_rewrite_endpoint` doesn't let us target specific post_types.
if ( isset( $post->post_type ) && ! post_type_supports( $post->post_type, AMP_QUERY_VAR ) ) {
if ( isset( $post->post_type ) && ! post_type_supports( $post->post_type, amp_get_slug() ) ) {
$errors[] = 'post-type-support';
}

Expand Down
2 changes: 1 addition & 1 deletion includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static function finish_init() {
* @since 0.7
*/
public static function redirect_canonical_amp() {
if ( false !== get_query_var( AMP_QUERY_VAR, false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical().
if ( false !== get_query_var( amp_get_slug(), false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical().
$url = preg_replace( '#^(https?://.+?)(/.*)$#', '$1', home_url( '/' ) );
if ( isset( $_SERVER['REQUEST_URI'] ) ) {
$url .= wp_unslash( $_SERVER['REQUEST_URI'] );
Expand Down
2 changes: 1 addition & 1 deletion includes/options/class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public static function check_supported_post_type_update_errors() {
continue;
}

$post_type_supported = post_type_supports( $post_type->name, AMP_QUERY_VAR );
$post_type_supported = post_type_supports( $post_type->name, amp_get_slug() );
$is_support_elected = in_array( $post_type->name, $supported_types, true );

$error = null;
Expand Down
2 changes: 1 addition & 1 deletion includes/options/class-amp-options-menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function render_post_types_support() {
id="<?php echo esc_attr( $element_id ); ?>"
name="<?php echo esc_attr( $element_name ); ?>"
value="<?php echo esc_attr( $post_type->name ); ?>"
<?php checked( true, amp_is_canonical() || post_type_supports( $post_type->name, AMP_QUERY_VAR ) ); ?>
<?php checked( true, amp_is_canonical() || post_type_supports( $post_type->name, amp_get_slug() ) ); ?>
<?php disabled( $is_builtin ); ?>
>
<label for="<?php echo esc_attr( $element_id ); ?>">
Expand Down
13 changes: 11 additions & 2 deletions tests/test-amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ public function return_example_url( $url, $post_id ) {
return 'http://overridden.example.com/?' . build_query( compact( 'url', 'post_id', 'current_filter' ) );
}

/**
* Test amp_get_slug().
*
* @covers amp_get_slug()
*/
public function test_amp_get_slug() {
$this->assertSame( 'amp', amp_get_slug() );
}

/**
* Test amp_get_permalink() without pretty permalinks.
*
Expand Down Expand Up @@ -261,7 +270,7 @@ public function test_amp_get_content_sanitizers_deprecated_param() {
* @covers \post_supports_amp()
*/
public function test_post_supports_amp() {
add_post_type_support( 'page', AMP_QUERY_VAR );
add_post_type_support( 'page', amp_get_slug() );

// Test disabled by default for page for posts and show on front.
update_option( 'show_on_front', 'page' );
Expand All @@ -282,7 +291,7 @@ public function test_post_supports_amp() {
$this->assertFalse( post_supports_amp( $post ) );

// Reset.
remove_post_type_support( 'page', AMP_QUERY_VAR );
remove_post_type_support( 'page', amp_get_slug() );
}

/**
Expand Down
6 changes: 3 additions & 3 deletions tests/test-class-amp-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ public function test_render_status() {
$this->instance->render_status( $post );
$this->assertContains( $amp_status_markup, ob_get_clean() );

remove_post_type_support( 'post', AMP_QUERY_VAR );
remove_post_type_support( 'post', amp_get_slug() );

ob_start();
$this->instance->render_status( $post );
$this->assertEmpty( ob_get_clean() );

add_post_type_support( 'post', AMP_QUERY_VAR );
add_post_type_support( 'post', amp_get_slug() );
wp_set_current_user( $this->factory->user->create( array(
'role' => 'subscriber',
) ) );
Expand Down Expand Up @@ -160,7 +160,7 @@ public function test_preview_post_link() {
$link = 'https://foo.bar';
$this->assertEquals( 'https://foo.bar', $this->instance->preview_post_link( $link ) );
$_POST['amp-preview'] = 'do-preview';
$this->assertEquals( 'https://foo.bar?' . AMP_QUERY_VAR . '=1', $this->instance->preview_post_link( $link ) );
$this->assertEquals( 'https://foo.bar?' . amp_get_slug() . '=1', $this->instance->preview_post_link( $link ) );
}

}
4 changes: 2 additions & 2 deletions tests/test-class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public function test_check_supported_post_type_update_errors() {
$this->assertEmpty( get_settings_errors() );

// Activation error.
remove_post_type_support( 'foo', AMP_QUERY_VAR );
remove_post_type_support( 'foo', amp_get_slug() );
AMP_Options_Manager::check_supported_post_type_update_errors();
$errors = get_settings_errors();
$this->assertCount( 1, $errors );
Expand All @@ -181,7 +181,7 @@ public function test_check_supported_post_type_update_errors() {

// Deactivation error.
AMP_Options_Manager::update_option( 'supported_post_types', array() );
add_post_type_support( 'foo', AMP_QUERY_VAR );
add_post_type_support( 'foo', amp_get_slug() );
AMP_Options_Manager::check_supported_post_type_update_errors();
$errors = get_settings_errors();
$this->assertCount( 1, $errors );
Expand Down
8 changes: 4 additions & 4 deletions tests/test-class-amp-post-type-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ public function test_add_post_type_support() {
AMP_Options_Manager::update_option( 'supported_post_types', array( 'poem' ) );

AMP_Post_Type_Support::add_post_type_support();
$this->assertTrue( post_type_supports( 'post', AMP_QUERY_VAR ) );
$this->assertTrue( post_type_supports( 'poem', AMP_QUERY_VAR ) );
$this->assertFalse( post_type_supports( 'book', AMP_QUERY_VAR ) );
$this->assertTrue( post_type_supports( 'post', amp_get_slug() ) );
$this->assertTrue( post_type_supports( 'poem', amp_get_slug() ) );
$this->assertFalse( post_type_supports( 'book', amp_get_slug() ) );
}

/**
Expand All @@ -92,7 +92,7 @@ public function test_get_support_error() {
// Post type support.
$book_id = $this->factory()->post->create( array( 'post_type' => 'book' ) );
$this->assertEquals( array( 'post-type-support' ), AMP_Post_Type_Support::get_support_errors( $book_id ) );
add_post_type_support( 'book', AMP_QUERY_VAR );
add_post_type_support( 'book', amp_get_slug() );
$this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $book_id ) );

// Password-protected.
Expand Down