From 6717e4f78b38a9faafa4c4ae5542bd4c6faf52d1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 15 Nov 2019 23:23:33 -0800 Subject: [PATCH 01/27] Pass entire Reader mode templates through post-processor Eliminate majority of Reader mode template's unique rendering path. --- amp.php | 8 ++-- includes/amp-helper-functions.php | 6 ++- includes/amp-post-template-functions.php | 6 ++- includes/class-amp-theme-support.php | 37 ++++++++++++--- includes/templates/class-amp-content.php | 2 + .../templates/class-amp-post-template.php | 39 +++------------ includes/templates/reader-template-loader.php | 24 ++++++++++ tests/php/test-amp-analytics-options.php | 47 +++---------------- tests/php/test-amp-render-post.php | 7 +++ tests/php/test-class-amp-theme-support.php | 3 -- 10 files changed, 91 insertions(+), 88 deletions(-) create mode 100644 includes/templates/reader-template-loader.php diff --git a/amp.php b/amp.php index e8a26429f5a..4332c776c45 100644 --- a/amp.php +++ b/amp.php @@ -379,9 +379,6 @@ function amp_init() { add_action( 'wp_loaded', 'amp_editor_core_blocks' ); add_filter( 'request', 'amp_force_query_var_value' ); - // Add actions for reader mode templates. - add_action( 'wp', 'amp_maybe_add_actions' ); - // Redirect the old url of amp page to the updated url. add_filter( 'old_slug_redirect_url', 'amp_redirect_old_slug_to_new_url' ); } @@ -449,6 +446,7 @@ function amp_force_query_var_value( $query_vars ) { * @return void */ function amp_maybe_add_actions() { + _deprecated_function( __FUNCTION__, '1.5' ); // Short-circuit when theme supports AMP, as everything is handled by AMP_Theme_Support. if ( current_theme_supports( AMP_Theme_Support::SLUG ) ) { @@ -639,6 +637,7 @@ function amp_add_post_template_actions() { * @deprecated This function is not used when 'amp' theme support is added. */ function amp_prepare_render() { + _deprecated_function( __FUNCTION__, '1.5' ); add_action( 'template_redirect', 'amp_render', 11 ); } @@ -649,6 +648,8 @@ function amp_prepare_render() { * @deprecated This function is not used when 'amp' theme support is added. */ function amp_render() { + _deprecated_function( __FUNCTION__, '1.5' ); + // Note that queried object is used instead of the ID so that the_preview for the queried post can apply. $post = get_queried_object(); if ( $post instanceof WP_Post ) { @@ -667,6 +668,7 @@ function amp_render() { * @global WP_Query $wp_query */ function amp_render_post( $post ) { + _deprecated_function( __FUNCTION__, '1.5' ); global $wp_query; if ( ! ( $post instanceof WP_Post ) ) { diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 91394bfc453..ebd8b912a5c 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1264,7 +1264,11 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { } // Show nothing if there are rejected validation errors for this URL. - if ( ! is_amp_endpoint() && count( AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( amp_get_current_url(), [ 'ignore_accepted' => true ] ) ) > 0 ) { + if ( + ! is_amp_endpoint() && + AMP_Theme_Support::READER_MODE_SLUG !== AMP_Theme_Support::get_support_mode() && + count( AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( amp_get_current_url(), [ 'ignore_accepted' => true ] ) ) > 0 + ) { return; } diff --git a/includes/amp-post-template-functions.php b/includes/amp-post-template-functions.php index 1f152616ec6..94a03cd955f 100644 --- a/includes/amp-post-template-functions.php +++ b/includes/amp-post-template-functions.php @@ -11,9 +11,7 @@ function amp_post_template_init_hooks() { add_action( 'amp_post_template_head', 'amp_post_template_add_title' ); add_action( 'amp_post_template_head', 'amp_post_template_add_canonical' ); - add_action( 'amp_post_template_head', 'amp_post_template_add_scripts' ); add_action( 'amp_post_template_head', 'amp_post_template_add_fonts' ); - add_action( 'amp_post_template_head', 'amp_post_template_add_boilerplate_css' ); add_action( 'amp_post_template_head', 'amp_print_schemaorg_metadata' ); add_action( 'amp_post_template_head', 'amp_add_generator_metadata' ); add_action( 'amp_post_template_head', 'wp_generator' ); @@ -47,11 +45,13 @@ function amp_post_template_add_canonical( $amp_template ) { /** * Print scripts. * + * @deprecated * @see amp_register_default_scripts() * @see amp_filter_script_loader_tag() * @param AMP_Post_Template $amp_template Template. */ function amp_post_template_add_scripts( $amp_template ) { + _deprecated_function( __FUNCTION__, '1.5' ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped echo amp_render_scripts( array_merge( @@ -79,10 +79,12 @@ function amp_post_template_add_fonts( $amp_template ) { /** * Print boilerplate CSS. * + * @deprecated * @since 0.3 * @see amp_get_boilerplate_code() */ function amp_post_template_add_boilerplate_css() { + _deprecated_function( __FUNCTION__, '1.5' ); echo amp_get_boilerplate_code(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 11b7d9a3361..090b99acef9 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -179,9 +179,11 @@ public static function init() { self::$init_start_time = microtime( true ); - if ( AMP_Options_Manager::is_website_experience_enabled() && current_theme_supports( self::SLUG ) ) { - // Ensure extra theme support for core themes is in place. - AMP_Core_Theme_Sanitizer::extend_theme_support(); + if ( AMP_Options_Manager::is_website_experience_enabled() ) { + if ( self::READER_MODE_SLUG !== self::get_support_mode() ) { + // Ensure extra theme support for core themes is in place. + AMP_Core_Theme_Sanitizer::extend_theme_support(); + } require_once AMP__DIR__ . '/includes/amp-post-template-functions.php'; @@ -417,15 +419,26 @@ public static function finish_init() { self::ensure_proper_amp_location(); - $theme_support = self::get_theme_support_args(); + $is_reader_mode = self::READER_MODE_SLUG === self::get_support_mode(); + $theme_support = self::get_theme_support_args(); if ( ! empty( $theme_support['template_dir'] ) ) { self::add_amp_template_filters(); + } elseif ( $is_reader_mode ) { + add_action( + 'template_include', + function() { + return __DIR__ . '/templates/reader-template-loader.php'; + }, + PHP_INT_MAX + ); } self::add_hooks(); self::$sanitizer_classes = amp_get_content_sanitizers(); - self::$sanitizer_classes = AMP_Validation_Manager::filter_sanitizer_args( self::$sanitizer_classes ); - self::$embed_handlers = self::register_content_embed_handlers(); + if ( ! $is_reader_mode ) { + self::$sanitizer_classes = AMP_Validation_Manager::filter_sanitizer_args( self::$sanitizer_classes ); + } + self::$embed_handlers = self::register_content_embed_handlers(); self::$sanitizer_classes['AMP_Embed_Sanitizer']['embed_handlers'] = self::$embed_handlers; foreach ( self::$sanitizer_classes as $sanitizer_class => $args ) { @@ -458,6 +471,18 @@ public static function ensure_proper_amp_location( $exit = true ) { if ( $has_query_var || $has_url_param ) { return self::redirect_non_amp_url( current_user_can( 'manage_options' ) ? 302 : 301, $exit ); } + } elseif ( self::READER_MODE_SLUG === self::get_support_mode() && is_singular() ) { + // Prevent infinite URL space under /amp/ endpoint. + global $wp; + $path_args = []; + wp_parse_str( $wp->matched_query, $path_args ); + if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { + wp_safe_redirect( amp_get_permalink( get_queried_object_id() ), 301 ); + if ( $exit ) { + exit; + } + return true; + } } else { /* * When in AMP transitional mode *with* theme support, then the proper AMP URL has the 'amp' URL param diff --git a/includes/templates/class-amp-content.php b/includes/templates/class-amp-content.php index 70d3f30df3d..1d76118198f 100644 --- a/includes/templates/class-amp-content.php +++ b/includes/templates/class-amp-content.php @@ -7,6 +7,8 @@ /** * Class AMP_Content + * + * @deprecated */ class AMP_Content { diff --git a/includes/templates/class-amp-post-template.php b/includes/templates/class-amp-post-template.php index 28e33c5e655..9a8b6c85d2f 100644 --- a/includes/templates/class-amp-post-template.php +++ b/includes/templates/class-amp-post-template.php @@ -289,7 +289,7 @@ private function build_post_data() { } /** - * Buuild post comments data. + * Build post comments data. */ private function build_post_comments_data() { if ( ! post_type_supports( $this->post->post_type, 'comments' ) ) { @@ -319,20 +319,14 @@ private function build_post_comments_data() { /** * Build post content. + * + * @see the_content() */ private function build_post_content() { - $amp_content = new AMP_Content( - post_password_required( $this->post ) ? get_the_password_form( $this->post ) : $this->post->post_content, - amp_get_content_embed_handlers( $this->post ), - amp_get_content_sanitizers( $this->post ), - [ - 'content_max_width' => $this->get( 'content_max_width' ), - ] - ); + /** This filter is documented in wp-includes/post-template.php */ + $content = apply_filters( 'the_content', get_the_content() ); - $this->add_data_by_key( 'post_amp_content', $amp_content->get_amp_content() ); - $this->merge_data_for_key( 'amp_component_scripts', $amp_content->get_amp_scripts() ); - $this->add_data_by_key( 'post_amp_stylesheets', $amp_content->get_amp_stylesheets() ); + $this->add_data_by_key( 'post_amp_content', $content ); } /** @@ -362,32 +356,13 @@ private function build_post_featured_image() { return; } - $dom = AMP_DOM_Utils::get_dom_from_content( $featured_html ); - $assets = AMP_Content_Sanitizer::sanitize_document( - $dom, - amp_get_content_sanitizers( $this->post ), - [ - 'content_max_width' => $this->get( 'content_max_width' ), - ] - ); - - $sanitized_html = AMP_DOM_Utils::get_content_from_dom( $dom ); - $this->add_data_by_key( 'featured_image', [ - 'amp_html' => $sanitized_html, + 'amp_html' => $featured_image, 'caption' => $featured_image->post_excerpt, ] ); - - if ( $assets['scripts'] ) { - $this->merge_data_for_key( 'amp_component_scripts', $assets['scripts'] ); - } - - if ( $assets['stylesheets'] ) { - $this->merge_data_for_key( 'post_amp_stylesheets', $assets['stylesheets'] ); - } } /** diff --git a/includes/templates/reader-template-loader.php b/includes/templates/reader-template-loader.php new file mode 100644 index 00000000000..7a948e23412 --- /dev/null +++ b/includes/templates/reader-template-loader.php @@ -0,0 +1,24 @@ +load(); diff --git a/tests/php/test-amp-analytics-options.php b/tests/php/test-amp-analytics-options.php index 8e0ba74eecb..61e19f184ad 100644 --- a/tests/php/test-amp-analytics-options.php +++ b/tests/php/test-amp-analytics-options.php @@ -122,44 +122,6 @@ public function test_two_options_inserted() { $this->assertCount( 2, $options ); } - /** - * Test that the analytics JS is added to the page - */ - public function test_analytics_js_added() { - - /* Insert analytics option */ - $this->insert_one_option( - $this->vendor, - $this->config_one - ); - - $amp_rendered = $this->render_post(); - - $libxml_previous_state = libxml_use_internal_errors( true ); - - // Create a new DOM document - $dom = new DOMDocument(); - // Load the rendered page into it - $dom->loadHTML( $amp_rendered ); - - $head = $dom->getElementsByTagName( 'head' )->item( 0 ); - - $scripts = $head->getElementsByTagName( 'script' ); - $analytics_js_found = false; - foreach ( $scripts as $script ) { - if ( 'amp-analytics' === $script->getAttribute( 'custom-element' ) ) { - $analytics_js_found = true; - break; - } - } - - libxml_clear_errors(); - libxml_use_internal_errors( $libxml_previous_state ); - - $this->assertTrue( $analytics_js_found ); - - } - /** * Test that exactly one analytics component are added to the page */ @@ -171,8 +133,9 @@ public function test_one_analytics_component_added() { $this->config_one ); - // Render AMP post - $amp_rendered = $this->render_post(); + ob_start(); + amp_print_analytics( [] ); + $amp_rendered = ob_get_clean(); $libxml_previous_state = libxml_use_internal_errors( true ); @@ -203,7 +166,9 @@ public function test_two_analytics_components_added() { $this->config_two ); - $amp_rendered = $this->render_post(); + ob_start(); + amp_print_analytics( [] ); + $amp_rendered = ob_get_clean(); $libxml_previous_state = libxml_use_internal_errors( true ); diff --git a/tests/php/test-amp-render-post.php b/tests/php/test-amp-render-post.php index f15e16c6cf8..aa87b69608d 100644 --- a/tests/php/test-amp-render-post.php +++ b/tests/php/test-amp-render-post.php @@ -1,6 +1,9 @@ assertEquals( 0, did_action( 'pre_amp_render_post' ), 'pre_amp_render_post action fire when it should not have.' ); } + /** + * @expectedDeprecated amp_render_post + */ public function test__valid_post() { $user_id = self::factory()->user->create(); $post_id = self::factory()->post->create( [ 'post_author' => $user_id ] ); @@ -30,6 +36,7 @@ public function test__valid_post() { * Test is_amp_endpoint. * * @covers ::is_amp_endpoint() + * @expectedDeprecated amp_render_post */ public function test__is_amp_endpoint() { $user_id = self::factory()->user->create(); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index fb253cd5424..2dfd557aec0 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -79,9 +79,6 @@ public function tearDown() { public function test_init() { $_REQUEST['__amp_source_origin'] = 'foo'; $_GET['__amp_source_origin'] = 'foo'; - AMP_Theme_Support::init(); - $this->assertFalse( has_action( 'widgets_init', [ self::TESTED_CLASS, 'register_widgets' ] ) ); - $this->assertFalse( has_action( 'wp', [ self::TESTED_CLASS, 'finish_init' ] ) ); add_theme_support( AMP_Theme_Support::SLUG ); AMP_Theme_Support::init(); From 0c34778a3157db638a7e314d32ad5922afe501c6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 16 Nov 2019 08:41:28 -0800 Subject: [PATCH 02/27] Fix tests in WP<5.2 --- includes/templates/class-amp-post-template.php | 2 +- tests/php/test-amp-render-post.php | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/includes/templates/class-amp-post-template.php b/includes/templates/class-amp-post-template.php index 9a8b6c85d2f..e01b0f6994a 100644 --- a/includes/templates/class-amp-post-template.php +++ b/includes/templates/class-amp-post-template.php @@ -324,7 +324,7 @@ private function build_post_comments_data() { */ private function build_post_content() { /** This filter is documented in wp-includes/post-template.php */ - $content = apply_filters( 'the_content', get_the_content() ); + $content = apply_filters( 'the_content', get_the_content( null, false, $this->post ) ); $this->add_data_by_key( 'post_amp_content', $content ); } diff --git a/tests/php/test-amp-render-post.php b/tests/php/test-amp-render-post.php index aa87b69608d..7637f3adeb6 100644 --- a/tests/php/test-amp-render-post.php +++ b/tests/php/test-amp-render-post.php @@ -19,6 +19,9 @@ public function test__valid_post() { $user_id = self::factory()->user->create(); $post_id = self::factory()->post->create( [ 'post_author' => $user_id ] ); + // Set global for WP<5.2 where get_the_content() doesn't take the $post parameter. + $GLOBALS['post'] = get_post( $post_id ); + $output = get_echo( 'amp_render_post', [ $post_id ] ); $this->assertContains( ' Date: Sat, 16 Nov 2019 09:12:19 -0800 Subject: [PATCH 03/27] Transition away from AMP_Post_Template to traditional template tags --- .../templates/class-amp-post-template.php | 70 ++++++++----------- templates/featured-image.php | 6 ++ templates/footer.php | 2 +- templates/header-bar.php | 4 +- templates/html-end.php | 12 +++- templates/html-start.php | 38 +++++++++- templates/meta-comments-link.php | 8 +-- templates/page.php | 4 +- templates/single.php | 4 +- 9 files changed, 92 insertions(+), 56 deletions(-) diff --git a/includes/templates/class-amp-post-template.php b/includes/templates/class-amp-post-template.php index e01b0f6994a..14f2ba11fba 100644 --- a/includes/templates/class-amp-post-template.php +++ b/includes/templates/class-amp-post-template.php @@ -110,36 +110,19 @@ public function __construct( $post ) { $this->data = [ 'content_max_width' => $content_max_width, - - 'document_title' => function_exists( 'wp_get_document_title' ) ? wp_get_document_title() : wp_title( '', false ), // Back-compat with 4.3. - 'canonical_url' => get_permalink( $this->ID ), - 'home_url' => home_url( '/' ), - 'blog_name' => get_bloginfo( 'name' ), - 'html_tag_attributes' => [], 'body_class' => '', - - 'site_icon_url' => apply_filters( 'amp_site_icon_url', function_exists( 'get_site_icon_url' ) ? get_site_icon_url( self::SITE_ICON_SIZE ) : '' ), 'placeholder_image_url' => amp_get_asset_url( 'images/placeholder-icon.png' ), - 'featured_image' => false, 'comments_link_url' => false, 'comments_link_text' => false, - - 'amp_runtime_script' => 'https://cdn.ampproject.org/v0.js', - 'amp_component_scripts' => [], - 'customizer_settings' => [], - 'font_urls' => [], - 'post_amp_stylesheets' => [], 'post_amp_styles' => [], // Deprecated. - 'amp_analytics' => amp_add_custom_analytics(), ]; - $this->build_post_content(); $this->build_post_data(); $this->build_customizer_settings(); $this->build_html_tag_attributes(); @@ -148,6 +131,7 @@ public function __construct( $post ) { * Filters AMP template data. * * @since 0.2 + * @deprecated * * @param array $data Template data. * @param WP_Post $post Post. @@ -164,6 +148,35 @@ public function __construct( $post ) { * @return mixed Value. */ public function get( $property, $default = null ) { + switch ( $property ) { + case 'amp_post_content': + /** This filter is documented in wp-includes/post-template.php */ + return apply_filters( 'the_content', get_the_content( null, false, $this->post ) ); + case 'post_title': + return get_the_title( $this->post ); + case 'document_title': + return wp_get_document_title(); + case 'canonical_url': + return get_permalink( $this->ID ); + case 'home_url': + return home_url( '/' ); + case 'blog_name': + return get_bloginfo( 'name', 'display' ); + case 'post_author': + return get_userdata( $this->post->post_author ); + case 'post_publish_timestamp': + return get_the_date( 'U', $this->ID ); + case 'post_modified_timestamp': + return get_post_modified_time( 'U', false, $this->post ); + case 'post': + return $this->post; + case 'post_id': + return $this->post->ID; + case 'site_icon_url': + /** This filter is documented in includes/amp-helper-functions.php */ + return apply_filters( 'amp_site_icon_url', function_exists( 'get_site_icon_url' ) ? get_site_icon_url( self::SITE_ICON_SIZE ) : '' ); + } + if ( isset( $this->data[ $property ] ) ) { return $this->data[ $property ]; } @@ -260,18 +273,7 @@ private function merge_data_for_key( $key, $value ) { * @since 0.2 */ private function build_post_data() { - $post_title = get_the_title( $this->ID ); - $post_publish_timestamp = get_the_date( 'U', $this->ID ); - $post_modified_timestamp = get_post_modified_time( 'U', false, $this->post ); - $post_author = get_userdata( $this->post->post_author ); - $data = [ - 'post' => $this->post, - 'post_id' => $this->ID, - 'post_title' => $post_title, - 'post_publish_timestamp' => $post_publish_timestamp, - 'post_modified_timestamp' => $post_modified_timestamp, - 'post_author' => $post_author, 'post_canonical_link_url' => '', 'post_canonical_link_text' => '', ]; @@ -317,18 +319,6 @@ private function build_post_comments_data() { ); } - /** - * Build post content. - * - * @see the_content() - */ - private function build_post_content() { - /** This filter is documented in wp-includes/post-template.php */ - $content = apply_filters( 'the_content', get_the_content( null, false, $this->post ) ); - - $this->add_data_by_key( 'post_amp_content', $content ); - } - /** * Build post featured image. */ diff --git a/templates/featured-image.php b/templates/featured-image.php index 3f0c2133635..35dcb4fe5e0 100644 --- a/templates/featured-image.php +++ b/templates/featured-image.php @@ -12,6 +12,12 @@ * @package AMP */ +/** + * Context. + * + * @var AMP_Post_Template $this + */ + $featured_image = $this->get( 'featured_image' ); if ( empty( $featured_image ) ) { diff --git a/templates/footer.php b/templates/footer.php index 3a58b8ac683..6839dc838d9 100644 --- a/templates/footer.php +++ b/templates/footer.php @@ -20,7 +20,7 @@ ?>
-

get( 'blog_name' ) ) ); ?>

+

diff --git a/templates/header-bar.php b/templates/header-bar.php index 58db6e509d6..5c1f42f6967 100644 --- a/templates/header-bar.php +++ b/templates/header-bar.php @@ -21,13 +21,13 @@ ?>
- + get( 'site_icon_url' ); ?> - get( 'blog_name' ) ) ); ?> + diff --git a/templates/html-end.php b/templates/html-end.php index 27b8b141275..3f93c8e2b0c 100644 --- a/templates/html-end.php +++ b/templates/html-end.php @@ -19,7 +19,17 @@ */ ?> - +. + * + * @since 0.2 + * @see wp_footer() + * + * @param AMP_Post_Template $this + */ +do_action( 'amp_post_template_footer', $this ); +?> diff --git a/templates/html-start.php b/templates/html-start.php index 1bd1b65ae7f..c1dbabc9185 100644 --- a/templates/html-start.php +++ b/templates/html-start.php @@ -23,12 +23,44 @@ - + in Reader mode templates. + * + * @since 0.2 + * + * @param AMP_Post_Template $this + */ + do_action( 'amp_post_template_head', $this ); + ?> - +. + * + * @since 1.2.1 + * @see wp_body_open() + * + * @param AMP_Post_Template $this + */ +do_action( 'amp_post_template_body_open', $this ); diff --git a/templates/meta-comments-link.php b/templates/meta-comments-link.php index b2179445ab6..18fa961ded7 100644 --- a/templates/meta-comments-link.php +++ b/templates/meta-comments-link.php @@ -18,13 +18,11 @@ * @var AMP_Post_Template $this */ -$comments_link_url = $this->get( 'comments_link_url' ); ?> - - get( 'comments_link_text' ); ?> +
- - + + get( 'comments_link_text' ) ); ?>
diff --git a/templates/page.php b/templates/page.php index e631fab9a1b..38f12f01fe7 100644 --- a/templates/page.php +++ b/templates/page.php @@ -25,13 +25,13 @@
-

get( 'post_title' ) ); ?>

+

load_parts( [ 'featured-image' ] ); ?>
- get( 'post_amp_content' ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> +
diff --git a/templates/single.php b/templates/single.php index a56aff770e4..1651da97851 100644 --- a/templates/single.php +++ b/templates/single.php @@ -25,14 +25,14 @@
-

get( 'post_title' ) ); ?>

+

load_parts( apply_filters( 'amp_post_article_header_meta', [ 'meta-author', 'meta-time' ] ) ); ?>
load_parts( [ 'featured-image' ] ); ?>
- get( 'post_amp_content' ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> +