Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure only remaining AMP scripts are printed when preparing response #1097

Merged
merged 2 commits into from
Apr 26, 2018
Merged

Ensure only remaining AMP scripts are printed when preparing response #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
846123a
Ensure only remaining AMP scripts are printed when preparing response
westonruter Apr 26, 2018
97455ce
Remove all hooks related to script printing when outputting remaining…
westonruter Apr 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions includes/class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,9 +1128,18 @@ public static function prepare_response( $response, $args = array() ) {
}
}

// Print all scripts, some of which may have already been printed and inject into head.
/*
* Print additional AMP component scripts which have been discovered by the sanitizers, and inject into the head.
* Before printing the AMP scripts, make sure that no plugins will be manipulating the output to be invalid AMP
* since at this point the sanitizers have completed and won't check what is output here.
*/
ob_start();
wp_print_scripts( array_keys( $amp_scripts ) );
$possible_hooks = array( 'wp_print_scripts', 'print_scripts_array', 'script_loader_src', 'clean_url', 'script_loader_tag', 'attribute_escape' );
foreach ( $possible_hooks as $possible_hook ) {
remove_all_filters( $possible_hook ); // An action and a filter are the same thing.
}
add_filter( 'script_loader_tag', 'amp_filter_script_loader_tag', PHP_INT_MAX, 2 ); // Make sure this one required filter is present.
wp_scripts()->do_items( array_keys( $amp_scripts ) );
$script_tags = ob_get_clean();
if ( ! empty( $script_tags ) ) {
$response = preg_replace(
Expand Down
10 changes: 9 additions & 1 deletion tests/test-class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,12 @@ public function test_prepare_response() {
add_action( 'wp_enqueue_scripts', function() {
wp_enqueue_script( 'amp-list' );
} );
add_action( 'wp_print_scripts', function() {
echo '<!-- wp_print_scripts -->';
} );
add_filter( 'script_loader_tag', function( $tag, $handle ) {
return preg_replace( '/(?<=<script)/', " handle='$handle' ", $tag );
}, 10, 2 );
add_action( 'wp_footer', function() {
wp_print_scripts( 'amp-mathml' );
?>
Expand Down Expand Up @@ -1016,6 +1022,8 @@ public function test_prepare_response() {
},
) );

$this->assertNotContains( 'handle=', $sanitized_html );
$this->assertEquals( 2, substr_count( $sanitized_html, '<!-- wp_print_scripts -->' ) );
$this->assertContains( '<meta charset="' . get_bloginfo( 'charset' ) . '">', $sanitized_html );
$this->assertContains( '<meta name="viewport" content="width=device-width,minimum-scale=1">', $sanitized_html );
$this->assertContains( '<style amp-boilerplate>', $sanitized_html );
Expand All @@ -1036,7 +1044,7 @@ public function test_prepare_response() {
$this->assertContains( '<script type=\'text/javascript\' src=\'https://cdn.ampproject.org/v0/amp-ad-latest.js\' async custom-element="amp-ad"></script>', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript

$this->assertContains( '<button>no-onclick</button>', $sanitized_html );
$this->assertCount( 3, $removed_nodes );
$this->assertCount( 4, $removed_nodes );
$this->assertInstanceOf( 'DOMElement', $removed_nodes['script'] );
$this->assertInstanceOf( 'DOMAttr', $removed_nodes['onclick'] );
}
Expand Down