diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 4e053fbd1e2..4c2f2ba6aef 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -181,10 +181,8 @@ function amp_add_amphtml_link() { if ( AMP_Theme_Support::is_paired_available() ) { $amp_url = add_query_arg( amp_get_slug(), '', $current_url ); } - } elseif ( is_singular() ) { + } elseif ( is_singular() && post_supports_amp( get_post( get_queried_object_id() ) ) ) { $amp_url = amp_get_permalink( get_queried_object_id() ); - } else { - $amp_url = add_query_arg( amp_get_slug(), '', $current_url ); } if ( ! $amp_url ) { diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index c67be4cccf8..f1eadd8b448 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -410,20 +410,32 @@ public static function finish_init() { add_filter( 'template_include', [ __CLASS__, 'serve_paired_browsing_experience' ] ); } + $has_query_var = ( + isset( $_GET[ amp_get_slug() ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + || + false !== get_query_var( amp_get_slug(), false ) + ); $is_reader_mode = self::READER_MODE_SLUG === self::get_support_mode(); - if ( $is_reader_mode && ! is_singular() && false !== get_query_var( amp_get_slug(), false ) ) { + if ( + $is_reader_mode + && + $has_query_var + && + ( ! is_singular() || ! post_supports_amp( get_post( get_queried_object_id() ) ) ) + ) { // Reader mode only supports the singular template (for now) so redirect non-singular queries in reader mode to non-AMP version. + // Also ensure redirecting to non-AMP version when accessing a post which does not support AMP. // A temporary redirect is used for admin users to allow them to see changes between reader mode and transitional modes. wp_safe_redirect( amp_remove_endpoint( amp_get_current_url() ), current_user_can( 'manage_options' ) ? 302 : 301 ); return; } elseif ( ! is_amp_endpoint() ) { /* - * Redirect to AMP-less variable if AMP is not available for this URL and yet the query var is present. + * Redirect to AMP-less URL if AMP is not available for this URL and yet the query var is present. * Temporary redirect is used for admin users because implied transitional mode and template support can be * 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 ( isset( $_GET[ amp_get_slug() ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended + if ( $has_query_var ) { self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301, true ); } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index a01c25e9fb5..5aed421f66e 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -273,44 +273,87 @@ public function test_amp_add_frontend_actions() { * * @return array */ - public function get_amphtml_urls() { - $post_id = self::factory()->post->create(); - return [ - 'is_home' => [ - home_url( '/' ), - add_query_arg( amp_get_slug(), '', home_url( '/' ) ), - ], - 'is_404' => [ - home_url( '/no-existe/' ), - add_query_arg( amp_get_slug(), '', home_url( '/no-existe/' ) ), - ], - 'is_post' => [ - get_permalink( $post_id ), - amp_get_permalink( $post_id ), - ], + public function get_reader_mode_amphtml_urls() { + $providers = [ + 'is_home' => static function () { + return [ + home_url( '/' ), + add_query_arg( amp_get_slug(), '', home_url( '/' ) ), + false, + ]; + }, + 'is_404' => static function () { + return [ + home_url( '/no-existe/' ), + add_query_arg( amp_get_slug(), '', home_url( '/no-existe/' ) ), + false, + ]; + }, + 'is_post' => function() { + $post_id = $this->factory()->post->create(); + return [ + get_permalink( $post_id ), + amp_get_permalink( $post_id ), + true, + ]; + }, + 'is_skipped_post' => function() { + $skipped_post_id = self::factory()->post->create(); + add_filter( + 'amp_skip_post', + static function ( $skip, $current_post ) use ( $skipped_post_id ) { + if ( $current_post === $skipped_post_id ) { + $skip = true; + } + return $skip; + }, + 10, + 2 + ); + return [ + get_permalink( $skipped_post_id ), + amp_get_permalink( $skipped_post_id ), + false, + ]; + }, ]; + return array_map( + function( $provider ) { + return [ $provider ]; + }, + $providers + ); } /** * Adding link when theme support is not present. * - * @dataProvider get_amphtml_urls + * @dataProvider get_reader_mode_amphtml_urls * @covers ::amp_add_amphtml_link() - * @param string $canonical_url Canonical URL. - * @param string $amphtml_url The amphtml URL. + * + * @param callable $data_provider Provider. */ - public function test_amp_add_amphtml_link_reader_mode( $canonical_url, $amphtml_url ) { + public function test_amp_add_amphtml_link_reader_mode( $data_provider ) { + list( $canonical_url, $amphtml_url, $available ) = $data_provider(); $this->assertFalse( current_theme_supports( AMP_Theme_Support::SLUG ) ); $this->assertFalse( amp_is_canonical() ); $get_amp_html_link = static function() { return get_echo( 'amp_add_amphtml_link' ); }; - $assert_amphtml_link_present = function() use ( $amphtml_url, $get_amp_html_link ) { - $this->assertEquals( - sprintf( '', esc_url( $amphtml_url ) ), - $get_amp_html_link() - ); + $assert_amphtml_link_present = function() use ( $amphtml_url, $get_amp_html_link, $available ) { + if ( $available ) { + $this->assertEquals( + sprintf( '', esc_url( $amphtml_url ) ), + $get_amp_html_link() + ); + } else { + $this->assertNotEquals( + sprintf( '', esc_url( $amphtml_url ) ), + $get_amp_html_link() + ); + $this->assertStringStartsWith( '