From 4b770daf0cb2764fe9db4915e08f31cb19076fe0 Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Sat, 29 Aug 2020 18:38:41 -0400 Subject: [PATCH 01/16] Remove unused parameter in call to AMP_Theme_Support::ensure_proper_amp_location --- tests/php/test-class-amp-theme-support.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 29e8390f3af..f8edfe4bbec 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -339,13 +339,13 @@ public function test_ensure_proper_amp_location_canonical() { // Already canonical. $_SERVER['REQUEST_URI'] = '/foo/bar/'; - $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location( false ) ); + $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location(false ) ); // URL query param. $_GET[ amp_get_slug() ] = ''; $_SERVER['REQUEST_URI'] = add_query_arg( amp_get_slug(), '', '/foo/bar' ); try { - $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location( false ) ); + $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { $e = $exception; } @@ -358,7 +358,7 @@ public function test_ensure_proper_amp_location_canonical() { set_query_var( amp_get_slug(), '' ); $_SERVER['REQUEST_URI'] = '/2016/01/24/foo/amp/'; try { - $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location( false ) ); + $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { $e = $exception; } @@ -382,15 +382,15 @@ public function test_ensure_proper_amp_location_transitional() { // URL query param, no redirection. $_GET[ amp_get_slug() ] = ''; - $_SERVER['REQUEST_URI'] = add_query_arg( amp_get_slug(), '', '/foo/bar' ); - $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location( false ) ); + $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); + $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location() ); // Endpoint, redirect. unset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended set_query_var( amp_get_slug(), '' ); $_SERVER['REQUEST_URI'] = '/2016/01/24/foo/amp/'; try { - $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location( false ) ); + $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { $e = $exception; } From d0ea3732a27d711707d369ac79a2645ee181abbf Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Sat, 29 Aug 2020 20:09:44 -0400 Subject: [PATCH 02/16] Default to ?amp=1 query parameter --- includes/admin/class-amp-post-meta-box.php | 4 +- includes/amp-helper-functions.php | 78 +++++++++---------- includes/class-amp-theme-support.php | 14 ++-- .../embeds/class-amp-core-block-handler.php | 2 +- .../options/class-amp-options-manager.php | 2 +- .../sanitizers/class-amp-form-sanitizer.php | 2 +- includes/templates/amp-paired-browsing.php | 2 +- .../class-amp-validated-url-post-type.php | 2 +- .../class-amp-validation-manager.php | 2 +- src/MobileRedirection.php | 2 +- src/PluginSuppression.php | 2 +- src/ReaderThemeLoader.php | 11 +-- tests/php/src/MobileRedirectionTest.php | 8 +- tests/php/src/ReaderThemeLoaderTest.php | 9 --- tests/php/test-amp-helper-functions.php | 58 +++++++------- tests/php/test-class-amp-link-sanitizer.php | 4 +- tests/php/test-class-amp-theme-support.php | 16 ++-- ...test-class-amp-validated-url-post-type.php | 2 +- 18 files changed, 100 insertions(+), 120 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 5da80bb4797..fc302c49658 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -188,7 +188,7 @@ public function enqueue_admin_assets() { 'ampPostMetaBox.boot( %s );', wp_json_encode( [ - 'previewLink' => esc_url_raw( add_query_arg( amp_get_slug(), '', get_preview_post_link( $post ) ) ), + 'previewLink' => esc_url_raw( amp_get_url( get_preview_post_link( $post ) ) ), 'canonical' => amp_is_canonical(), 'enabled' => empty( $support_errors ), 'canSupport' => 0 === count( array_diff( $support_errors, [ 'post-status-disabled' ] ) ), @@ -424,7 +424,7 @@ public function preview_post_link( $link ) { ); if ( $is_amp ) { - $link = add_query_arg( amp_get_slug(), true, $link ); + $link = amp_get_url( $link ); } return $link; diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 47b1ea0fe3f..4ef9076d4f7 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -573,7 +573,7 @@ function amp_redirect_old_slug_to_new_url( $link ) { if ( amp_is_request() && ! amp_is_canonical() ) { if ( ! amp_is_legacy() ) { - $link = add_query_arg( amp_get_slug(), '', $link ); + $link = amp_get_url( $link ); } else { $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); } @@ -660,16 +660,6 @@ function amp_get_current_url() { * @return string AMP permalink. */ function amp_get_permalink( $post_id ) { - - // When theme support is present (i.e. not using legacy Reader post templates), the plain query var should always be used. - if ( ! amp_is_legacy() ) { - $permalink = get_permalink( $post_id ); - if ( ! amp_is_canonical() ) { - $permalink = add_query_arg( amp_get_slug(), '', $permalink ); - } - return $permalink; - } - /** * Filters the AMP permalink to short-circuit normal generation. * @@ -688,35 +678,7 @@ function amp_get_permalink( $post_id ) { } $permalink = get_permalink( $post_id ); - - if ( amp_is_canonical() ) { - $amp_url = $permalink; - } else { - $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 ) ) - || - // Attachment pages don't accept the /amp/ endpoint. - 'attachment' === get_post_type( $post_id ) - ); - if ( $use_query_var ) { - $amp_url = add_query_arg( amp_get_slug(), '', $permalink ); - } else { - $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']; - } - } - } + $amp_url = amp_is_canonical() ? $permalink : amp_get_url( $permalink ); /** * Filters AMP permalink. @@ -797,7 +759,7 @@ function amp_add_amphtml_link() { } if ( AMP_Theme_Support::is_paired_available() ) { - $amp_url = add_query_arg( amp_get_slug(), '', amp_get_current_url() ); + $amp_url = amp_get_url( amp_get_current_url() ); } else { $amp_url = amp_get_permalink( get_queried_object_id() ); } @@ -1874,7 +1836,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 = add_query_arg( amp_get_slug(), '', amp_get_current_url() ); + $href = amp_get_url( amp_get_current_url() ); } $href = remove_query_arg( QueryVar::NOAMP, $href ); @@ -1909,7 +1871,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'] = add_query_arg( amp_get_slug(), '1', $args['href'] ); + $args['href'] = amp_get_url( $args['href'] ); } $wp_admin_bar->add_node( $args ); } @@ -1943,3 +1905,33 @@ function amp_generate_script_hash( $script ) { ); return 'sha384-' . $hash; } + +/** + * Get the AMP version of a URL. + * + * @since 2.1 + * + * @param string $url URL. + * @return string AMP URL. + */ +function amp_get_url( $url ) { + /** + * Filters the AMP version of the given URL. + * + * @since 2.1 + * + * @param string URL with AMP query parameter added. + */ + return apply_filters( 'amp_url', add_query_arg( amp_get_slug(), 1, $url ) ); +} + +/** + * Determine whether the current request has the AMP query parameter set with the required value '1'. + * + * @since 2.1 + * + * @return bool True if the AMP query parameter is set with the required value, false if not. + */ +function amp_has_query_var() { + return isset( $_GET[ amp_get_slug() ] ) && 1 === filter_var( $_GET[ amp_get_slug() ], FILTER_VALIDATE_INT ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended +} diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 61e2d3ab92f..f237b53750f 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -326,9 +326,9 @@ public static function finish_init() { } $has_query_var = ( - isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended - || - false !== get_query_var( amp_get_slug(), false ) + isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + || + false !== get_query_var( amp_get_slug(), false ) ); if ( ! amp_is_request() ) { @@ -411,7 +411,7 @@ public static function ensure_proper_amp_location() { global $wp; $path_args = []; wp_parse_str( $wp->matched_query, $path_args ); - if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { + if ( isset( $path_args[ amp_get_slug() ] ) && '1' !== $path_args[ amp_get_slug() ] ) { if ( wp_safe_redirect( amp_get_permalink( get_queried_object_id() ), 301 ) ) { // @codeCoverageIgnoreStart exit; @@ -419,7 +419,7 @@ public static function ensure_proper_amp_location() { } return true; } - } elseif ( $has_query_var && ! $has_url_param ) { + } elseif ( $has_query_var && ! amp_has_query_var() ) { /* * When in AMP transitional mode *with* theme support, then the proper AMP URL has the 'amp' URL param * and not the /amp/ endpoint. The URL param is now the exclusive way to mark AMP in transitional mode @@ -427,7 +427,7 @@ public static function ensure_proper_amp_location() { * amp_is_request() before the parse_query action. */ $old_url = amp_get_current_url(); - $new_url = add_query_arg( amp_get_slug(), '', amp_remove_endpoint( $old_url ) ); + $new_url = amp_get_url( amp_remove_endpoint( $old_url ) ); if ( $old_url !== $new_url ) { // A temporary redirect is used for admin users to allow them to see changes between reader mode and transitional modes. if ( wp_safe_redirect( $new_url, current_user_can( 'manage_options' ) ? 302 : 301 ) ) { @@ -1130,7 +1130,7 @@ public static function amend_comment_form() { } /** - * Amend the comments/redpond links to go to non-AMP page when in legacy Reader mode. + * Amend the comments/respond links to go to non-AMP page when in legacy Reader mode. * * @see get_comments_link() * @see comments_popup_link() diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php index dac094ff38e..56cbf6829aa 100644 --- a/includes/embeds/class-amp-core-block-handler.php +++ b/includes/embeds/class-amp-core-block-handler.php @@ -313,7 +313,7 @@ private function process_archives_widgets( Document $dom, $args = [] ) { * * @var DOMElement $option */ - $option->setAttribute( 'value', add_query_arg( amp_get_slug(), '', $option->getAttribute( 'value' ) ) ); + $option->setAttribute( 'value', amp_get_url( $option->getAttribute( 'value' ) ) ); } } } diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php index 3883d58cfa0..b0fb6be56fc 100644 --- a/includes/options/class-amp-options-manager.php +++ b/includes/options/class-amp-options-manager.php @@ -171,7 +171,7 @@ static function ( $supported ) { && get_stylesheet() === $options[ Option::READER_THEME ] && - ! isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + ! amp_has_query_var() ) { /* * When Reader mode is selected and a Reader theme has been chosen, if the active theme switches to be the diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index 02d3eaa7521..78317623fe6 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -87,7 +87,7 @@ public function sanitize() { // Record that action was converted to action-xhr. $action_url = add_query_arg( AMP_HTTP::ACTION_XHR_CONVERTED_QUERY_VAR, 1, $action_url ); if ( ! amp_is_canonical() ) { - $action_url = add_query_arg( amp_get_slug(), '', $action_url ); + $action_url = amp_get_url( $action_url ); } $node->setAttribute( 'action-xhr', $action_url ); // Append success/error handlers if not found. diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index a9daee648cb..17c35049f29 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -9,7 +9,7 @@ $url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); -$amp_url = add_query_arg( amp_get_slug(), '1', $url ); +$amp_url = amp_get_url( $url ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 746c3063294..9bdf2b149d4 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -718,7 +718,7 @@ public static function get_url_from_post( $post ) { // Add AMP query var if in transitional mode. if ( ! amp_is_canonical() ) { - $url = add_query_arg( amp_get_slug(), '', $url ); + $url = amp_get_url( $url ); } // Set URL scheme based on whether HTTPS is current. diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 5ac8a42c26a..949a8beaf97 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -383,7 +383,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $current_url ); if ( ! amp_is_canonical() ) { - $amp_url = add_query_arg( amp_get_slug(), '', $amp_url ); + $amp_url = amp_get_url( $amp_url ); } $validate_url = AMP_Validated_URL_Post_Type::get_recheck_url( AMP_Validated_URL_Post_Type::get_invalid_url_post( $amp_url ) ?: $amp_url ); diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index 937719da2d4..6a6a3a60bbd 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -84,7 +84,7 @@ public function sanitize_options( $options, $new_options ) { * @return string AMP URL. */ public function get_current_amp_url() { - $url = add_query_arg( amp_get_slug(), '1', amp_get_current_url() ); + $url = amp_get_url( amp_get_current_url() ); $url = remove_query_arg( QueryVar::NOAMP, $url ); return $url; } diff --git a/src/PluginSuppression.php b/src/PluginSuppression.php index 7fd7f7f0e50..e9e2146de89 100644 --- a/src/PluginSuppression.php +++ b/src/PluginSuppression.php @@ -82,7 +82,7 @@ public function is_reader_theme_request() { && ReaderThemes::DEFAULT_READER_THEME !== AMP_Options_Manager::get_option( Option::READER_THEME ) && - isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + amp_has_query_var() ); } diff --git a/src/ReaderThemeLoader.php b/src/ReaderThemeLoader.php index 31a2b4dfbd0..ccdab0583df 100644 --- a/src/ReaderThemeLoader.php +++ b/src/ReaderThemeLoader.php @@ -94,15 +94,6 @@ public function is_theme_overridden() { return $this->theme_overridden; } - /** - * Is an AMP request. - * - * @return bool Whether AMP request. - */ - public function is_amp_request() { - return isset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - } - /** * Register the service with the system. * @@ -298,7 +289,7 @@ public function get_active_theme() { */ public function override_theme() { $this->theme_overridden = false; - if ( ! $this->is_enabled() || ! $this->is_amp_request() ) { + if ( ! $this->is_enabled() || ! amp_has_query_var() ) { return; } diff --git a/tests/php/src/MobileRedirectionTest.php b/tests/php/src/MobileRedirectionTest.php index 275ead1b71a..798d0b9c026 100644 --- a/tests/php/src/MobileRedirectionTest.php +++ b/tests/php/src/MobileRedirectionTest.php @@ -108,7 +108,7 @@ public function test_sanitize_options() { public function test_get_current_amp_url() { $this->go_to( add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, '/foo/' ) ); $this->assertEquals( - add_query_arg( QueryVar::AMP, '1', home_url( '/foo/' ) ), + amp_get_url( home_url( '/foo/' ) ), $this->instance->get_current_amp_url() ); } @@ -140,7 +140,7 @@ public function test_redirect_on_transitional_and_not_available() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); AMP_Options_Manager::update_option( Option::ALL_TEMPLATES_SUPPORTED, false ); AMP_Options_Manager::update_option( Option::SUPPORTED_TEMPLATES, [ 'is_author' ] ); - $this->go_to( add_query_arg( QueryVar::AMP, '1', '/' ) ); + $this->go_to( amp_get_url( '/' ) ); $this->assertFalse( amp_is_canonical() ); $this->assertFalse( amp_is_available() ); $this->instance->redirect(); @@ -174,7 +174,7 @@ public function test_redirect_when_server_side_and_not_applicable() { add_filter( 'amp_mobile_client_side_redirection', '__return_false' ); add_filter( 'amp_pre_is_mobile', '__return_false' ); - $this->go_to( add_query_arg( QueryVar::AMP, '1', '/' ) ); + $this->go_to( amp_get_url( '/' ) ); $this->assertFalse( amp_is_request() ); $this->assertFalse( $this->instance->is_mobile_request() ); @@ -215,7 +215,7 @@ static function ( $redirect_url ) use ( &$redirected_url ) { $this->instance->redirect(); $this->assertNotNull( $redirected_url ); $this->assertEquals( - add_query_arg( QueryVar::AMP, '1', home_url( '/' ) ), + amp_get_url( home_url( '/' ) ), $redirected_url ); } diff --git a/tests/php/src/ReaderThemeLoaderTest.php b/tests/php/src/ReaderThemeLoaderTest.php index b36ad410ef2..38e2662a110 100644 --- a/tests/php/src/ReaderThemeLoaderTest.php +++ b/tests/php/src/ReaderThemeLoaderTest.php @@ -65,15 +65,6 @@ public function test_is_enabled() { $this->assertTrue( $this->instance->is_enabled() ); } - /** @covers ::is_amp_request() */ - public function test_is_amp_request() { - $_GET[ amp_get_slug() ] = '1'; - $this->assertTrue( $this->instance->is_amp_request() ); - - unset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - $this->assertFalse( $this->instance->is_amp_request() ); - } - public function test__construct() { $this->assertInstanceOf( ReaderThemeLoader::class, $this->instance ); $this->assertInstanceOf( Service::class, $this->instance ); diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 5a9c6a70b54..c48871d738f 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -464,9 +464,9 @@ public function test_amp_get_permalink_without_pretty_permalinks() { ] ); - $this->assertStringEndsWith( '&', amp_get_permalink( $published_post ) ); - $this->assertStringEndsWith( '&', amp_get_permalink( $drafted_post ) ); - $this->assertStringEndsWith( '&', amp_get_permalink( $published_page ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $published_post ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $drafted_post ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $published_page ) ); add_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); add_filter( 'amp_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); @@ -478,6 +478,7 @@ public function test_amp_get_permalink_without_pretty_permalinks() { $url = amp_get_permalink( $published_post ); $this->assertStringContains( 'current_filter=amp_get_permalink', $url ); remove_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ] ); + remove_filter( 'amp_get_permalink', [ $this, 'return_example_url' ] ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG ); $this->assertEquals( get_permalink( $published_post ), amp_get_permalink( $published_post ) ); @@ -494,13 +495,16 @@ public function test_amp_get_permalink_without_pretty_permalinks() { foreach ( $argses as $args ) { delete_option( AMP_Options_Manager::OPTION_NAME ); // To specify the defaults. add_theme_support( AMP_Theme_Support::SLUG, $args ); - $this->assertStringEndsWith( '&', amp_get_permalink( $published_post ) ); - $this->assertStringEndsWith( '&', amp_get_permalink( $drafted_post ) ); - $this->assertStringEndsWith( '&', amp_get_permalink( $published_page ) ); + + remove_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ] ); + remove_filter( 'amp_get_permalink', [ $this, 'return_example_url' ] ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $published_post ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $drafted_post ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $published_page ) ); add_filter( 'amp_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); - $this->assertStringNotContains( 'current_filter=amp_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply. + $this->assertStringEndsWith( 'current_filter=amp_get_permalink', amp_get_permalink( $published_post ) ); add_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); - $this->assertStringNotContains( 'current_filter=amp_pre_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply. + $this->assertStringEndsWith( 'current_filter=amp_pre_get_permalink', amp_get_permalink( $published_post ) ); } } @@ -539,12 +543,12 @@ public function test_amp_get_permalink_with_pretty_permalinks() { 'post_type' => 'page', ] ); - $this->assertStringEndsWith( '&', amp_get_permalink( $drafted_post ) ); - $this->assertStringEndsWith( '/amp/', amp_get_permalink( $published_post ) ); - $this->assertStringEndsWith( '?amp', amp_get_permalink( $published_page ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $drafted_post ) ); + $this->assertStringEndsWith( '?amp=1', amp_get_permalink( $published_post ) ); + $this->assertStringEndsWith( '?amp=1', amp_get_permalink( $published_page ) ); add_filter( 'post_link', $add_anchor_fragment ); - $this->assertStringEndsWith( '/amp/#anchor', amp_get_permalink( $published_post ) ); + $this->assertStringEndsWith( '?amp=1#anchor', amp_get_permalink( $published_post ) ); remove_filter( 'post_link', $add_anchor_fragment ); add_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); @@ -560,17 +564,19 @@ public function test_amp_get_permalink_with_pretty_permalinks() { // Now check with theme support added (in transitional mode). add_theme_support( AMP_Theme_Support::SLUG, [ 'template_dir' => './' ] ); - $this->assertStringEndsWith( '&', amp_get_permalink( $drafted_post ) ); - $this->assertStringEndsWith( '?amp', amp_get_permalink( $published_post ) ); - $this->assertStringEndsWith( '?amp', amp_get_permalink( $published_page ) ); + $this->assertStringEndsWith( '&=1', amp_get_permalink( $drafted_post ) ); + $this->assertStringEndsWith( '?amp=1', amp_get_permalink( $published_post ) ); + $this->assertStringEndsWith( '?amp=1', amp_get_permalink( $published_page ) ); add_filter( 'amp_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); - $this->assertStringNotContains( 'current_filter=amp_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply. + $this->assertStringEndsWith( 'current_filter=amp_get_permalink', amp_get_permalink( $published_post ) ); add_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ], 10, 2 ); - $this->assertStringNotContains( 'current_filter=amp_pre_get_permalink', amp_get_permalink( $published_post ) ); // Filter does not apply. + $this->assertStringEndsWith( 'current_filter=amp_pre_get_permalink', amp_get_permalink( $published_post ) ); // Make sure that if permalink has anchor that it is persists. + remove_filter( 'amp_pre_get_permalink', [ $this, 'return_example_url' ] ); + remove_filter( 'amp_get_permalink', [ $this, 'return_example_url' ] ); add_filter( 'post_link', $add_anchor_fragment ); - $this->assertStringEndsWith( '/?amp#anchor', amp_get_permalink( $published_post ) ); + $this->assertStringEndsWith( '/?amp=1#anchor', amp_get_permalink( $published_post ) ); } /** @@ -631,14 +637,14 @@ public function get_reader_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - add_query_arg( amp_get_slug(), '', home_url( '/' ) ), + amp_get_url( home_url( '/' ) ), false, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - add_query_arg( amp_get_slug(), '', home_url( '/no-existe/' ) ), + amp_get_url( home_url( '/no-existe/' ) ), false, ]; }, @@ -730,14 +736,14 @@ public function get_transitional_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - add_query_arg( amp_get_slug(), '', home_url( '/' ) ), + amp_get_url( home_url( '/' ) ), true, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - add_query_arg( amp_get_slug(), '', home_url( '/no-existe/' ) ), + amp_get_url( home_url( '/no-existe/' ) ), true, ]; }, @@ -853,7 +859,7 @@ public function test_amp_is_request() { $this->assertFalse( amp_is_request() ); // Legacy query var. - set_query_var( amp_get_slug(), '' ); + set_query_var( amp_get_slug(), 1 ); $this->assertTrue( amp_is_available() ); $this->assertTrue( amp_is_request() ); unset( $GLOBALS['wp_query']->query_vars[ amp_get_slug() ] ); @@ -862,7 +868,7 @@ public function test_amp_is_request() { // Transitional theme support. add_theme_support( AMP_Theme_Support::SLUG, [ 'template_dir' => './' ] ); - $_GET['amp'] = ''; + $_GET['amp'] = 1; $this->assertTrue( amp_is_available() ); $this->assertTrue( amp_is_request() ); unset( $_GET['amp'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended @@ -1044,7 +1050,7 @@ public function test_amp_is_request_before_wp_action_for_reader_mode() { */ public function test_amp_is_request_before_wp_action_for_transitional_mode_with_query_var() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( add_query_arg( amp_get_slug(), '1', home_url( '/' ) ) ); + $this->go_to( amp_get_url( home_url( '/' ) ) ); global $wp_actions; unset( $wp_actions['wp'] ); $this->assertTrue( AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ); @@ -1747,7 +1753,7 @@ public function test_amp_add_admin_bar_item() { $this->assertStringNotContains( 'autofocus', $item->href ); // Confirm that link is added to non-AMP version. - set_query_var( amp_get_slug(), '' ); + set_query_var( amp_get_slug(), 1 ); $this->assertTrue( amp_is_request() ); $admin_bar = new WP_Admin_Bar(); amp_add_admin_bar_view_link( $admin_bar ); diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index ada8d1e57af..0b7f00c0d75 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -253,7 +253,7 @@ public function test_disable_mobile_redirect_for_excluded_url() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( add_query_arg( amp_get_slug(), '', home_url( '/' ) ) ); + $this->go_to( amp_get_url( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); @@ -276,7 +276,7 @@ public function test_disable_mobile_redirect_for_url_with_noamphtml_rel() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( add_query_arg( amp_get_slug(), '', home_url( '/' ) ) ); + $this->go_to( amp_get_url( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index f8edfe4bbec..5c5454ccc62 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -324,7 +324,7 @@ function ( $url ) use ( $requested_url, &$redirected ) { return null; } ); - $this->go_to( add_query_arg( amp_get_slug(), '', $requested_url ) ); + $this->go_to( amp_get_url( $requested_url ) ); $this->assertTrue( $redirected ); } @@ -339,11 +339,11 @@ public function test_ensure_proper_amp_location_canonical() { // Already canonical. $_SERVER['REQUEST_URI'] = '/foo/bar/'; - $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location(false ) ); + $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location() ); // URL query param. $_GET[ amp_get_slug() ] = ''; - $_SERVER['REQUEST_URI'] = add_query_arg( amp_get_slug(), '', '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { @@ -355,7 +355,7 @@ public function test_ensure_proper_amp_location_canonical() { // Endpoint. unset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - set_query_var( amp_get_slug(), '' ); + set_query_var( amp_get_slug(), 1 ); $_SERVER['REQUEST_URI'] = '/2016/01/24/foo/amp/'; try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); @@ -381,13 +381,13 @@ public function test_ensure_proper_amp_location_transitional() { $e = null; // URL query param, no redirection. - $_GET[ amp_get_slug() ] = ''; + $_GET[ amp_get_slug() ] = '1'; $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location() ); // Endpoint, redirect. unset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - set_query_var( amp_get_slug(), '' ); + set_query_var( amp_get_slug(), 1 ); $_SERVER['REQUEST_URI'] = '/2016/01/24/foo/amp/'; try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); @@ -416,7 +416,7 @@ static function( $code ) use ( &$redirect_status_code ) { ); // Try AMP URL param. - $_SERVER['REQUEST_URI'] = add_query_arg( amp_get_slug(), '', '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); try { $redirect_status_code = null; $this->assertTrue( AMP_Theme_Support::redirect_non_amp_url( 302 ) ); @@ -2176,7 +2176,7 @@ public function test_prepare_response_redirect() { AMP_Theme_Support::PAIRED_FLAG => true, ] ); - $this->go_to( home_url( '/?amp' ) ); + $this->go_to( home_url( '/?amp=1' ) ); add_filter( 'amp_content_sanitizers', static function( $sanitizers ) { diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 9e4b5a00820..ff330225df9 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -326,7 +326,7 @@ public function test_get_url_from_post() { $this->assertNotInstanceOf( 'WP_Error', $invalid_post_id ); $this->assertEquals( - add_query_arg( amp_get_slug(), '', get_permalink( $post ) ), + amp_get_url( get_permalink( $post ) ), AMP_Validated_URL_Post_Type::get_url_from_post( $invalid_post_id ) ); From f80625401a71db0ba7e89b661dfc4872c8b9ce39 Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Sun, 30 Aug 2020 01:24:29 -0400 Subject: [PATCH 03/16] Remove amp_url filter --- includes/amp-helper-functions.php | 9 +-------- includes/class-amp-theme-support.php | 6 +++--- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 4ef9076d4f7..6293d3273a2 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1915,14 +1915,7 @@ function amp_generate_script_hash( $script ) { * @return string AMP URL. */ function amp_get_url( $url ) { - /** - * Filters the AMP version of the given URL. - * - * @since 2.1 - * - * @param string URL with AMP query parameter added. - */ - return apply_filters( 'amp_url', add_query_arg( amp_get_slug(), 1, $url ) ); + return add_query_arg( amp_get_slug(), 1, $url ); } /** diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index f237b53750f..fc9d50ce080 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -326,9 +326,9 @@ public static function finish_init() { } $has_query_var = ( - isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended - || - false !== get_query_var( amp_get_slug(), false ) + isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + || + false !== get_query_var( amp_get_slug(), false ) ); if ( ! amp_is_request() ) { From 83ec4a869442e89fda638f7bd408f6db7b3458c2 Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Mon, 21 Sep 2020 20:22:31 -0400 Subject: [PATCH 04/16] Revert making the value '1' required for the AMP query var Co-authored-by: Weston Ruter --- includes/amp-helper-functions.php | 6 +++--- includes/class-amp-theme-support.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 6293d3273a2..285cb085a57 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1915,16 +1915,16 @@ function amp_generate_script_hash( $script ) { * @return string AMP URL. */ function amp_get_url( $url ) { - return add_query_arg( amp_get_slug(), 1, $url ); + return add_query_arg( amp_get_slug(), '1', $url ); } /** - * Determine whether the current request has the AMP query parameter set with the required value '1'. + * Determine whether the current request has the AMP query parameter set. * * @since 2.1 * * @return bool True if the AMP query parameter is set with the required value, false if not. */ function amp_has_query_var() { - return isset( $_GET[ amp_get_slug() ] ) && 1 === filter_var( $_GET[ amp_get_slug() ], FILTER_VALIDATE_INT ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + return isset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index fc9d50ce080..cfd5ce78ee4 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -411,7 +411,7 @@ public static function ensure_proper_amp_location() { global $wp; $path_args = []; wp_parse_str( $wp->matched_query, $path_args ); - if ( isset( $path_args[ amp_get_slug() ] ) && '1' !== $path_args[ amp_get_slug() ] ) { + if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { if ( wp_safe_redirect( amp_get_permalink( get_queried_object_id() ), 301 ) ) { // @codeCoverageIgnoreStart exit; From aaf0e2ed114a60d456562f8e834a5683b0123c2f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 5 Oct 2020 15:41:55 -0700 Subject: [PATCH 05/16] Ensure amp_remove_endpoint removes repeated /amp/ slugs --- includes/amp-helper-functions.php | 18 +++++++++++++----- tests/php/test-amp-helper-functions.php | 4 ++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 1793459e297..7affa78a773 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -740,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 endpoint. - $url = preg_replace( ':/' . preg_quote( amp_get_slug(), ':' ) . '(?=/?(\?|#|$)):', '', $url ); - - // Strip query var. - $url = remove_query_arg( amp_get_slug(), $url ); + // Strip query var, including ?amp, ?amp=1, etc. + $url = remove_query_arg( $slug, $url ); return $url; } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 42890758969..69688a1ec9b 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -611,9 +611,13 @@ public function test_amp_get_permalink_with_theme_support() { */ public function test_amp_remove_endpoint() { $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/?amp' ) ); + $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/?amp=1' ) ); + $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/amp/?amp=1' ) ); $this->assertEquals( 'https://example.com/foo/?#bar', amp_remove_endpoint( 'https://example.com/foo/?amp#bar' ) ); $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/amp/' ) ); $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/?blaz' ) ); + $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/amp/?blaz' ) ); + $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/foo/amp/bar/?blaz' ) ); } /** From ecd9860a004e2f0d0660cbc437d54487c736242c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 5 Oct 2020 16:16:44 -0700 Subject: [PATCH 06/16] Extend amp_has_query_var() to look at WP_Query for query vars --- includes/amp-helper-functions.php | 23 ++++++---- tests/php/test-amp-helper-functions.php | 57 +++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 7affa78a773..226675c5422 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -869,13 +869,7 @@ function amp_is_request() { $is_amp_url = ( amp_is_canonical() || - isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended - || - ( - $wp_query instanceof WP_Query - && - false !== $wp_query->get( amp_get_slug(), false ) - ) + amp_has_query_var() ); // If AMP is not available, then it's definitely not an AMP endpoint. @@ -1958,6 +1952,7 @@ function amp_generate_script_hash( $script ) { * 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. @@ -1970,9 +1965,21 @@ function amp_get_url( $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() { - return isset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + 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 ) + ) + ); } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 69688a1ec9b..f007a597d2c 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -1818,6 +1818,63 @@ public function test_amp_generate_script_hash() { $this->assertSame( 'sha384-_MAJ0_NC2k8jrjehfi-5LdQasBICZXvp4gOwOx0D3mIStvDCGvZDzcTfXLgMrLL1', amp_generate_script_hash( 'document.body.textContent = \'\';' ) ); } + /** @return array */ + public function data_amp_has_query_var() { + return [ + 'nothing' => [ + '', + false, + ], + 'url_param_bare' => [ + '?amp', + true, + ], + 'url_param_value' => [ + '?amp=1', + true, + ], + 'endpoint_bare_slashed' => [ + 'amp/', + true, + ], + 'endpoint_bare_unslashed' => [ + 'amp', + true, + ], + 'endpoint_with_value' => [ + 'amp/x/', + true, + ], + 'endpoint_and_url_param' => [ + 'amp/?amp=1', + true, + ], + ]; + } + + /** + * @dataProvider data_amp_has_query_var + * @covers ::amp_has_query_var() + * @param string $suffix + * @param bool $is_amp + */ + public function test_amp_has_query_var( $suffix, $is_amp ) { + add_filter( 'wp_redirect', '__return_empty_string' ); // Prevent ensure_proper_amp_location() from redirecting. + global $wp_rewrite; + update_option( 'permalink_structure', '/%year%/%monthnum%/%day%/%postname%/' ); + $wp_rewrite->init(); + add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK ); + $wp_rewrite->flush_rules(); + + $permalink = get_permalink( self::factory()->post->create() ); + $this->assertNotContains( '?', $permalink ); + $url = $permalink . $suffix; + $this->go_to( $url ); + $this->assertTrue( is_singular(), 'Expected singular query.' ); + $this->assertTrue( amp_is_available(), 'Expected AMP to be available.' ); + $this->assertEquals( $is_amp, amp_has_query_var() ); + } + /** * Get a mock publisher logo URL, to test that the filter works as expected. * From 123a64faaf300e80b23429ffdd662fcc4ed5e10b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 5 Oct 2020 16:19:07 -0700 Subject: [PATCH 07/16] Add value of 1 to amp query var added in AMP-to-AMP linking --- includes/sanitizers/class-amp-link-sanitizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 9a770ac2547..7ea202dd2d5 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -194,7 +194,7 @@ private function process_element( DOMElement $element, $attribute_name ) { // Only add the AMP query var when requested (in Transitional or Reader mode). if ( ! empty( $this->args['paired'] ) ) { - $query_vars[ amp_get_slug() ] = ''; + $query_vars[ amp_get_slug() ] = '1'; // @todo Would be preferable to use amp_get_url() somehow here. } } From 52a6fbf781c958091e4b36b9b6dd69856bb5d9b8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 5 Oct 2020 16:21:36 -0700 Subject: [PATCH 08/16] Reuse amp_has_query_var() in AMP_Theme_Support::finish_init() --- includes/class-amp-theme-support.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 0bafe6a2335..2914fef0f20 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -327,12 +327,6 @@ public static function finish_init() { add_filter( 'template_include', [ __CLASS__, 'serve_paired_browsing_experience' ], PHP_INT_MAX ); } - $has_query_var = ( - isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended - || - false !== get_query_var( amp_get_slug(), false ) - ); - if ( ! amp_is_request() ) { /* * Redirect to AMP-less URL if AMP is not available for this URL and yet the query var is present. @@ -340,7 +334,7 @@ public static function finish_init() { * enabled by user ay any time, so they will be able to make AMP available for this URL and see the change * without wrestling with the redirect cache. */ - if ( $has_query_var ) { + if ( amp_has_query_var() ) { self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } From cf221bddea80adaef5354aa263bd2b56b7e9c7e4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 5 Oct 2020 16:35:26 -0700 Subject: [PATCH 09/16] Remove obsolete /amp/=>?amp redirection in ensure_proper_amp_location() This logic was originally added in https://github.com/ampproject/amp-wp/pull/1203 for https://github.com/ampproject/amp-wp/issues/1148, but that was ultimately reverted in https://github.com/ampproject/amp-wp/pull/1235. --- includes/class-amp-theme-support.php | 37 +++++--------- tests/php/test-class-amp-theme-support.php | 59 ++++++++++++++-------- 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 2914fef0f20..0579da59afa 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -384,13 +384,11 @@ static function() { * * @since 1.0 * @since 2.0 Removed $exit param. + * @since 2.1 Remove obsolete redirection from /amp/ to ?amp when on non-legacy Reader mode. * * @return bool Whether redirection should have been done. */ public static function ensure_proper_amp_location() { - $has_query_var = false !== get_query_var( amp_get_slug(), false ); // May come from URL param or endpoint slug. - $has_url_param = isset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - if ( amp_is_canonical() ) { /* * When AMP-first/canonical, then when there is an /amp/ endpoint or ?amp URL param, @@ -399,34 +397,23 @@ public static function ensure_proper_amp_location() { * should happen infrequently. For admin users, this is kept temporary to allow them * to not be hampered by browser remembering permanent redirects and preventing test. */ - if ( $has_query_var || $has_url_param ) { + if ( amp_has_query_var() ) { return self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } - } elseif ( amp_is_legacy() && is_singular() ) { - // Prevent infinite URL space under /amp/ endpoint. + } elseif ( amp_has_query_var() ) { + /* + * Prevent infinite URL space under /amp/ endpoint. Note that WordPress allows endpoints to have a value, + * such as the case of /feed/ where /feed/atom/ is the same as saying ?feed=atom. In this case, we need to + * check for /amp/x/ to protect against links like `AMP!`. + * See https://github.com/ampproject/amp-wp/pull/1846. + */ global $wp; $path_args = []; wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { - if ( wp_safe_redirect( amp_get_permalink( get_queried_object_id() ), 301 ) ) { - // @codeCoverageIgnoreStart - exit; - // @codeCoverageIgnoreEnd - } - return true; - } - } elseif ( $has_query_var && ! amp_has_query_var() ) { - /* - * When in AMP transitional mode *with* theme support, then the proper AMP URL has the 'amp' URL param - * and not the /amp/ endpoint. The URL param is now the exclusive way to mark AMP in transitional mode - * when amp theme support present. This is important for plugins to be able to reliably call - * amp_is_request() before the parse_query action. - */ - $old_url = amp_get_current_url(); - $new_url = amp_get_url( amp_remove_endpoint( $old_url ) ); - if ( $old_url !== $new_url ) { - // A temporary redirect is used for admin users to allow them to see changes between reader mode and transitional modes. - if ( wp_safe_redirect( $new_url, current_user_can( 'manage_options' ) ? 302 : 301 ) ) { + $current_url = amp_get_current_url(); + $redirect_url = amp_get_url( amp_remove_endpoint( $current_url ) ); + if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; // @codeCoverageIgnoreEnd diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 21a69d4e12c..f686d786185 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -367,34 +367,49 @@ public function test_ensure_proper_amp_location_canonical() { } /** - * Test ensure_proper_amp_location for transitional. + * Test ensure_proper_amp_location for infinite URL space. * + * @link https://github.com/ampproject/amp-wp/pull/1846 * @covers AMP_Theme_Support::ensure_proper_amp_location() */ - public function test_ensure_proper_amp_location_transitional() { - add_theme_support( - AMP_Theme_Support::SLUG, - [ - 'template_dir' => './', - ] + public function test_ensure_proper_amp_location_infinite_url_space() { + AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); + + global $wp_rewrite; + update_option( 'permalink_structure', '/%year%/%monthnum%/%day%/%postname%/' ); + $wp_rewrite->init(); + add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK ); + $wp_rewrite->flush_rules(); + + $redirections = []; + add_filter( + 'wp_redirect', + static function ( $url ) use ( &$redirections ) { + $redirections[] = $url; + return ''; + } ); - $e = null; + $permalink = get_permalink( self::factory()->post->create() ); - // URL query param, no redirection. - $_GET[ amp_get_slug() ] = '1'; - $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); - $this->assertFalse( AMP_Theme_Support::ensure_proper_amp_location() ); + $this->go_to( $permalink ); + $this->assertCount( 0, $redirections ); + $this->assertFalse( amp_is_request() ); - // Endpoint, redirect. - unset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - set_query_var( amp_get_slug(), 1 ); - $_SERVER['REQUEST_URI'] = '/2016/01/24/foo/amp/'; - try { - $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); - } catch ( Exception $exception ) { - $e = $exception; - } - $this->assertStringContains( 'headers already sent', $e->getMessage() ); + $this->go_to( add_query_arg( 'amp', '1', $permalink ) ); + $this->assertCount( 0, $redirections ); + $this->assertTrue( amp_is_request() ); + + $this->go_to( $permalink . 'amp/' ); + $this->assertCount( 0, $redirections ); + $this->assertTrue( amp_is_request() ); + + $this->go_to( $permalink . 'amp/amp/' ); + $this->assertCount( 1, $redirections ); + $this->assertEquals( amp_get_url( $permalink ), end( $redirections ) ); + + $this->go_to( $permalink . 'amp/foo/' ); + $this->assertCount( 2, $redirections ); + $this->assertEquals( amp_get_url( $permalink ), end( $redirections ) ); } /** From b66c233fa342ee922e04c89cae6887ec99c7b5a9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 6 Oct 2020 12:03:09 -0700 Subject: [PATCH 10/16] Rename amp_get_url() to amp_get_paired_url() --- includes/admin/class-amp-post-meta-box.php | 4 ++-- includes/amp-helper-functions.php | 13 ++++++------- includes/class-amp-theme-support.php | 2 +- .../embeds/class-amp-core-block-handler.php | 2 +- .../sanitizers/class-amp-form-sanitizer.php | 2 +- .../sanitizers/class-amp-link-sanitizer.php | 5 +++-- includes/templates/amp-paired-browsing.php | 2 +- .../class-amp-validated-url-post-type.php | 2 +- .../validation/class-amp-validation-manager.php | 2 +- src/MobileRedirection.php | 2 +- tests/php/src/MobileRedirectionTest.php | 8 ++++---- tests/php/test-amp-helper-functions.php | 17 ++++++++++++----- tests/php/test-class-amp-link-sanitizer.php | 4 ++-- tests/php/test-class-amp-theme-support.php | 10 +++++----- .../test-class-amp-validated-url-post-type.php | 2 +- 15 files changed, 42 insertions(+), 35 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index dbfd4bf5404..c9e7137b951 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -188,7 +188,7 @@ public function enqueue_admin_assets() { 'ampPostMetaBox.boot( %s );', wp_json_encode( [ - 'previewLink' => esc_url_raw( amp_get_url( get_preview_post_link( $post ) ) ), + 'previewLink' => esc_url_raw( amp_get_paired_url( get_preview_post_link( $post ) ) ), 'canonical' => amp_is_canonical(), 'enabled' => empty( $support_errors ), 'canSupport' => 0 === count( array_diff( $support_errors, [ 'post-status-disabled' ] ) ), @@ -447,7 +447,7 @@ public function preview_post_link( $link ) { ); if ( $is_amp ) { - $link = amp_get_url( $link ); + $link = amp_get_paired_url( $link ); } return $link; diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 226675c5422..2354482e698 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -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 ); + $link = amp_get_paired_url( $link ); } else { $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); } @@ -717,7 +717,7 @@ function amp_get_permalink( $post_id ) { } $permalink = get_permalink( $post_id ); - $amp_url = amp_is_canonical() ? $permalink : amp_get_url( $permalink ); + $amp_url = amp_is_canonical() ? $permalink : amp_get_paired_url( $permalink ); /** * Filters AMP permalink. @@ -806,7 +806,7 @@ function amp_add_amphtml_link() { } if ( AMP_Theme_Support::is_paired_available() ) { - $amp_url = amp_get_url( amp_get_current_url() ); + $amp_url = amp_get_paired_url( amp_get_current_url() ); } else { $amp_url = amp_get_permalink( get_queried_object_id() ); } @@ -1878,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 = amp_get_paired_url( amp_get_current_url() ); } $href = remove_query_arg( QueryVar::NOAMP, $href ); @@ -1913,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'] ); + $args['href'] = amp_get_paired_url( $args['href'] ); } $wp_admin_bar->add_node( $args ); } @@ -1952,12 +1952,11 @@ function amp_generate_script_hash( $script ) { * 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 ) { +function amp_get_paired_url( $url ) { return add_query_arg( amp_get_slug(), '1', $url ); } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 0579da59afa..0cbfc972b8d 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -412,7 +412,7 @@ public static function ensure_proper_amp_location() { wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { $current_url = amp_get_current_url(); - $redirect_url = amp_get_url( amp_remove_endpoint( $current_url ) ); + $redirect_url = amp_get_paired_url( amp_remove_endpoint( $current_url ) ); if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php index 56cbf6829aa..75d128bad43 100644 --- a/includes/embeds/class-amp-core-block-handler.php +++ b/includes/embeds/class-amp-core-block-handler.php @@ -313,7 +313,7 @@ private function process_archives_widgets( Document $dom, $args = [] ) { * * @var DOMElement $option */ - $option->setAttribute( 'value', amp_get_url( $option->getAttribute( 'value' ) ) ); + $option->setAttribute( 'value', amp_get_paired_url( $option->getAttribute( 'value' ) ) ); } } } diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index 78317623fe6..ca19f29b979 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -87,7 +87,7 @@ public function sanitize() { // Record that action was converted to action-xhr. $action_url = add_query_arg( AMP_HTTP::ACTION_XHR_CONVERTED_QUERY_VAR, 1, $action_url ); if ( ! amp_is_canonical() ) { - $action_url = amp_get_url( $action_url ); + $action_url = amp_get_paired_url( $action_url ); } $node->setAttribute( 'action-xhr', $action_url ); // Append success/error handlers if not found. diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 7ea202dd2d5..086bafb104f 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -194,13 +194,14 @@ private function process_element( DOMElement $element, $attribute_name ) { // Only add the AMP query var when requested (in Transitional or Reader mode). if ( ! empty( $this->args['paired'] ) ) { - $query_vars[ amp_get_slug() ] = '1'; // @todo Would be preferable to use amp_get_url() somehow here. + $query_vars[ amp_get_slug() ] = '1'; // @todo Would be preferable to use amp_get_paired_url() somehow here. } } /** * Filters the query vars that are added to the link/form which is considered for AMP-to-AMP linking. * + * @todo This may end up not being the right approach anymore. We may need to instead just pass the action into amp_get_paired_url(). * @internal * * @param string[] $query_vars Query vars. @@ -221,7 +222,7 @@ private function process_element( DOMElement $element, $attribute_name ) { $element->appendChild( $input ); } } else { - $url = add_query_arg( $query_vars, $url ); + $url = add_query_arg( $query_vars, $url ); // @todo Instead make use of amp_get_paired_url(). $element->setAttribute( $attribute_name, $url ); } } diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index 17c35049f29..2e2cdfb680f 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -9,7 +9,7 @@ $url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); -$amp_url = amp_get_url( $url ); +$amp_url = amp_get_paired_url( $url ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index e0c35c73daa..5b98fc78564 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -726,7 +726,7 @@ public static function get_url_from_post( $post ) { // Add AMP query var if in transitional mode. if ( ! amp_is_canonical() ) { - $url = amp_get_url( $url ); + $url = amp_get_paired_url( $url ); } // Set URL scheme based on whether HTTPS is current. diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index d0e5e5f9e7a..85e819d6c4c 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -350,7 +350,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $current_url ); if ( ! amp_is_canonical() ) { - $amp_url = amp_get_url( $amp_url ); + $amp_url = amp_get_paired_url( $amp_url ); } $validate_url = AMP_Validated_URL_Post_Type::get_recheck_url( AMP_Validated_URL_Post_Type::get_invalid_url_post( $amp_url ) ?: $amp_url ); diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index ddf3fbbe44e..7c41d0e0750 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -88,7 +88,7 @@ public function sanitize_options( $options, $new_options ) { * @return string AMP URL. */ public function get_current_amp_url() { - $url = amp_get_url( amp_get_current_url() ); + $url = amp_get_paired_url( amp_get_current_url() ); $url = remove_query_arg( QueryVar::NOAMP, $url ); return $url; } diff --git a/tests/php/src/MobileRedirectionTest.php b/tests/php/src/MobileRedirectionTest.php index a03b092d2b8..286b1f8a435 100644 --- a/tests/php/src/MobileRedirectionTest.php +++ b/tests/php/src/MobileRedirectionTest.php @@ -119,7 +119,7 @@ public function test_sanitize_options() { public function test_get_current_amp_url() { $this->go_to( add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, '/foo/' ) ); $this->assertEquals( - amp_get_url( home_url( '/foo/' ) ), + amp_get_paired_url( home_url( '/foo/' ) ), $this->instance->get_current_amp_url() ); } @@ -151,7 +151,7 @@ public function test_redirect_on_transitional_and_not_available() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); AMP_Options_Manager::update_option( Option::ALL_TEMPLATES_SUPPORTED, false ); AMP_Options_Manager::update_option( Option::SUPPORTED_TEMPLATES, [ 'is_author' ] ); - $this->go_to( amp_get_url( '/' ) ); + $this->go_to( amp_get_paired_url( '/' ) ); $this->assertFalse( amp_is_canonical() ); $this->assertFalse( amp_is_available() ); $this->instance->redirect(); @@ -185,7 +185,7 @@ public function test_redirect_when_server_side_and_not_applicable() { add_filter( 'amp_mobile_client_side_redirection', '__return_false' ); add_filter( 'amp_pre_is_mobile', '__return_false' ); - $this->go_to( amp_get_url( '/' ) ); + $this->go_to( amp_get_paired_url( '/' ) ); $this->assertFalse( amp_is_request() ); $this->assertFalse( $this->instance->is_mobile_request() ); @@ -226,7 +226,7 @@ static function ( $redirect_url ) use ( &$redirected_url ) { $this->instance->redirect(); $this->assertNotNull( $redirected_url ); $this->assertEquals( - amp_get_url( home_url( '/' ) ), + amp_get_paired_url( home_url( '/' ) ), $redirected_url ); } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index f007a597d2c..3a93e447cf8 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -641,14 +641,14 @@ public function get_reader_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - amp_get_url( home_url( '/' ) ), + amp_get_paired_url( home_url( '/' ) ), false, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - amp_get_url( home_url( '/no-existe/' ) ), + amp_get_paired_url( home_url( '/no-existe/' ) ), false, ]; }, @@ -740,14 +740,14 @@ public function get_transitional_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - amp_get_url( home_url( '/' ) ), + amp_get_paired_url( home_url( '/' ) ), true, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - amp_get_url( home_url( '/no-existe/' ) ), + amp_get_paired_url( home_url( '/no-existe/' ) ), true, ]; }, @@ -1054,7 +1054,7 @@ public function test_amp_is_request_before_wp_action_for_reader_mode() { */ public function test_amp_is_request_before_wp_action_for_transitional_mode_with_query_var() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_url( home_url( '/' ) ) ); + $this->go_to( amp_get_paired_url( home_url( '/' ) ) ); global $wp_actions; unset( $wp_actions['wp'] ); $this->assertTrue( AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ); @@ -1818,6 +1818,13 @@ public function test_amp_generate_script_hash() { $this->assertSame( 'sha384-_MAJ0_NC2k8jrjehfi-5LdQasBICZXvp4gOwOx0D3mIStvDCGvZDzcTfXLgMrLL1', amp_generate_script_hash( 'document.body.textContent = \'\';' ) ); } + /** @covers ::amp_get_paired_url() */ + public function test_amp_get_paired_url() { + $this->assertEquals( home_url( '/?amp=1' ), amp_get_paired_url( home_url( '/' ) ) ); + $this->assertEquals( home_url( '/?foo=bar&=1' ), amp_get_paired_url( home_url( '/?foo=bar' ) ) ); + $this->assertEquals( home_url( '/?foo=bar&=1#baz' ), amp_get_paired_url( home_url( '/?foo=bar#baz' ) ) ); + } + /** @return array */ public function data_amp_has_query_var() { return [ diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index b8fd4202813..a8ca8582d44 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -252,7 +252,7 @@ public function test_disable_mobile_redirect_for_excluded_url() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_url( home_url( '/' ) ) ); + $this->go_to( amp_get_paired_url( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); @@ -274,7 +274,7 @@ public function test_disable_mobile_redirect_for_url_with_noamphtml_rel() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_url( home_url( '/' ) ) ); + $this->go_to( amp_get_paired_url( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index f686d786185..fc1591df2ff 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -324,7 +324,7 @@ function ( $url ) use ( $requested_url, &$redirected ) { return null; } ); - $this->go_to( amp_get_url( $requested_url ) ); + $this->go_to( amp_get_paired_url( $requested_url ) ); $this->assertTrue( $redirected ); } @@ -343,7 +343,7 @@ public function test_ensure_proper_amp_location_canonical() { // URL query param. $_GET[ amp_get_slug() ] = ''; - $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_get_paired_url( '/foo/bar' ); try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { @@ -405,11 +405,11 @@ static function ( $url ) use ( &$redirections ) { $this->go_to( $permalink . 'amp/amp/' ); $this->assertCount( 1, $redirections ); - $this->assertEquals( amp_get_url( $permalink ), end( $redirections ) ); + $this->assertEquals( amp_get_paired_url( $permalink ), end( $redirections ) ); $this->go_to( $permalink . 'amp/foo/' ); $this->assertCount( 2, $redirections ); - $this->assertEquals( amp_get_url( $permalink ), end( $redirections ) ); + $this->assertEquals( amp_get_paired_url( $permalink ), end( $redirections ) ); } /** @@ -431,7 +431,7 @@ static function( $code ) use ( &$redirect_status_code ) { ); // Try AMP URL param. - $_SERVER['REQUEST_URI'] = amp_get_url( '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_get_paired_url( '/foo/bar' ); try { $redirect_status_code = null; $this->assertTrue( AMP_Theme_Support::redirect_non_amp_url( 302 ) ); diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 2d5a9f175be..6536fdd6c5d 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -327,7 +327,7 @@ public function test_get_url_from_post() { $this->assertNotInstanceOf( 'WP_Error', $invalid_post_id ); $this->assertEquals( - amp_get_url( get_permalink( $post ) ), + amp_get_paired_url( get_permalink( $post ) ), AMP_Validated_URL_Post_Type::get_url_from_post( $invalid_post_id ) ); From d5610f2fba6ec8d8355c549a11729107f2ebfb2f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 6 Oct 2020 15:06:46 -0700 Subject: [PATCH 11/16] Assemble CRUD functions for paired AMP endpoints --- includes/admin/class-amp-post-meta-box.php | 4 +- includes/amp-helper-functions.php | 109 ++++++++++++------ includes/class-amp-theme-support.php | 8 +- .../embeds/class-amp-core-block-handler.php | 2 +- .../options/class-amp-options-manager.php | 2 +- .../sanitizers/class-amp-form-sanitizer.php | 2 +- .../sanitizers/class-amp-link-sanitizer.php | 4 +- includes/templates/amp-paired-browsing.php | 2 +- .../class-amp-validated-url-post-type.php | 2 +- .../class-amp-validation-manager.php | 2 +- src/MobileRedirection.php | 2 +- src/PluginSuppression.php | 2 +- src/ReaderThemeLoader.php | 2 +- tests/php/src/MobileRedirectionTest.php | 8 +- tests/php/test-amp-helper-functions.php | 71 +++++++----- tests/php/test-class-amp-link-sanitizer.php | 4 +- tests/php/test-class-amp-theme-support.php | 10 +- ...test-class-amp-validated-url-post-type.php | 2 +- 18 files changed, 148 insertions(+), 90 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index c9e7137b951..64c02a0c6af 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -188,7 +188,7 @@ public function enqueue_admin_assets() { 'ampPostMetaBox.boot( %s );', wp_json_encode( [ - 'previewLink' => esc_url_raw( amp_get_paired_url( get_preview_post_link( $post ) ) ), + 'previewLink' => esc_url_raw( amp_get_paired_endpoint( get_preview_post_link( $post ) ) ), 'canonical' => amp_is_canonical(), 'enabled' => empty( $support_errors ), 'canSupport' => 0 === count( array_diff( $support_errors, [ 'post-status-disabled' ] ) ), @@ -447,7 +447,7 @@ public function preview_post_link( $link ) { ); if ( $is_amp ) { - $link = amp_get_paired_url( $link ); + $link = amp_get_paired_endpoint( $link ); } return $link; diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 2354482e698..77aa649014e 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -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_paired_url( $link ); + $link = amp_get_paired_endpoint( $link ); } else { $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); } @@ -717,7 +717,7 @@ function amp_get_permalink( $post_id ) { } $permalink = get_permalink( $post_id ); - $amp_url = amp_is_canonical() ? $permalink : amp_get_paired_url( $permalink ); + $amp_url = amp_is_canonical() ? $permalink : amp_get_paired_endpoint( $permalink ); /** * Filters AMP permalink. @@ -735,27 +735,14 @@ function amp_get_permalink( $post_id ) { * Remove the AMP endpoint (and query var) from a given URL. * * @since 0.7 + * @since 2.1 Deprecated. + * @deprecated Use amp_remove_paired_endpoint() instead. * * @param string $url URL. * @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; + return amp_remove_paired_endpoint( $url ); } /** @@ -806,7 +793,7 @@ function amp_add_amphtml_link() { } if ( AMP_Theme_Support::is_paired_available() ) { - $amp_url = amp_get_paired_url( amp_get_current_url() ); + $amp_url = amp_get_paired_endpoint( amp_get_current_url() ); } else { $amp_url = amp_get_permalink( get_queried_object_id() ); } @@ -869,7 +856,7 @@ function amp_is_request() { $is_amp_url = ( amp_is_canonical() || - amp_has_query_var() + amp_is_paired_endpoint() ); // If AMP is not available, then it's definitely not an AMP endpoint. @@ -1878,7 +1865,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_paired_url( amp_get_current_url() ); + $href = amp_get_paired_endpoint( amp_get_current_url() ); } $href = remove_query_arg( QueryVar::NOAMP, $href ); @@ -1913,7 +1900,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_paired_url( $args['href'] ); + $args['href'] = amp_get_paired_endpoint( $args['href'] ); } $wp_admin_bar->add_node( $args ); } @@ -1949,14 +1936,14 @@ function amp_generate_script_hash( $script ) { } /** - * Get the AMP version of a URL. + * Get the paired AMP endpoint for a URL. * * @since 2.1 * * @param string $url URL. * @return string AMP URL. */ -function amp_get_paired_url( $url ) { +function amp_get_paired_endpoint( $url ) { return add_query_arg( amp_get_slug(), '1', $url ); } @@ -1964,21 +1951,73 @@ function amp_get_paired_url( $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. * + * @param string $url URL to examine. If empty, will use the current URL. * @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() { - global $wp_query; +function amp_is_paired_endpoint( $url = '' ) { $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 ) - ) + + // If the URL was not provided, then use the environment which is already parsed. + if ( empty( $url ) ) { + global $wp_query; + return ( + isset( $_GET[ $slug ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + || + ( + $wp_query instanceof WP_Query + && + false !== $wp_query->get( $slug, false ) + ) + ); + } + + $parsed_url = wp_parse_url( $url ); + if ( ! empty( $parsed_url['query'] ) ) { + $query_vars = []; + wp_parse_str( $parsed_url['query'], $query_vars ); + if ( isset( $query_vars[ $slug ] ) ) { + return true; + } + } + + if ( ! empty( $parsed_url['path'] ) ) { + $pattern = sprintf( + '#/%s(/[^/^])?/?$#', + preg_quote( $slug, '#' ) + ); + if ( preg_match( $pattern, $parsed_url['path'] ) ) { + return true; + } + } + + return false; +} + +/** + * Remove the paired AMP endpoint from a given URL. + * + * @since 2.1 + * + * @param string $url URL. + * @return string URL with AMP stripped. + */ +function amp_remove_paired_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; } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 0cbfc972b8d..2b11451fda8 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -334,7 +334,7 @@ public static function finish_init() { * enabled by user ay any time, so they will be able to make AMP available for this URL and see the change * without wrestling with the redirect cache. */ - if ( amp_has_query_var() ) { + if ( amp_is_paired_endpoint() ) { self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } @@ -397,10 +397,10 @@ public static function ensure_proper_amp_location() { * should happen infrequently. For admin users, this is kept temporary to allow them * to not be hampered by browser remembering permanent redirects and preventing test. */ - if ( amp_has_query_var() ) { + if ( amp_is_paired_endpoint() ) { return self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } - } elseif ( amp_has_query_var() ) { + } elseif ( amp_is_paired_endpoint() ) { /* * Prevent infinite URL space under /amp/ endpoint. Note that WordPress allows endpoints to have a value, * such as the case of /feed/ where /feed/atom/ is the same as saying ?feed=atom. In this case, we need to @@ -412,7 +412,7 @@ public static function ensure_proper_amp_location() { wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { $current_url = amp_get_current_url(); - $redirect_url = amp_get_paired_url( amp_remove_endpoint( $current_url ) ); + $redirect_url = amp_get_paired_endpoint( amp_remove_endpoint( $current_url ) ); if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php index 75d128bad43..087c4b51626 100644 --- a/includes/embeds/class-amp-core-block-handler.php +++ b/includes/embeds/class-amp-core-block-handler.php @@ -313,7 +313,7 @@ private function process_archives_widgets( Document $dom, $args = [] ) { * * @var DOMElement $option */ - $option->setAttribute( 'value', amp_get_paired_url( $option->getAttribute( 'value' ) ) ); + $option->setAttribute( 'value', amp_get_paired_endpoint( $option->getAttribute( 'value' ) ) ); } } } diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php index cd2248d7c2d..12cbf7ccf5f 100644 --- a/includes/options/class-amp-options-manager.php +++ b/includes/options/class-amp-options-manager.php @@ -171,7 +171,7 @@ static function ( $supported ) { && get_stylesheet() === $options[ Option::READER_THEME ] && - ! amp_has_query_var() + ! amp_is_paired_endpoint() ) { /* * When Reader mode is selected and a Reader theme has been chosen, if the active theme switches to be the diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index ca19f29b979..9c910e689e9 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -87,7 +87,7 @@ public function sanitize() { // Record that action was converted to action-xhr. $action_url = add_query_arg( AMP_HTTP::ACTION_XHR_CONVERTED_QUERY_VAR, 1, $action_url ); if ( ! amp_is_canonical() ) { - $action_url = amp_get_paired_url( $action_url ); + $action_url = amp_get_paired_endpoint( $action_url ); } $node->setAttribute( 'action-xhr', $action_url ); // Append success/error handlers if not found. diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 086bafb104f..d0b8dbf1a13 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -194,7 +194,7 @@ private function process_element( DOMElement $element, $attribute_name ) { // Only add the AMP query var when requested (in Transitional or Reader mode). if ( ! empty( $this->args['paired'] ) ) { - $query_vars[ amp_get_slug() ] = '1'; // @todo Would be preferable to use amp_get_paired_url() somehow here. + $query_vars[ amp_get_slug() ] = '1'; // @todo Would be preferable to use amp_get_paired_endpoint() somehow here. } } @@ -222,7 +222,7 @@ private function process_element( DOMElement $element, $attribute_name ) { $element->appendChild( $input ); } } else { - $url = add_query_arg( $query_vars, $url ); // @todo Instead make use of amp_get_paired_url(). + $url = add_query_arg( $query_vars, $url ); // @todo Instead make use of amp_get_paired_endpoint(). $element->setAttribute( $attribute_name, $url ); } } diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index 2e2cdfb680f..fc9be09af49 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -9,7 +9,7 @@ $url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); -$amp_url = amp_get_paired_url( $url ); +$amp_url = amp_get_paired_endpoint( $url ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 5b98fc78564..8279e5c97a2 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -726,7 +726,7 @@ public static function get_url_from_post( $post ) { // Add AMP query var if in transitional mode. if ( ! amp_is_canonical() ) { - $url = amp_get_paired_url( $url ); + $url = amp_get_paired_endpoint( $url ); } // Set URL scheme based on whether HTTPS is current. diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 85e819d6c4c..83f25e91e34 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -350,7 +350,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $current_url ); if ( ! amp_is_canonical() ) { - $amp_url = amp_get_paired_url( $amp_url ); + $amp_url = amp_get_paired_endpoint( $amp_url ); } $validate_url = AMP_Validated_URL_Post_Type::get_recheck_url( AMP_Validated_URL_Post_Type::get_invalid_url_post( $amp_url ) ?: $amp_url ); diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index 7c41d0e0750..497c39a94eb 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -88,7 +88,7 @@ public function sanitize_options( $options, $new_options ) { * @return string AMP URL. */ public function get_current_amp_url() { - $url = amp_get_paired_url( amp_get_current_url() ); + $url = amp_get_paired_endpoint( amp_get_current_url() ); $url = remove_query_arg( QueryVar::NOAMP, $url ); return $url; } diff --git a/src/PluginSuppression.php b/src/PluginSuppression.php index cc74bd6da9f..62e73be16ac 100644 --- a/src/PluginSuppression.php +++ b/src/PluginSuppression.php @@ -92,7 +92,7 @@ public function is_reader_theme_request() { && ReaderThemes::DEFAULT_READER_THEME !== AMP_Options_Manager::get_option( Option::READER_THEME ) && - amp_has_query_var() + amp_is_paired_endpoint() ); } diff --git a/src/ReaderThemeLoader.php b/src/ReaderThemeLoader.php index 672454c0ba2..9dbaa6acc6f 100644 --- a/src/ReaderThemeLoader.php +++ b/src/ReaderThemeLoader.php @@ -302,7 +302,7 @@ public function get_active_theme() { */ public function override_theme() { $this->theme_overridden = false; - if ( ! $this->is_enabled() || ! amp_has_query_var() ) { + if ( ! $this->is_enabled() || ! amp_is_paired_endpoint() ) { return; } diff --git a/tests/php/src/MobileRedirectionTest.php b/tests/php/src/MobileRedirectionTest.php index 286b1f8a435..ad6de550913 100644 --- a/tests/php/src/MobileRedirectionTest.php +++ b/tests/php/src/MobileRedirectionTest.php @@ -119,7 +119,7 @@ public function test_sanitize_options() { public function test_get_current_amp_url() { $this->go_to( add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, '/foo/' ) ); $this->assertEquals( - amp_get_paired_url( home_url( '/foo/' ) ), + amp_get_paired_endpoint( home_url( '/foo/' ) ), $this->instance->get_current_amp_url() ); } @@ -151,7 +151,7 @@ public function test_redirect_on_transitional_and_not_available() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); AMP_Options_Manager::update_option( Option::ALL_TEMPLATES_SUPPORTED, false ); AMP_Options_Manager::update_option( Option::SUPPORTED_TEMPLATES, [ 'is_author' ] ); - $this->go_to( amp_get_paired_url( '/' ) ); + $this->go_to( amp_get_paired_endpoint( '/' ) ); $this->assertFalse( amp_is_canonical() ); $this->assertFalse( amp_is_available() ); $this->instance->redirect(); @@ -185,7 +185,7 @@ public function test_redirect_when_server_side_and_not_applicable() { add_filter( 'amp_mobile_client_side_redirection', '__return_false' ); add_filter( 'amp_pre_is_mobile', '__return_false' ); - $this->go_to( amp_get_paired_url( '/' ) ); + $this->go_to( amp_get_paired_endpoint( '/' ) ); $this->assertFalse( amp_is_request() ); $this->assertFalse( $this->instance->is_mobile_request() ); @@ -226,7 +226,7 @@ static function ( $redirect_url ) use ( &$redirected_url ) { $this->instance->redirect(); $this->assertNotNull( $redirected_url ); $this->assertEquals( - amp_get_paired_url( home_url( '/' ) ), + amp_get_paired_endpoint( home_url( '/' ) ), $redirected_url ); } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 3a93e447cf8..3c3a57070bf 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -605,19 +605,19 @@ public function test_amp_get_permalink_with_theme_support() { } /** - * Test amp_remove_endpoint. + * Test amp_remove_paired_endpoint. * - * @covers ::amp_remove_endpoint() + * @covers ::amp_remove_paired_endpoint() */ - public function test_amp_remove_endpoint() { - $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/?amp' ) ); - $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/?amp=1' ) ); - $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/amp/?amp=1' ) ); - $this->assertEquals( 'https://example.com/foo/?#bar', amp_remove_endpoint( 'https://example.com/foo/?amp#bar' ) ); - $this->assertEquals( 'https://example.com/foo/', amp_remove_endpoint( 'https://example.com/foo/amp/' ) ); - $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/?blaz' ) ); - $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/amp/?blaz' ) ); - $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_endpoint( 'https://example.com/foo/amp/foo/amp/bar/?blaz' ) ); + public function test_amp_remove_paired_endpoint() { + $this->assertEquals( 'https://example.com/foo/', amp_remove_paired_endpoint( 'https://example.com/foo/?amp' ) ); + $this->assertEquals( 'https://example.com/foo/', amp_remove_paired_endpoint( 'https://example.com/foo/?amp=1' ) ); + $this->assertEquals( 'https://example.com/foo/', amp_remove_paired_endpoint( 'https://example.com/foo/amp/?amp=1' ) ); + $this->assertEquals( 'https://example.com/foo/?#bar', amp_remove_paired_endpoint( 'https://example.com/foo/?amp#bar' ) ); + $this->assertEquals( 'https://example.com/foo/', amp_remove_paired_endpoint( 'https://example.com/foo/amp/' ) ); + $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_paired_endpoint( 'https://example.com/foo/amp/?blaz' ) ); + $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_paired_endpoint( 'https://example.com/foo/amp/amp/?blaz' ) ); + $this->assertEquals( 'https://example.com/foo/?blaz', amp_remove_paired_endpoint( 'https://example.com/foo/amp/foo/amp/bar/?blaz' ) ); } /** @@ -641,14 +641,14 @@ public function get_reader_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - amp_get_paired_url( home_url( '/' ) ), + amp_get_paired_endpoint( home_url( '/' ) ), false, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - amp_get_paired_url( home_url( '/no-existe/' ) ), + amp_get_paired_endpoint( home_url( '/no-existe/' ) ), false, ]; }, @@ -740,14 +740,14 @@ public function get_transitional_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - amp_get_paired_url( home_url( '/' ) ), + amp_get_paired_endpoint( home_url( '/' ) ), true, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - amp_get_paired_url( home_url( '/no-existe/' ) ), + amp_get_paired_endpoint( home_url( '/no-existe/' ) ), true, ]; }, @@ -1054,7 +1054,7 @@ public function test_amp_is_request_before_wp_action_for_reader_mode() { */ public function test_amp_is_request_before_wp_action_for_transitional_mode_with_query_var() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_paired_url( home_url( '/' ) ) ); + $this->go_to( amp_get_paired_endpoint( home_url( '/' ) ) ); global $wp_actions; unset( $wp_actions['wp'] ); $this->assertTrue( AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ); @@ -1818,15 +1818,15 @@ public function test_amp_generate_script_hash() { $this->assertSame( 'sha384-_MAJ0_NC2k8jrjehfi-5LdQasBICZXvp4gOwOx0D3mIStvDCGvZDzcTfXLgMrLL1', amp_generate_script_hash( 'document.body.textContent = \'\';' ) ); } - /** @covers ::amp_get_paired_url() */ - public function test_amp_get_paired_url() { - $this->assertEquals( home_url( '/?amp=1' ), amp_get_paired_url( home_url( '/' ) ) ); - $this->assertEquals( home_url( '/?foo=bar&=1' ), amp_get_paired_url( home_url( '/?foo=bar' ) ) ); - $this->assertEquals( home_url( '/?foo=bar&=1#baz' ), amp_get_paired_url( home_url( '/?foo=bar#baz' ) ) ); + /** @covers ::amp_get_paired_endpoint() */ + public function test_amp_get_paired_endpoint() { + $this->assertEquals( home_url( '/?amp=1' ), amp_get_paired_endpoint( home_url( '/' ) ) ); + $this->assertEquals( home_url( '/?foo=bar&=1' ), amp_get_paired_endpoint( home_url( '/?foo=bar' ) ) ); + $this->assertEquals( home_url( '/?foo=bar&=1#baz' ), amp_get_paired_endpoint( home_url( '/?foo=bar#baz' ) ) ); } /** @return array */ - public function data_amp_has_query_var() { + public function data_amp_is_paired_endpoint() { return [ 'nothing' => [ '', @@ -1856,16 +1856,21 @@ public function data_amp_has_query_var() { 'amp/?amp=1', true, ], + 'endpoint_with_extras' => [ + 'amp/?foo=var#baz', + true, + ], ]; } /** - * @dataProvider data_amp_has_query_var - * @covers ::amp_has_query_var() + * @dataProvider data_amp_is_paired_endpoint + * @covers ::amp_is_paired_endpoint() + * * @param string $suffix * @param bool $is_amp */ - public function test_amp_has_query_var( $suffix, $is_amp ) { + public function test_amp_is_paired_endpoint_go_to( $suffix, $is_amp ) { add_filter( 'wp_redirect', '__return_empty_string' ); // Prevent ensure_proper_amp_location() from redirecting. global $wp_rewrite; update_option( 'permalink_structure', '/%year%/%monthnum%/%day%/%postname%/' ); @@ -1879,7 +1884,21 @@ public function test_amp_has_query_var( $suffix, $is_amp ) { $this->go_to( $url ); $this->assertTrue( is_singular(), 'Expected singular query.' ); $this->assertTrue( amp_is_available(), 'Expected AMP to be available.' ); - $this->assertEquals( $is_amp, amp_has_query_var() ); + $this->assertEquals( $is_amp, amp_is_paired_endpoint() ); + } + + /** + * @dataProvider data_amp_is_paired_endpoint + * @covers ::amp_is_paired_endpoint() + * + * @param string $suffix + * @param bool $is_amp + */ + public function test_amp_is_paired_endpoint_passed( $suffix, $is_amp ) { + $permalink = home_url( '/foo/' ); + $this->assertNotContains( '?', $permalink ); + $url = $permalink . $suffix; + $this->assertEquals( $is_amp, amp_is_paired_endpoint( $url ) ); } /** diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index a8ca8582d44..97a8042aca9 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -252,7 +252,7 @@ public function test_disable_mobile_redirect_for_excluded_url() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_paired_url( home_url( '/' ) ) ); + $this->go_to( amp_get_paired_endpoint( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); @@ -274,7 +274,7 @@ public function test_disable_mobile_redirect_for_url_with_noamphtml_rel() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_paired_url( home_url( '/' ) ) ); + $this->go_to( amp_get_paired_endpoint( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index fc1591df2ff..7ec8d96479d 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -324,7 +324,7 @@ function ( $url ) use ( $requested_url, &$redirected ) { return null; } ); - $this->go_to( amp_get_paired_url( $requested_url ) ); + $this->go_to( amp_get_paired_endpoint( $requested_url ) ); $this->assertTrue( $redirected ); } @@ -343,7 +343,7 @@ public function test_ensure_proper_amp_location_canonical() { // URL query param. $_GET[ amp_get_slug() ] = ''; - $_SERVER['REQUEST_URI'] = amp_get_paired_url( '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_get_paired_endpoint( '/foo/bar' ); try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { @@ -405,11 +405,11 @@ static function ( $url ) use ( &$redirections ) { $this->go_to( $permalink . 'amp/amp/' ); $this->assertCount( 1, $redirections ); - $this->assertEquals( amp_get_paired_url( $permalink ), end( $redirections ) ); + $this->assertEquals( amp_get_paired_endpoint( $permalink ), end( $redirections ) ); $this->go_to( $permalink . 'amp/foo/' ); $this->assertCount( 2, $redirections ); - $this->assertEquals( amp_get_paired_url( $permalink ), end( $redirections ) ); + $this->assertEquals( amp_get_paired_endpoint( $permalink ), end( $redirections ) ); } /** @@ -431,7 +431,7 @@ static function( $code ) use ( &$redirect_status_code ) { ); // Try AMP URL param. - $_SERVER['REQUEST_URI'] = amp_get_paired_url( '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_get_paired_endpoint( '/foo/bar' ); try { $redirect_status_code = null; $this->assertTrue( AMP_Theme_Support::redirect_non_amp_url( 302 ) ); diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 6536fdd6c5d..1c3c46673e1 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -327,7 +327,7 @@ public function test_get_url_from_post() { $this->assertNotInstanceOf( 'WP_Error', $invalid_post_id ); $this->assertEquals( - amp_get_paired_url( get_permalink( $post ) ), + amp_get_paired_endpoint( get_permalink( $post ) ), AMP_Validated_URL_Post_Type::get_url_from_post( $invalid_post_id ) ); From aafcaeca615bafa2b3cbe6b097aaab7dba756b9b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 6 Oct 2020 15:13:02 -0700 Subject: [PATCH 12/16] Skip test_theme_data_exists when Neve already installed --- tests/php/src/Admin/ReaderThemesTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/php/src/Admin/ReaderThemesTest.php b/tests/php/src/Admin/ReaderThemesTest.php index 432c74fe508..291089ff438 100644 --- a/tests/php/src/Admin/ReaderThemesTest.php +++ b/tests/php/src/Admin/ReaderThemesTest.php @@ -344,7 +344,9 @@ public function test_can_install_theme() { * @covers ::theme_data_exists */ public function test_theme_data_exists() { - $this->assertFalse( ( new ReaderThemes() )->theme_data_exists( 'neve' ) ); + if ( ( new ReaderThemes() )->theme_data_exists( 'neve' ) ) { + $this->markTestSkipped( 'Neve is already installed.' ); + } $neve_theme = [ 'name' => 'Neve', From 04f53d933c4486fc4341a5b7410946de73a2472f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 6 Oct 2020 15:53:43 -0700 Subject: [PATCH 13/16] Remove hard-coded dependency on amp=1 query var for AMP-to-AMP linking --- .../sanitizers/class-amp-link-sanitizer.php | 46 +++++++++++-------- tests/php/test-class-amp-link-sanitizer.php | 2 +- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index d0b8dbf1a13..17959e60d87 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -183,25 +183,19 @@ private function process_element( DOMElement $element, $attribute_name ) { $query_vars = []; + // Add rel=amphtml. if ( ! $excluded ) { $rel[] = Attribute::REL_AMPHTML; $rel = array_diff( $rel, [ Attribute::REL_NOAMPHTML ] ); - $element->setAttribute( Attribute::REL, implode( ' ', $rel ) ); - - // Only add the AMP query var when requested (in Transitional or Reader mode). - if ( ! empty( $this->args['paired'] ) ) { - $query_vars[ amp_get_slug() ] = '1'; // @todo Would be preferable to use amp_get_paired_endpoint() somehow here. - } } /** * Filters the query vars that are added to the link/form which is considered for AMP-to-AMP linking. * - * @todo This may end up not being the right approach anymore. We may need to instead just pass the action into amp_get_paired_url(). * @internal * * @param string[] $query_vars Query vars. @@ -212,18 +206,32 @@ private function process_element( DOMElement $element, $attribute_name ) { */ $query_vars = apply_filters( 'amp_to_amp_linking_element_query_vars', $query_vars, $excluded, $url, $element, $rel ); - if ( $query_vars ) { - if ( Tag::FORM === $element->nodeName ) { - foreach ( $query_vars as $name => $value ) { - $input = $this->dom->createElement( Tag::INPUT ); - $input->setAttribute( Attribute::NAME, $name ); - $input->setAttribute( Attribute::VALUE, $value ); - $input->setAttribute( Attribute::TYPE, 'hidden' ); - $element->appendChild( $input ); - } - } else { - $url = add_query_arg( $query_vars, $url ); // @todo Instead make use of amp_get_paired_endpoint(). - $element->setAttribute( $attribute_name, $url ); + if ( ! empty( $query_vars ) ) { + $url = add_query_arg( $query_vars, $url ); + } + + // Only add the AMP query var when requested (in Transitional or Reader mode). + if ( ! $excluded && ! empty( $this->args['paired'] ) ) { + $url = amp_get_paired_endpoint( $url ); + } + + $element->setAttribute( $attribute_name, $url ); + + // Given that form action query vars get overridden by the inputs, they need to be extracted and added as inputs. + if ( Tag::FORM === $element->nodeName ) { + $query = wp_parse_url( $url, PHP_URL_QUERY ); + if ( $query ) { + $parsed_query_vars = []; + wp_parse_str( $query, $parsed_query_vars ); + $query_vars = array_merge( $query_vars, $parsed_query_vars ); + } + + foreach ( $query_vars as $name => $value ) { + $input = $this->dom->createElement( Tag::INPUT ); + $input->setAttribute( Attribute::NAME, $name ); + $input->setAttribute( Attribute::VALUE, $value ); + $input->setAttribute( Attribute::TYPE, 'hidden' ); + $element->appendChild( $input ); } } } diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index 97a8042aca9..2929c85ba26 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -195,7 +195,7 @@ public function test_amp_to_amp_navigation( $paired ) { if ( $paired && $link_data['expected_amp'] ) { $this->assertStringContains( '?' . amp_get_slug(), $element->getAttribute( 'href' ), "ID: $id" ); } elseif ( ! $paired || ! $link_data['expected_amp'] ) { - $this->assertStringNotContains( '?' . amp_get_slug(), $element->getAttribute( 'href' ), "ID: $id" ); + $this->assertStringNotContains( '?' . amp_get_slug() . '=1', $element->getAttribute( 'href' ), "ID: $id" ); } } From ce34a995865b99460222c5d64d012ed9ab3c8a1f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 6 Oct 2020 20:36:52 -0700 Subject: [PATCH 14/16] Replace usages of amp_remove_endpoint() with amp_remove_paired_endpoint() --- includes/amp-helper-functions.php | 2 +- includes/class-amp-theme-support.php | 10 +++++----- .../validation/class-amp-validated-url-post-type.php | 2 +- includes/validation/class-amp-validation-manager.php | 2 +- src/MobileRedirection.php | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 77aa649014e..e8a152c7842 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1861,7 +1861,7 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { $is_amp_request = amp_is_request(); if ( $is_amp_request ) { - $href = amp_remove_endpoint( amp_get_current_url() ); + $href = amp_remove_paired_endpoint( amp_get_current_url() ); } elseif ( is_singular() ) { $href = amp_get_permalink( get_queried_object_id() ); // For sake of Reader mode. } else { diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 2b11451fda8..8bd16eeaefc 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -412,7 +412,7 @@ public static function ensure_proper_amp_location() { wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { $current_url = amp_get_current_url(); - $redirect_url = amp_get_paired_endpoint( amp_remove_endpoint( $current_url ) ); + $redirect_url = amp_get_paired_endpoint( amp_remove_paired_endpoint( $current_url ) ); if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; @@ -439,7 +439,7 @@ public static function ensure_proper_amp_location() { */ public static function redirect_non_amp_url( $status = 302 ) { $current_url = amp_get_current_url(); - $non_amp_url = amp_remove_endpoint( $current_url ); + $non_amp_url = amp_remove_paired_endpoint( $current_url ); if ( $non_amp_url === $current_url ) { return false; } @@ -1189,7 +1189,7 @@ public static function get_current_canonical_url() { $url = add_query_arg( $added_query_vars, $url ); } - return amp_remove_endpoint( $url ); + return amp_remove_paired_endpoint( $url ); } /** @@ -1834,7 +1834,7 @@ public static function finish_output_buffering( $response ) { // Add link to non-AMP version if not canonical. if ( ! amp_is_canonical() ) { - $non_amp_url = amp_remove_endpoint( amp_get_current_url() ); + $non_amp_url = amp_remove_paired_endpoint( amp_get_current_url() ); // Prevent user from being redirected back to AMP version. if ( true === AMP_Options_Manager::get_option( Option::MOBILE_REDIRECT ) ) { @@ -2189,7 +2189,7 @@ static function( Optimizer\Error $error ) { // Redirect to the non-AMP version if not on an AMP-first site. if ( ! $can_serve && ! amp_is_canonical() ) { - $non_amp_url = amp_remove_endpoint( amp_get_current_url() ); + $non_amp_url = amp_remove_paired_endpoint( amp_get_current_url() ); // Redirect to include query var to preventing AMP from even being considered available. $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_AVAILABLE, $non_amp_url ); diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 8279e5c97a2..17f8c075867 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -764,7 +764,7 @@ protected static function get_markup_status_preview_url( $url ) { */ protected static function normalize_url_for_storage( $url ) { // Only ever store the canonical version. - $url = amp_remove_endpoint( $url ); + $url = amp_remove_paired_endpoint( $url ); // Remove fragment identifier in the rare case it could be provided. It is irrelevant for validation. $url = strtok( $url, '#' ); diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 83f25e91e34..f43afcefe82 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -338,7 +338,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $is_amp_request = amp_is_request(); $current_url = amp_get_current_url(); - $non_amp_url = amp_remove_endpoint( $current_url ); + $non_amp_url = amp_remove_paired_endpoint( $current_url ); $non_amp_url = add_query_arg( QueryVar::NOAMP, amp_is_canonical() ? QueryVar::NOAMP_AVAILABLE : QueryVar::NOAMP_MOBILE, diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index 497c39a94eb..d2eb9117b2d 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -428,7 +428,7 @@ public function add_mobile_version_switcher_link() { $is_amp = amp_is_request(); if ( $is_amp ) { $rel = [ Attribute::REL_NOAMPHTML, Attribute::REL_NOFOLLOW ]; - $url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, amp_remove_endpoint( amp_get_current_url() ) ); + $url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, amp_remove_paired_endpoint( amp_get_current_url() ) ); $text = __( 'Exit mobile version', 'amp' ); } else { $rel = [ Attribute::REL_AMPHTML ]; From 0931662a088f57e533a2eddc1c16af3ae7039a67 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 6 Oct 2020 21:09:01 -0700 Subject: [PATCH 15/16] Use strings for query var values --- tests/php/test-amp-helper-functions.php | 6 +++--- tests/php/test-class-amp-theme-support.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 3c3a57070bf..d9d2d9d62fb 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -863,7 +863,7 @@ public function test_amp_is_request() { $this->assertFalse( amp_is_request() ); // Legacy query var. - set_query_var( amp_get_slug(), 1 ); + set_query_var( amp_get_slug(), '1' ); $this->assertTrue( amp_is_available() ); $this->assertTrue( amp_is_request() ); unset( $GLOBALS['wp_query']->query_vars[ amp_get_slug() ] ); @@ -872,7 +872,7 @@ public function test_amp_is_request() { // Transitional theme support. add_theme_support( AMP_Theme_Support::SLUG, [ 'template_dir' => './' ] ); - $_GET['amp'] = 1; + $_GET['amp'] = '1'; $this->assertTrue( amp_is_available() ); $this->assertTrue( amp_is_request() ); unset( $_GET['amp'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended @@ -1758,7 +1758,7 @@ public function test_amp_add_admin_bar_item() { $this->assertStringNotContains( 'autofocus', $item->href ); // Confirm that link is added to non-AMP version. - set_query_var( amp_get_slug(), 1 ); + set_query_var( amp_get_slug(), '1' ); $this->assertTrue( amp_is_request() ); $admin_bar = new WP_Admin_Bar(); amp_add_admin_bar_view_link( $admin_bar ); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 7ec8d96479d..cf52041374f 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -355,7 +355,7 @@ public function test_ensure_proper_amp_location_canonical() { // Endpoint. unset( $_GET[ amp_get_slug() ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - set_query_var( amp_get_slug(), 1 ); + set_query_var( amp_get_slug(), '1' ); $_SERVER['REQUEST_URI'] = '/2016/01/24/foo/amp/'; try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); From 0f7a3a93e33d30fe3810e0850d892fa8d35acf1f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 9 Oct 2020 16:07:34 -0700 Subject: [PATCH 16/16] Use amp_has_paired_endpoint() and amp_add_paired_endpoint() naming --- includes/admin/class-amp-post-meta-box.php | 4 +- includes/amp-helper-functions.php | 20 +++++----- includes/class-amp-theme-support.php | 8 ++-- .../embeds/class-amp-core-block-handler.php | 2 +- .../options/class-amp-options-manager.php | 2 +- .../sanitizers/class-amp-form-sanitizer.php | 2 +- .../sanitizers/class-amp-link-sanitizer.php | 2 +- includes/templates/amp-paired-browsing.php | 2 +- .../class-amp-validated-url-post-type.php | 2 +- .../class-amp-validation-manager.php | 2 +- src/MobileRedirection.php | 2 +- src/PluginSuppression.php | 2 +- src/ReaderThemeLoader.php | 2 +- tests/php/src/MobileRedirectionTest.php | 8 ++-- tests/php/test-amp-helper-functions.php | 38 +++++++++---------- tests/php/test-class-amp-link-sanitizer.php | 4 +- tests/php/test-class-amp-theme-support.php | 10 ++--- ...test-class-amp-validated-url-post-type.php | 2 +- 18 files changed, 57 insertions(+), 57 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 64c02a0c6af..acae775f39f 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -188,7 +188,7 @@ public function enqueue_admin_assets() { 'ampPostMetaBox.boot( %s );', wp_json_encode( [ - 'previewLink' => esc_url_raw( amp_get_paired_endpoint( get_preview_post_link( $post ) ) ), + 'previewLink' => esc_url_raw( amp_add_paired_endpoint( get_preview_post_link( $post ) ) ), 'canonical' => amp_is_canonical(), 'enabled' => empty( $support_errors ), 'canSupport' => 0 === count( array_diff( $support_errors, [ 'post-status-disabled' ] ) ), @@ -447,7 +447,7 @@ public function preview_post_link( $link ) { ); if ( $is_amp ) { - $link = amp_get_paired_endpoint( $link ); + $link = amp_add_paired_endpoint( $link ); } return $link; diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index e8a152c7842..4da9cbb5a14 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -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_paired_endpoint( $link ); + $link = amp_add_paired_endpoint( $link ); } else { $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); } @@ -717,7 +717,7 @@ function amp_get_permalink( $post_id ) { } $permalink = get_permalink( $post_id ); - $amp_url = amp_is_canonical() ? $permalink : amp_get_paired_endpoint( $permalink ); + $amp_url = amp_is_canonical() ? $permalink : amp_add_paired_endpoint( $permalink ); /** * Filters AMP permalink. @@ -793,7 +793,7 @@ function amp_add_amphtml_link() { } if ( AMP_Theme_Support::is_paired_available() ) { - $amp_url = amp_get_paired_endpoint( amp_get_current_url() ); + $amp_url = amp_add_paired_endpoint( amp_get_current_url() ); } else { $amp_url = amp_get_permalink( get_queried_object_id() ); } @@ -856,7 +856,7 @@ function amp_is_request() { $is_amp_url = ( amp_is_canonical() || - amp_is_paired_endpoint() + amp_has_paired_endpoint() ); // If AMP is not available, then it's definitely not an AMP endpoint. @@ -1865,7 +1865,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_paired_endpoint( amp_get_current_url() ); + $href = amp_add_paired_endpoint( amp_get_current_url() ); } $href = remove_query_arg( QueryVar::NOAMP, $href ); @@ -1900,7 +1900,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_paired_endpoint( $args['href'] ); + $args['href'] = amp_add_paired_endpoint( $args['href'] ); } $wp_admin_bar->add_node( $args ); } @@ -1936,19 +1936,19 @@ function amp_generate_script_hash( $script ) { } /** - * Get the paired AMP endpoint for a URL. + * Turn a given URL into a paired AMP URL. * * @since 2.1 * * @param string $url URL. * @return string AMP URL. */ -function amp_get_paired_endpoint( $url ) { +function amp_add_paired_endpoint( $url ) { return add_query_arg( amp_get_slug(), '1', $url ); } /** - * Determine whether the current request has the AMP query parameter set. + * Determine a given URL is for a paired AMP request. * * @since 2.1 * @@ -1956,7 +1956,7 @@ function amp_get_paired_endpoint( $url ) { * @return bool True if the AMP query parameter is set with the required value, false if not. * @global WP_Query $wp_query */ -function amp_is_paired_endpoint( $url = '' ) { +function amp_has_paired_endpoint( $url = '' ) { $slug = amp_get_slug(); // If the URL was not provided, then use the environment which is already parsed. diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 8bd16eeaefc..065bbf1b2b0 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -334,7 +334,7 @@ public static function finish_init() { * enabled by user ay any time, so they will be able to make AMP available for this URL and see the change * without wrestling with the redirect cache. */ - if ( amp_is_paired_endpoint() ) { + if ( amp_has_paired_endpoint() ) { self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } @@ -397,10 +397,10 @@ public static function ensure_proper_amp_location() { * should happen infrequently. For admin users, this is kept temporary to allow them * to not be hampered by browser remembering permanent redirects and preventing test. */ - if ( amp_is_paired_endpoint() ) { + if ( amp_has_paired_endpoint() ) { return self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301 ); } - } elseif ( amp_is_paired_endpoint() ) { + } elseif ( amp_has_paired_endpoint() ) { /* * Prevent infinite URL space under /amp/ endpoint. Note that WordPress allows endpoints to have a value, * such as the case of /feed/ where /feed/atom/ is the same as saying ?feed=atom. In this case, we need to @@ -412,7 +412,7 @@ public static function ensure_proper_amp_location() { wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { $current_url = amp_get_current_url(); - $redirect_url = amp_get_paired_endpoint( amp_remove_paired_endpoint( $current_url ) ); + $redirect_url = amp_add_paired_endpoint( amp_remove_paired_endpoint( $current_url ) ); if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php index 087c4b51626..ce96e607327 100644 --- a/includes/embeds/class-amp-core-block-handler.php +++ b/includes/embeds/class-amp-core-block-handler.php @@ -313,7 +313,7 @@ private function process_archives_widgets( Document $dom, $args = [] ) { * * @var DOMElement $option */ - $option->setAttribute( 'value', amp_get_paired_endpoint( $option->getAttribute( 'value' ) ) ); + $option->setAttribute( 'value', amp_add_paired_endpoint( $option->getAttribute( 'value' ) ) ); } } } diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php index 12cbf7ccf5f..d590b9a6337 100644 --- a/includes/options/class-amp-options-manager.php +++ b/includes/options/class-amp-options-manager.php @@ -171,7 +171,7 @@ static function ( $supported ) { && get_stylesheet() === $options[ Option::READER_THEME ] && - ! amp_is_paired_endpoint() + ! amp_has_paired_endpoint() ) { /* * When Reader mode is selected and a Reader theme has been chosen, if the active theme switches to be the diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index 9c910e689e9..cd9885bed0b 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -87,7 +87,7 @@ public function sanitize() { // Record that action was converted to action-xhr. $action_url = add_query_arg( AMP_HTTP::ACTION_XHR_CONVERTED_QUERY_VAR, 1, $action_url ); if ( ! amp_is_canonical() ) { - $action_url = amp_get_paired_endpoint( $action_url ); + $action_url = amp_add_paired_endpoint( $action_url ); } $node->setAttribute( 'action-xhr', $action_url ); // Append success/error handlers if not found. diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 17959e60d87..669aec9a978 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -212,7 +212,7 @@ private function process_element( DOMElement $element, $attribute_name ) { // Only add the AMP query var when requested (in Transitional or Reader mode). if ( ! $excluded && ! empty( $this->args['paired'] ) ) { - $url = amp_get_paired_endpoint( $url ); + $url = amp_add_paired_endpoint( $url ); } $element->setAttribute( $attribute_name, $url ); diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index fc9be09af49..461fcd5b576 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -9,7 +9,7 @@ $url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); -$amp_url = amp_get_paired_endpoint( $url ); +$amp_url = amp_add_paired_endpoint( $url ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 17f8c075867..d98c1ae4665 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -726,7 +726,7 @@ public static function get_url_from_post( $post ) { // Add AMP query var if in transitional mode. if ( ! amp_is_canonical() ) { - $url = amp_get_paired_endpoint( $url ); + $url = amp_add_paired_endpoint( $url ); } // Set URL scheme based on whether HTTPS is current. diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index f43afcefe82..6f0cde16897 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -350,7 +350,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $current_url ); if ( ! amp_is_canonical() ) { - $amp_url = amp_get_paired_endpoint( $amp_url ); + $amp_url = amp_add_paired_endpoint( $amp_url ); } $validate_url = AMP_Validated_URL_Post_Type::get_recheck_url( AMP_Validated_URL_Post_Type::get_invalid_url_post( $amp_url ) ?: $amp_url ); diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index d2eb9117b2d..d50626aaab0 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -88,7 +88,7 @@ public function sanitize_options( $options, $new_options ) { * @return string AMP URL. */ public function get_current_amp_url() { - $url = amp_get_paired_endpoint( amp_get_current_url() ); + $url = amp_add_paired_endpoint( amp_get_current_url() ); $url = remove_query_arg( QueryVar::NOAMP, $url ); return $url; } diff --git a/src/PluginSuppression.php b/src/PluginSuppression.php index 62e73be16ac..ce250b87a02 100644 --- a/src/PluginSuppression.php +++ b/src/PluginSuppression.php @@ -92,7 +92,7 @@ public function is_reader_theme_request() { && ReaderThemes::DEFAULT_READER_THEME !== AMP_Options_Manager::get_option( Option::READER_THEME ) && - amp_is_paired_endpoint() + amp_has_paired_endpoint() ); } diff --git a/src/ReaderThemeLoader.php b/src/ReaderThemeLoader.php index 9dbaa6acc6f..c7e928aa835 100644 --- a/src/ReaderThemeLoader.php +++ b/src/ReaderThemeLoader.php @@ -302,7 +302,7 @@ public function get_active_theme() { */ public function override_theme() { $this->theme_overridden = false; - if ( ! $this->is_enabled() || ! amp_is_paired_endpoint() ) { + if ( ! $this->is_enabled() || ! amp_has_paired_endpoint() ) { return; } diff --git a/tests/php/src/MobileRedirectionTest.php b/tests/php/src/MobileRedirectionTest.php index ad6de550913..65b92dc6b13 100644 --- a/tests/php/src/MobileRedirectionTest.php +++ b/tests/php/src/MobileRedirectionTest.php @@ -119,7 +119,7 @@ public function test_sanitize_options() { public function test_get_current_amp_url() { $this->go_to( add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, '/foo/' ) ); $this->assertEquals( - amp_get_paired_endpoint( home_url( '/foo/' ) ), + amp_add_paired_endpoint( home_url( '/foo/' ) ), $this->instance->get_current_amp_url() ); } @@ -151,7 +151,7 @@ public function test_redirect_on_transitional_and_not_available() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); AMP_Options_Manager::update_option( Option::ALL_TEMPLATES_SUPPORTED, false ); AMP_Options_Manager::update_option( Option::SUPPORTED_TEMPLATES, [ 'is_author' ] ); - $this->go_to( amp_get_paired_endpoint( '/' ) ); + $this->go_to( amp_add_paired_endpoint( '/' ) ); $this->assertFalse( amp_is_canonical() ); $this->assertFalse( amp_is_available() ); $this->instance->redirect(); @@ -185,7 +185,7 @@ public function test_redirect_when_server_side_and_not_applicable() { add_filter( 'amp_mobile_client_side_redirection', '__return_false' ); add_filter( 'amp_pre_is_mobile', '__return_false' ); - $this->go_to( amp_get_paired_endpoint( '/' ) ); + $this->go_to( amp_add_paired_endpoint( '/' ) ); $this->assertFalse( amp_is_request() ); $this->assertFalse( $this->instance->is_mobile_request() ); @@ -226,7 +226,7 @@ static function ( $redirect_url ) use ( &$redirected_url ) { $this->instance->redirect(); $this->assertNotNull( $redirected_url ); $this->assertEquals( - amp_get_paired_endpoint( home_url( '/' ) ), + amp_add_paired_endpoint( home_url( '/' ) ), $redirected_url ); } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index d9d2d9d62fb..5454c2d6297 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -641,14 +641,14 @@ public function get_reader_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - amp_get_paired_endpoint( home_url( '/' ) ), + amp_add_paired_endpoint( home_url( '/' ) ), false, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - amp_get_paired_endpoint( home_url( '/no-existe/' ) ), + amp_add_paired_endpoint( home_url( '/no-existe/' ) ), false, ]; }, @@ -740,14 +740,14 @@ public function get_transitional_mode_amphtml_urls() { 'is_home' => static function () { return [ home_url( '/' ), - amp_get_paired_endpoint( home_url( '/' ) ), + amp_add_paired_endpoint( home_url( '/' ) ), true, ]; }, 'is_404' => static function () { return [ home_url( '/no-existe/' ), - amp_get_paired_endpoint( home_url( '/no-existe/' ) ), + amp_add_paired_endpoint( home_url( '/no-existe/' ) ), true, ]; }, @@ -1054,7 +1054,7 @@ public function test_amp_is_request_before_wp_action_for_reader_mode() { */ public function test_amp_is_request_before_wp_action_for_transitional_mode_with_query_var() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_paired_endpoint( home_url( '/' ) ) ); + $this->go_to( amp_add_paired_endpoint( home_url( '/' ) ) ); global $wp_actions; unset( $wp_actions['wp'] ); $this->assertTrue( AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED ) ); @@ -1818,15 +1818,15 @@ public function test_amp_generate_script_hash() { $this->assertSame( 'sha384-_MAJ0_NC2k8jrjehfi-5LdQasBICZXvp4gOwOx0D3mIStvDCGvZDzcTfXLgMrLL1', amp_generate_script_hash( 'document.body.textContent = \'\';' ) ); } - /** @covers ::amp_get_paired_endpoint() */ - public function test_amp_get_paired_endpoint() { - $this->assertEquals( home_url( '/?amp=1' ), amp_get_paired_endpoint( home_url( '/' ) ) ); - $this->assertEquals( home_url( '/?foo=bar&=1' ), amp_get_paired_endpoint( home_url( '/?foo=bar' ) ) ); - $this->assertEquals( home_url( '/?foo=bar&=1#baz' ), amp_get_paired_endpoint( home_url( '/?foo=bar#baz' ) ) ); + /** @covers ::amp_add_paired_endpoint() */ + public function test_amp_add_paired_endpoint() { + $this->assertEquals( home_url( '/?amp=1' ), amp_add_paired_endpoint( home_url( '/' ) ) ); + $this->assertEquals( home_url( '/?foo=bar&=1' ), amp_add_paired_endpoint( home_url( '/?foo=bar' ) ) ); + $this->assertEquals( home_url( '/?foo=bar&=1#baz' ), amp_add_paired_endpoint( home_url( '/?foo=bar#baz' ) ) ); } /** @return array */ - public function data_amp_is_paired_endpoint() { + public function data_amp_has_paired_endpoint() { return [ 'nothing' => [ '', @@ -1864,13 +1864,13 @@ public function data_amp_is_paired_endpoint() { } /** - * @dataProvider data_amp_is_paired_endpoint - * @covers ::amp_is_paired_endpoint() + * @dataProvider data_amp_has_paired_endpoint + * @covers ::amp_has_paired_endpoint() * * @param string $suffix * @param bool $is_amp */ - public function test_amp_is_paired_endpoint_go_to( $suffix, $is_amp ) { + public function test_amp_has_paired_endpoint_go_to( $suffix, $is_amp ) { add_filter( 'wp_redirect', '__return_empty_string' ); // Prevent ensure_proper_amp_location() from redirecting. global $wp_rewrite; update_option( 'permalink_structure', '/%year%/%monthnum%/%day%/%postname%/' ); @@ -1884,21 +1884,21 @@ public function test_amp_is_paired_endpoint_go_to( $suffix, $is_amp ) { $this->go_to( $url ); $this->assertTrue( is_singular(), 'Expected singular query.' ); $this->assertTrue( amp_is_available(), 'Expected AMP to be available.' ); - $this->assertEquals( $is_amp, amp_is_paired_endpoint() ); + $this->assertEquals( $is_amp, amp_has_paired_endpoint() ); } /** - * @dataProvider data_amp_is_paired_endpoint - * @covers ::amp_is_paired_endpoint() + * @dataProvider data_amp_has_paired_endpoint + * @covers ::amp_has_paired_endpoint() * * @param string $suffix * @param bool $is_amp */ - public function test_amp_is_paired_endpoint_passed( $suffix, $is_amp ) { + public function test_amp_has_paired_endpoint_passed( $suffix, $is_amp ) { $permalink = home_url( '/foo/' ); $this->assertNotContains( '?', $permalink ); $url = $permalink . $suffix; - $this->assertEquals( $is_amp, amp_is_paired_endpoint( $url ) ); + $this->assertEquals( $is_amp, amp_has_paired_endpoint( $url ) ); } /** diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index 2929c85ba26..335982a6715 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -252,7 +252,7 @@ public function test_disable_mobile_redirect_for_excluded_url() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_paired_endpoint( home_url( '/' ) ) ); + $this->go_to( amp_add_paired_endpoint( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); @@ -274,7 +274,7 @@ public function test_disable_mobile_redirect_for_url_with_noamphtml_rel() { AMP_Options_Manager::update_option( Option::MOBILE_REDIRECT, true ); AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); - $this->go_to( amp_get_paired_endpoint( home_url( '/' ) ) ); + $this->go_to( amp_add_paired_endpoint( home_url( '/' ) ) ); $mobile_redirection->redirect(); $link = home_url( '/' ); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index cf52041374f..a5eb3f4d12e 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -324,7 +324,7 @@ function ( $url ) use ( $requested_url, &$redirected ) { return null; } ); - $this->go_to( amp_get_paired_endpoint( $requested_url ) ); + $this->go_to( amp_add_paired_endpoint( $requested_url ) ); $this->assertTrue( $redirected ); } @@ -343,7 +343,7 @@ public function test_ensure_proper_amp_location_canonical() { // URL query param. $_GET[ amp_get_slug() ] = ''; - $_SERVER['REQUEST_URI'] = amp_get_paired_endpoint( '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_add_paired_endpoint( '/foo/bar' ); try { $this->assertTrue( AMP_Theme_Support::ensure_proper_amp_location() ); } catch ( Exception $exception ) { @@ -405,11 +405,11 @@ static function ( $url ) use ( &$redirections ) { $this->go_to( $permalink . 'amp/amp/' ); $this->assertCount( 1, $redirections ); - $this->assertEquals( amp_get_paired_endpoint( $permalink ), end( $redirections ) ); + $this->assertEquals( amp_add_paired_endpoint( $permalink ), end( $redirections ) ); $this->go_to( $permalink . 'amp/foo/' ); $this->assertCount( 2, $redirections ); - $this->assertEquals( amp_get_paired_endpoint( $permalink ), end( $redirections ) ); + $this->assertEquals( amp_add_paired_endpoint( $permalink ), end( $redirections ) ); } /** @@ -431,7 +431,7 @@ static function( $code ) use ( &$redirect_status_code ) { ); // Try AMP URL param. - $_SERVER['REQUEST_URI'] = amp_get_paired_endpoint( '/foo/bar' ); + $_SERVER['REQUEST_URI'] = amp_add_paired_endpoint( '/foo/bar' ); try { $redirect_status_code = null; $this->assertTrue( AMP_Theme_Support::redirect_non_amp_url( 302 ) ); diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 1c3c46673e1..8b4a5b5c43b 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -327,7 +327,7 @@ public function test_get_url_from_post() { $this->assertNotInstanceOf( 'WP_Error', $invalid_post_id ); $this->assertEquals( - amp_get_paired_endpoint( get_permalink( $post ) ), + amp_add_paired_endpoint( get_permalink( $post ) ), AMP_Validated_URL_Post_Type::get_url_from_post( $invalid_post_id ) );