From 43c24ac5595e37c754701b490e7c79a6a0e8c3a9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 26 Jun 2018 12:21:32 -0700 Subject: [PATCH] Introduce amp_render_scripts() to print AMP component scripts and nothing else --- includes/amp-helper-functions.php | 47 ++++++++++++++++++++++++++ includes/amp-post-template-actions.php | 23 ++++--------- includes/class-amp-theme-support.php | 28 ++------------- tests/test-amp-helper-functions.php | 11 ++++++ tests/test-class-amp-theme-support.php | 3 +- 5 files changed, 69 insertions(+), 43 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index b357fcfc3fd..0d655e2bd5d 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -270,6 +270,53 @@ function amp_register_default_scripts( $wp_scripts ) { } } +/** + * Generate HTML for AMP scripts that have not yet been printed. + * + * This is adapted from `wp_scripts()->do_items()`, but it runs only the bare minimum required to output + * the missing scripts, without allowing other filters to apply which may cause an invalid AMP response. + * The HTML for the scripts is returned instead of being printed. + * + * @since 0.7.2 + * @see WP_Scripts::do_items() + * @see AMP_Base_Embed_Handler::get_scripts() + * @see AMP_Base_Sanitizer::get_scripts() + * + * @param array $scripts Script handles mapped to URLs or true. + * @return string HTML for scripts tags that have not yet been done. + */ +function amp_render_scripts( $scripts ) { + $script_tags = ''; + + /* + * Make sure the src is up to date. This allows for embed handlers to override the + * default extension version by defining a different URL. + */ + foreach ( $scripts as $handle => $src ) { + if ( is_string( $src ) && wp_script_is( $handle, 'registered' ) ) { + wp_scripts()->registered[ $handle ]->src = $src; + } + } + + foreach ( array_diff( array_keys( $scripts ), wp_scripts()->done ) as $handle ) { + if ( ! wp_script_is( $handle, 'registered' ) ) { + continue; + } + + $script_dep = wp_scripts()->registered[ $handle ]; + $script_tags .= amp_filter_script_loader_tag( + sprintf( + "\n", // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript + esc_url( $script_dep->src ) + ), + $handle + ); + + wp_scripts()->done[] = $handle; + } + return $script_tags; +} + /** * Add AMP script attributes to enqueued scripts. * diff --git a/includes/amp-post-template-actions.php b/includes/amp-post-template-actions.php index b956faaf6c6..b4cf53b97eb 100644 --- a/includes/amp-post-template-actions.php +++ b/includes/amp-post-template-actions.php @@ -51,22 +51,13 @@ function amp_post_template_add_canonical( $amp_template ) { * @param AMP_Post_Template $amp_template Template. */ function amp_post_template_add_scripts( $amp_template ) { - - // Just in case the runtime has been overridden by amp_post_template_data filter. - wp_scripts()->registered['amp-runtime']->src = $amp_template->get( 'amp_runtime_script' ); - - // Make sure any filtered extension script URLs get updated in registered scripts before printing. - $scripts = $amp_template->get( 'amp_component_scripts', array() ); - foreach ( $scripts as $handle => $value ) { - if ( is_string( $value ) && wp_script_is( $handle, 'registered' ) ) { - wp_scripts()->registered[ $handle ]->src = $value; - } - } - - wp_print_scripts( array_merge( - array( 'amp-runtime' ), - array_keys( $scripts ) - ) ); + echo amp_render_scripts( array_merge( + array( + // Just in case the runtime has been overridden by amp_post_template_data filter. + 'amp-runtime' => $amp_template->get( 'amp_runtime_script' ), + ), + $amp_template->get( 'amp_component_scripts', array() ) + ) ); // WPCS: xss ok. } /** diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 60deac4ad41..044fcfe0613 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1133,32 +1133,8 @@ public static function prepare_response( $response, $args = array() ) { ); } - // Allow for embed handlers to override the default extension version by defining a different URL. - foreach ( $amp_scripts as $handle => $value ) { - if ( is_string( $value ) && wp_script_is( $handle, 'registered' ) ) { - wp_scripts()->registered[ $handle ]->src = $value; - } - } - - /* - * Inject additional AMP component scripts which have been discovered by the sanitizers into the head. - * This is adapted from wp_scripts()->do_items(), but it runs only the bare minimum required to output - * the missing scripts, without allowing other filters to apply which may cause an invalid AMP response. - */ - $script_tags = ''; - foreach ( array_diff( array_keys( $amp_scripts ), wp_scripts()->done ) as $handle ) { - if ( ! wp_script_is( $handle, 'registered' ) ) { - continue; - } - $script_dep = wp_scripts()->registered[ $handle ]; - $script_tags .= amp_filter_script_loader_tag( - sprintf( - "\n", // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript - esc_url( $script_dep->src ) - ), - $handle - ); - } + // Inject additional AMP component scripts which have been discovered by the sanitizers into the head. + $script_tags = amp_render_scripts( $amp_scripts ); if ( ! empty( $script_tags ) ) { $response = preg_replace( '#(?=)#', diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php index 327e021ff6c..b2ea76215dc 100644 --- a/tests/test-amp-helper-functions.php +++ b/tests/test-amp-helper-functions.php @@ -183,6 +183,7 @@ public function capture_filter_call( $value ) { * * @covers \amp_register_default_scripts() * @covers \amp_filter_script_loader_tag() + * @covers \amp_render_scripts() * @global WP_Scripts $wp_scripts */ public function test_script_registering() { @@ -212,6 +213,16 @@ public function test_script_registering() { $this->assertContains( '', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript $this->assertContains( '', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript + // Try rendering via amp_render_scripts() instead of amp_render_scripts(), which is how component scripts get added normally. + $output = amp_render_scripts( array( + 'amp-mathml' => true, // But already printed above. + 'amp-carousel' => 'https://cdn.ampproject.org/v0/amp-mustache-2.0.js', + 'amp-accordion' => true, + ) ); + $this->assertNotContains( 'amp-mathml', $output, 'The amp-mathml component was already printed above.' ); + $this->assertContains( '', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript + $this->assertContains( '', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript + // Try some experimental component to ensure expected script attributes are added. wp_register_script( 'amp-foo', 'https://cdn.ampproject.org/v0/amp-foo-0.1.js', array( 'amp-runtime' ), null ); ob_start(); diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index bcf23f239b4..83f47fdd149 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -980,6 +980,7 @@ public function test_filter_customize_partial_render() { * @global WP_Widget_Factory $wp_widget_factory * @global WP_Scripts $wp_scripts * @covers AMP_Theme_Support::prepare_response() + * @covers \amp_render_scripts() */ public function test_prepare_response() { global $wp_widget_factory, $wp_scripts, $wp_styles; @@ -1049,7 +1050,7 @@ public function test_prepare_response() { $this->assertContains( '', $sanitized_html ); $this->assertContains( '', $sanitized_html ); $this->assertContains( '#s', $sanitized_html ); $this->assertContains( '', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript $this->assertContains( '', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript $this->assertContains( '', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript