Merge branch '0.7' of https://github.com/Automattic/amp-wp into develop

chetansatasiya · Jun 27, 2018 · 09dbb8a · 09dbb8a
2 parents 18093fa + e5fa3a5
commit 09dbb8a
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 46 deletions.
diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php
@@ -399,6 +399,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(
+				"<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.
  *
@@ -668,6 +715,8 @@ function amp_get_post_image_metadata( $post = null ) {
 
 	if ( has_post_thumbnail( $post->ID ) ) {
 		$post_image_id = get_post_thumbnail_id( $post->ID );
+	} elseif ( ( 'attachment' === $post->post_type ) && wp_attachment_is( 'image', $post ) ) {
+		$post_image_id = $post->ID;
 	} else {
 		$attached_image_ids = get_posts(
 			array(

diff --git a/includes/amp-post-template-actions.php b/includes/amp-post-template-actions.php
@@ -52,22 +52,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
@@ -1321,32 +1321,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>)#',

diff --git a/readme.md b/readme.md
@@ -8,7 +8,7 @@ Enable Accelerated Mobile Pages (AMP) on your WordPress site.
 **Tags:** [amp](https://wordpress.org/plugins/tags/amp), [mobile](https://wordpress.org/plugins/tags/mobile)  
 **Requires at least:** 4.7  
 **Tested up to:** 4.9  
-**Stable tag:** 0.7.1  
+**Stable tag:** 0.7.2  
 **License:** [GPLv2 or later](http://www.gnu.org/licenses/gpl-2.0.html)  
 **Requires PHP:** 5.3.2  
 
@@ -103,6 +103,13 @@ Follow along with or [contribute](https://github.com/Automattic/amp-wp/blob/deve
 
 For a full list of the closed issues and merged pull requests in this release, see the [1.0 milestone](https://github.com/Automattic/amp-wp/milestone/7?closed=1).
 
+### 0.7.2 (2018-06-27) ###
+- Prevent plugins from outputting custom scripts in classic templates via `wp_print_scripts` action. See [#1225](https://github.com/Automattic/amp-wp/issues/1225), [#1227](https://github.com/Automattic/amp-wp/pull/1227). Props westonruter.
+- Display Schema.org image data for 'attachment' post type. See [#1157](https://github.com/Automattic/amp-wp/issues/1157), [#1176](https://github.com/Automattic/amp-wp/pull/1176). Props kienstra.
+- Output `alt` attribute in legacy templating gravatar image. See [#1179](https://github.com/Automattic/amp-wp/pull/1179). Props kienstra.
+
+See [0.7.2 milestone](https://github.com/Automattic/amp-wp/milestone/9?closed=1).
+
 ### 0.7.1 (2018-05-23) ###
 - Limit showing AMP validation warnings to when `amp` theme support is present. See [#1132](https://github.com/Automattic/amp-wp/pull/1132). Props westonruter.
 - Supply the extracted dimensions to images determined to need them; fixes regression from 0.6 this is key for Gutenberg compat. See [#1117](https://github.com/Automattic/amp-wp/pull/1117). Props westonruter.

diff --git a/readme.txt b/readme.txt
@@ -3,7 +3,7 @@ Contributors: batmoo, joen, automattic, potatomaster, albertomedina, google, xwp
 Tags: amp, mobile
 Requires at least: 4.7
 Tested up to: 4.9
-Stable tag: 0.7.1
+Stable tag: 0.7.2
 License: GPLv2 or later
 License URI: http://www.gnu.org/licenses/gpl-2.0.html
 Requires PHP: 5.3.2
@@ -86,6 +86,14 @@ Follow along with or [contribute](https://github.com/Automattic/amp-wp/blob/deve
 
 For a full list of the closed issues and merged pull requests in this release, see the [1.0 milestone](https://github.com/Automattic/amp-wp/milestone/7?closed=1).
 
+= 0.7.2 (2018-06-27) =
+
+- Prevent plugins from outputting custom scripts in classic templates via `wp_print_scripts` action. See [#1225](https://github.com/Automattic/amp-wp/issues/1225), [#1227](https://github.com/Automattic/amp-wp/pull/1227). Props westonruter.
+- Display Schema.org image data for 'attachment' post type. See [#1157](https://github.com/Automattic/amp-wp/issues/1157), [#1176](https://github.com/Automattic/amp-wp/pull/1176). Props kienstra.
+- Output `alt` attribute in legacy templating gravatar image. See [#1179](https://github.com/Automattic/amp-wp/pull/1179). Props kienstra.
+
+See [0.7.2 milestone](https://github.com/Automattic/amp-wp/milestone/9?closed=1).
+
 = 0.7.1 (2018-05-23) =
 
 - Limit showing AMP validation warnings to when `amp` theme support is present. See [#1132](https://github.com/Automattic/amp-wp/pull/1132). Props westonruter.

diff --git a/templates/meta-author.php b/templates/meta-author.php
@@ -2,7 +2,7 @@
 <?php if ( $post_author ) : ?>
 	<div class="amp-wp-meta amp-wp-byline">
 		<?php if ( function_exists( 'get_avatar_url' ) ) : ?>
-			<amp-img src="<?php echo esc_url( get_avatar_url( $post_author->user_email, array( 'size' => 24 ) ) ); ?>" width="24" height="24" layout="fixed"></amp-img>
+			<amp-img src="<?php echo esc_url( get_avatar_url( $post_author->user_email, array( 'size' => 24 ) ) ); ?>" alt="<?php echo esc_attr( $post_author->display_name ); ?>" width="24" height="24" layout="fixed"></amp-img>
 		<?php endif; ?>
 		<span class="amp-wp-author author vcard"><?php echo esc_html( $post_author->display_name ); ?></span>
 	</div>

diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php
@@ -363,6 +363,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() {
@@ -392,6 +393,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();
@@ -552,6 +563,51 @@ public function test_amp_get_post_image_metadata() {
 		) );
 		$metadata = amp_get_post_image_metadata( $post_id );
 		$this->assertStringEndsWith( 'test-image.jpg', $metadata['url'] );
+
+		// Test an 'attachment' post type.
+		$attachment_src          = 'example/attachment.jpeg';
+		$attachment_height       = 45;
+		$attachment_width        = 600;
+		$attachment_id           = $this->factory()->attachment->create_object(
+			$attachment_src,
+			0,
+			array(
+				'post_mime_type' => 'image/jpeg',
+			)
+		);
+		$expected_attachment_img = wp_get_attachment_image_src( $attachment_id, 'full', false );
+
+		update_post_meta(
+			$attachment_id,
+			'_wp_attachment_metadata',
+			array(
+				'height' => $attachment_height,
+				'width'  => $attachment_width,
+			)
+		);
+		$this->go_to( get_permalink( $attachment_id ) );
+
+		$this->assertEquals(
+			array(
+				'@type'  => 'ImageObject',
+				'height' => $attachment_height,
+				'url'    => $expected_attachment_img[0],
+				'width'  => $attachment_width,
+			),
+			amp_get_post_image_metadata( $attachment_id )
+		);
+
+		// Test a video as an 'attachment' post type, which shouldn't have a schema.org image.
+		$attachment_src = 'example/test-video.mpeg';
+		$attachment_id  = $this->factory()->attachment->create_object(
+			$attachment_src,
+			0,
+			array(
+				'post_mime_type' => 'video/mpeg',
+			)
+		);
+		$this->go_to( get_permalink( $attachment_id ) );
+		$this->assertFalse( amp_get_post_image_metadata( $attachment_id ) );
 	}
 
 	/**

diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php
@@ -1071,6 +1071,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() {
 		add_filter( 'amp_validation_error_sanitized', '__return_true' );
@@ -1141,7 +1142,7 @@ public function test_prepare_response() {
 		$this->assertContains( '<meta name="viewport" content="width=device-width,minimum-scale=1">', $sanitized_html );
 		$this->assertContains( '<style amp-boilerplate>', $sanitized_html );
 		$this->assertContains( '<noscript><style amp-boilerplate>', $sanitized_html );
-		$this->assertRegExp( '#<style amp-custom>.*?body{background:black}.*?</style>#s', $sanitized_html );
+		$this->assertRegExp( '#<style amp-custom>.*?body\s*{\s*background:\s*black;?\s*}.*?</style>#s', $sanitized_html );
 		$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