Skip to content

Commit

Permalink
Remove all hooks related to script printing when outputting remaining…
Browse files Browse the repository at this point in the history
… AMP scripts

This is to guard against anything AMP-invalid from being output since remaining scripts are printed after the sanitizer has finished
  • Loading branch information
westonruter committed Apr 26, 2018
1 parent 846123a commit 97455ce
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
12 changes: 10 additions & 2 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -1128,9 +1128,17 @@ 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();
remove_all_filters( 'print_scripts_array' ); // Make sure only the script handles we pass in will in fact get printed.
$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 ) ) {
Expand Down
10 changes: 9 additions & 1 deletion tests/test-class-amp-theme-support.php
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

0 comments on commit 97455ce

Please sign in to comment.