Improve AMP support for Photon

3rd-party/class.jetpack-amp-support.php

@@ -37,6 +37,9 @@ public static function init() {
 
 		// Add post template metadata for legacy AMP.
 		add_filter( 'amp_post_template_metadata', array( 'Jetpack_AMP_Support', 'amp_post_template_metadata' ), 10, 2 );
+
+		// Filter photon image args for AMP Stories.
+		add_filter( 'jetpack_photon_post_image_args', array( 'Jetpack_AMP_Support', 'filter_photon_post_image_args_for_stories' ), 10, 2 );
 	}
 
 	/**
@@ -365,6 +368,74 @@ public static function amp_disable_sharedaddy_css( $enqueue ) {
 
 		return $enqueue;
 	}
+
+	/**
+	 * Ensure proper Photon image dimensions for AMP Stories.
+	 *
+	 * @param array $args Array of Photon Arguments.
+	 * @param array $details {
+	 *     Array of image details.
+	 *
+	 *     @type string    $tag            Image tag (Image HTML output).
+	 *     @type string    $src            Image URL.
+	 *     @type string    $src_orig       Original Image URL.
+	 *     @type int|false $width          Image width.
+	 *     @type int|false $height         Image height.
+	 *     @type int|false $width_orig     Original image width before constrained by content_width.
+	 *     @type int|false $height_orig    Original Image height before constrained by content_width.
+	 *     @type string    $transform_orig Original transform before constrained by content_width.
+	 * }
+	 * @return array Args.
+	 */
+	public static function filter_photon_post_image_args_for_stories( $args, $details ) {
+		if ( ! is_singular( 'amp_story' ) ) {
+			return $args;
+		}
+
+		// Percentage-based dimensions are not allowed in AMP, so this shouldn't happen, but short-circuit just in case.
+		if ( false !== strpos( $details['width_orig'], '%' ) || false !== strpos( $details['height_orig'], '%' ) ) {
+			return $args;
+		}
+
+		$max_height = 1440; // See image size with the slug \AMP_Story_Post_Type::MAX_IMAGE_SIZE_SLUG.
+		$transform  = $details['transform_orig'];
+		$width      = $details['width_orig'];
+		$height     = $details['height_orig'];
+
+		// If height is available, constrain to $max_height.
+		if ( false !== $height ) {
+			if ( $height > $max_height && false !== $height ) {
+				$width  = ( $max_height * $width ) / $height;
+				$height = $max_height;
+			} elseif ( $height > $max_height ) {
+				$height = $max_height;
+			}
+		}
+
+		/*
+		 * Set a height if none is found.
+		 * If height is set in this manner and height is available, use `fit` instead of `resize` to prevent skewing.
+		 */
+		if ( false === $height ) {
+			$height = $max_height;
+			if ( false !== $width ) {
+				$transform = 'fit';
+			}
+		}
+
+		// Build array of Photon args and expose to filter before passing to Photon URL function.
+		$args = array();
+
+		if ( false !== $width && false !== $height ) {
+			$args[ $transform ] = $width . ',' . $height;
+		} elseif ( false !== $width ) {
+			$args['w'] = $width;
+		} elseif ( false !== $height ) {
+			$args['h'] = $height;
+		}
+
+		return $args;
+	}
 }
 
 add_action( 'init', array( 'Jetpack_AMP_Support', 'init' ), 1 );

class.photon.php

@@ -239,7 +239,7 @@ public static function filter_photon_norezise_maybe_inject_sizes_api( $sizes, $a
 	public static function parse_images_from_html( $content ) {
 		$images = array();
 
-		if ( preg_match_all( '#(?:<a[^>]+?href=["|\'](?P<link_url>[^\s]+?)["|\'][^>]*?>\s*)?(?P<img_tag><img[^>]*?\s+?src=["|\'](?P<img_url>[^\s]+?)["|\'].*?>){1}(?:\s*</a>)?#is', $content, $images ) ) {
+		if ( preg_match_all( '#(?:<a[^>]+?href=["|\'](?P<link_url>[^\s]+?)["|\'][^>]*?>\s*)?(?P<img_tag><(?:img|amp-img|amp-anim)[^>]*?\s+?src=["|\'](?P<img_url>[^\s]+?)["|\'].*?>){1}(?:\s*</a>)?#is', $content, $images ) ) {
 			foreach ( $images as $key => $unused ) {
 				// Simplify the output as much as possible, mostly for confirming test results.
 				if ( is_numeric( $key ) && $key > 0 )
@@ -419,6 +419,10 @@ public static function filter_the_content( $content ) {
 						list( $width, $height ) = Jetpack_Photon::parse_dimensions_from_filename( $src );
 					}
 
+					$width_orig     = $width;
+					$height_orig    = $height;
+					$transform_orig = $transform;
+
 					// If width is available, constrain to $content_width
 					if ( false !== $width && false === strpos( $width, '%' ) && is_numeric( $content_width ) ) {
 						if ( $width > $content_width && false !== $height && false === strpos( $height, '%' ) ) {
@@ -467,17 +471,21 @@ public static function filter_the_content( $content ) {
 					 * @since 2.0.0
 					 *
 					 * @param array $args Array of Photon Arguments.
-					 * @param array $args {
-					 * 	 Array of image details.
+					 * @param array $details {
+					 *     Array of image details.
 					 *
-					 * 	 @type $tag Image tag (Image HTML output).
-					 * 	 @type $src Image URL.
-					 * 	 @type $src_orig Original Image URL.
-					 * 	 @type $width Image width.
-					 * 	 @type $height Image height.
+					 *     @type string    $tag            Image tag (Image HTML output).
+					 *     @type string    $src            Image URL.
+					 *     @type string    $src_orig       Original Image URL.
+					 *     @type int|false $width          Image width.
+					 *     @type int|false $height         Image height.
+					 *     @type int|false $width_orig     Original image width before constrained by content_width.
+					 *     @type int|false $height_orig    Original Image height before constrained by content_width.
+					 *     @type string    $transform      Transform.
+					 *     @type string    $transform_orig Original transform before constrained by content_width.
 					 * }
 					 */
-					$args = apply_filters( 'jetpack_photon_post_image_args', $args, compact( 'tag', 'src', 'src_orig', 'width', 'height' ) );
+					$args = apply_filters( 'jetpack_photon_post_image_args', $args, compact( 'tag', 'src', 'src_orig', 'width', 'height', 'width_orig', 'height_orig', 'transform', 'transform_orig' ) );
 
 					$photon_url = jetpack_photon_url( $src, $args );
 
@@ -536,7 +544,9 @@ public static function filter_the_content( $content ) {
 						}
 
 						// Tag an image for dimension checking
-						$new_tag = preg_replace( '#(\s?/)?>(\s*</a>)?$#i', ' data-recalc-dims="1"\1>\2', $new_tag );
+						if ( ! self::is_amp_endpoint() ) {
+							$new_tag = preg_replace( '#(\s?/)?>(\s*</a>)?$#i', ' data-recalc-dims="1"\1>\2', $new_tag );
+						}
 
 						// Replace original tag with modified version
 						$content = str_replace( $tag, $new_tag, $content );
@@ -1181,7 +1191,7 @@ public function noresize_intermediate_sizes( $sizes ) {
 	 * @return null
 	 */
 	public function action_wp_enqueue_scripts() {
-		if ( Jetpack_AMP_Support::is_amp_request() ) {
+		if ( self::is_amp_endpoint() ) {
 			return;
 		}
 		wp_enqueue_script(
@@ -1264,4 +1274,16 @@ public function cleanup_rest_photon_image_downsize( $response ) {
 	public function _override_image_downsize_in_rest_edit_context() {
 		return true;
 	}
+
+	/**
+	 * Return whether the current page is AMP.
+	 *
+	 * This is only present for the sake of WordPress.com where the Jetpack_AMP_Support
+	 * class does not yet exist. This mehod may only be called at the wp action or later.
+	 *
+	 * @return bool Whether AMP page.
+	 */
+	private static function is_amp_endpoint() {
+		return class_exists( 'Jetpack_AMP_Support' ) && Jetpack_AMP_Support::is_amp_request();
+	}
 }

tests/php/test_class.jetpack_photon.php

@@ -804,6 +804,7 @@ public function test_photon_filter_the_content_does_not_have_width_height_when_a
 
 		$this->assertArrayNotHasKey( 'width', $attributes );
 		$this->assertArrayNotHasKey( 'height', $attributes );
+		$this->assertContains( 'data-recalc-dims', $filtered_content );
 	}
 
 	/**
@@ -815,9 +816,9 @@ public function test_photon_filter_the_content_does_not_have_width_height_when_a
 	public function test_photon_filter_the_content_width_height_attributes_when_image_args_filtered( $filter_callback, $has_attributes, $width, $height ) {
 		list( $sample_html ) = $this->get_photon_sample_content( 'a-tags-without-images.html' );
 
-		add_filter( 'jetpack_photon_post_image_args', array( $this, $filter_callback ) );
+		add_filter( 'jetpack_photon_post_image_args', $filter_callback, 10, 2 );
 		$filtered_content = Jetpack_Photon::filter_the_content( $sample_html );
-		remove_filter( 'jetpack_photon_post_image_args', array( $this, $filter_callback ) );
+		remove_filter( 'jetpack_photon_post_image_args', $filter_callback, 10, 2 );
 
 		$first_line = strtok( $filtered_content, "\n" ); // Should contain an image tag on the first line
 		$attributes = wp_kses_hair( $first_line, wp_allowed_protocols() );
@@ -836,56 +837,138 @@ public function test_photon_filter_the_content_width_height_attributes_when_imag
 	}
 
 	public function photon_attributes_when_filtered_data_provider() {
+		$that = $this; // For sake of PHP 5.3.
+
+		$assert_details = function ( $details ) use ( $that ) {
+			$that->assertInternalType( 'array', $details );
+			$that->assertArrayHasKey( 'tag', $details );
+			$that->assertArrayHasKey( 'src', $details );
+			$that->assertArrayHasKey( 'src_orig', $details );
+			$that->assertArrayHasKey( 'width', $details );
+			$that->assertArrayHasKey( 'width_orig', $details );
+			$that->assertArrayHasKey( 'height', $details );
+			$that->assertArrayHasKey( 'height_orig', $details );
+			$that->assertArrayHasKey( 'transform', $details );
+			$that->assertArrayHasKey( 'transform_orig', $details );
+		};
+
 		return array(
 			'photon_post_image_args_force_resize' => array(
-				'photon_post_image_args_force_resize',
+				function( $args, $details ) use ( $assert_details ) {
+					$assert_details( $details );
+					return array(
+						'resize' => '300,250'
+					);
+				},
 				true,
 				300,
 				250
 			),
 			'photon_post_image_args_force_fit' => array(
-				'photon_post_image_args_force_fit',
+				function ( $args, $details ) use ( $assert_details ) {
+					$assert_details( $details );
+					return array(
+						'fit' => '600,600'
+					);
+				},
 				true,
 				600,
 				600
 			),
 			'photon_post_image_args_force_lb' => array(
-				'photon_post_image_args_force_lb',
+				function ( $args, $details ) use ( $assert_details ) {
+					$assert_details( $details );
+					return array(
+						'lb' => '800,100,000000'
+					);
+				},
 				true,
 				800,
 				100
 			),
 			'photon_post_image_args_force_width_only' => array(
-				'photon_post_image_args_force_width_only',
+				function ( $args, $details ) use ( $assert_details ) {
+					$assert_details( $details );
+					return array(
+						'w' => '104'
+					);
+				},
 				false,
 				false,
 				false
 			),
 		);
 	}
 
-	public function photon_post_image_args_force_resize() {
-		return array(
-			'resize' => '300,250'
-		);
+	/**
+	 * @author westonruter
+	 * @covers Jetpack_Photon::filter_the_content
+	 * @dataProvider photon_attributes_when_amp_response
+	 * @since 7.6.0
+	 *
+	 * @param string $sample_html Sample HTML.
+	 * @param string $photon_src  Photon URL suffix (after the subdomain).
+	 */
+	public function test_photon_filter_the_content_for_amp_responses( $sample_html, $photon_src ) {
+		add_filter( 'jetpack_is_amp_request', '__return_true' );
+		$filtered_content = Jetpack_Photon::filter_the_content( $sample_html );
+		$attributes = wp_kses_hair( $filtered_content, wp_allowed_protocols() );
+		$this->assertStringEndsWith( $photon_src, html_entity_decode( $attributes['src']['value'] ) );
+		$this->assertArrayHasKey( 'width', $attributes );
+		$this->assertArrayHasKey( 'height', $attributes );
+		$this->assertNotContains( 'data-recalc-dims', $filtered_content );
 	}
 
-	public function photon_post_image_args_force_fit() {
+	/**
+	 * Data provider for testing AMP responses.
+	 *
+	 * @return array
+	 */
+	public function photon_attributes_when_amp_response() {
 		return array(
-			'fit' => '600,600'
+			'amp-img' => array(
+				'<amp-img class="aligncenter wp-image-6372" title="Tube Bomber salmon dry fly" alt="Tube Bomber salmon dry fly" src="http://www.fishmadman.com/pages/wp-content/uploads/2012/02/Rav-fra-2004-2009-11-1024x611.jpg" width="102" height="61"></amp-img>',
+				'.wp.com/www.fishmadman.com/pages/wp-content/uploads/2012/02/Rav-fra-2004-2009-11-1024x611.jpg?resize=102%2C61',
+			),
+			'amp-anim' => array(
+				'<amp-anim alt="LOL" src="https://example.com/lol.gif" width="32" height="32"></amp-anim>',
+				'.wp.com/example.com/lol.gif?resize=32%2C32&ssl=1',
+			),
 		);
 	}
 
-	public function photon_post_image_args_force_lb() {
-		return array(
-			'lb' => '800,100,000000'
+	/**
+	 * @author westonruter
+	 * @covers Jetpack_Photon::filter_the_content
+	 * @covers Jetpack_AMP_Support::filter_photon_post_image_args_for_stories
+	 * @since 7.6.0
+	 */
+	public function test_photon_filter_the_content_for_amp_story() {
+		$post_type = 'amp_story';
+		add_filter( 'jetpack_is_amp_request', '__return_true' );
+		register_post_type( $post_type, array( 'public' => true ) );
+		Jetpack_AMP_Support::init();
+		$post = $this->factory()->post->create_and_get( compact( 'post_type' ) );
+		$this->go_to( get_permalink( $post ) );
+		$this->assertTrue( is_singular( $post_type ) );
+
+		$content = implode(
+			"\n",
+			array(
+				'<!-- wp:amp/amp-story-page {"mediaId":2414,"mediaType":"image","focalPoint":{"x":0.4900990099009901,"y":0.5131578947368421}} -->',
+				'<amp-story-page style="background-color:#ffffff" id="a6c81a13-14a0-464b-88fa-9612e86bacf7" class="wp-block-amp-amp-story-page"><amp-story-grid-layer template="fill"><amp-img layout="fill" src="https://example.com/wp-content/uploads/2019/06/huge.jpg" style="object-position:49.00990099009901% 51.31578947368421%"></amp-img></amp-story-grid-layer><amp-story-grid-layer template="fill"></amp-story-grid-layer></amp-story-page>',
+				'<!-- /wp:amp/amp-story-page -->',
+			)
 		);
-	}
 
-	public function photon_post_image_args_force_width_only() {
-		return array(
-			'w' => '104'
+		$filtered_content = apply_filters( 'the_content', $content, $post->ID );
+
+		$this->assertContains(
+			'.wp.com/example.com/wp-content/uploads/2019/06/huge.jpg?h=1440&#038;ssl=1',
+			$filtered_content
 		);
+
+		unregister_post_type( $post_type );
 	}
 
 	/**