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

Introduce amp_render_scripts() to print AMP component scripts and nothing else #1227

Merged
merged 1 commit into from
Jun 26, 2018
Merged

Introduce amp_render_scripts() to print AMP component scripts and nothing else #1227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 47 additions & 0 deletions includes/amp-helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice how this is decomposed into amp_render_scripts(), to allow using it for Legacy Templating and theme support.

$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(
"<script type='text/javascript' src='%s'></script>\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.
*
Expand Down
23 changes: 7 additions & 16 deletions includes/amp-post-template-actions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

/**
Expand Down
28 changes: 2 additions & 26 deletions includes/class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"<script type='text/javascript' src='%s'></script>\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(
'#(?=</head>)#',
Expand Down
11 changes: 11 additions & 0 deletions tests/test-amp-helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -212,6 +213,16 @@ public function test_script_registering() {
$this->assertContains( '<script type=\'text/javascript\' src=\'https://cdn.ampproject.org/v0/amp-mathml-latest.js\' async custom-element="amp-mathml"></script>', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
$this->assertContains( '<script type=\'text/javascript\' src=\'https://cdn.ampproject.org/v0/amp-mustache-0.1.js\' async custom-template="amp-mustache"></script>', $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( '<script type=\'text/javascript\' src=\'https://cdn.ampproject.org/v0/amp-mustache-2.0.js\' async custom-element="amp-carousel"></script>', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
$this->assertContains( '<script type=\'text/javascript\' src=\'https://cdn.ampproject.org/v0/amp-accordion-latest.js\' async custom-element="amp-accordion"></script>', $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();
Expand Down
3 changes: 2 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 @@ -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;
Expand Down Expand Up @@ -1049,7 +1050,7 @@ public function test_prepare_response() {
$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 );
$this->assertContains( '<style amp-custom>body { background: black; }', $sanitized_html );
$this->assertRegExp( '#<style amp-custom>.*?body\s*{\s*background:\s*black;?\s*}.*?</style>#s', $sanitized_html );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of $this->assertRegExp().

$this->assertContains( '<script type="text/javascript" src="https://cdn.ampproject.org/v0.js" async></script>', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
$this->assertContains( '<script type="text/javascript" src="https://cdn.ampproject.org/v0/amp-list-latest.js" async custom-element="amp-list"></script>', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
$this->assertContains( '<script type="text/javascript" src="https://cdn.ampproject.org/v0/amp-mathml-latest.js" async custom-element="amp-mathml"></script>', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
Expand Down