Add support for extracting SVG dimensions from viewBox

ampproject · May 17, 2018 · 0e88d14 · 0e88d14
1 parent a0abaa9
commit 0e88d14
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 28 deletions.
diff --git a/includes/lib/fasterimage/FasterImage.php b/includes/lib/fasterimage/FasterImage.php
@@ -35,6 +35,8 @@ class FasterImage
      */
     public function batch(array $urls)
     {
+    	echo __METHOD__ . ':' . __LINE__ . PHP_EOL;
+    	print_r($urls);
 
         $multi   = curl_multi_init();
         $results = array();
@@ -72,10 +74,13 @@ public function batch(array $urls)
             $error = curl_error($$count);
 
             if ( $error ) {
-                $results[$uri]['failure_reason'] = $error;
+                $results[$uri]['failure_reason'] = sprintf( '%s (error code: %s)', $error, curl_errno( $$count ) );
             }
         }
 
+	    echo __METHOD__ . ':' . __LINE__ . PHP_EOL;
+	    print_r($results);
+
         return $results;
     }
 
@@ -97,6 +102,7 @@ public function setTimeout($seconds)
      */
     protected function handle($url, & $result)
     {
+	    echo __METHOD__ . ':' . __LINE__ . PHP_EOL;
         $stream           = new Stream();
         $parser           = new ImageParser($stream);
         $result['rounds'] = 0;
@@ -128,13 +134,15 @@ protected function handle($url, & $result)
         curl_setopt($ch, CURLOPT_ENCODING, "");
 
         curl_setopt($ch, CURLOPT_WRITEFUNCTION, function ($ch, $str) use (& $result, & $parser, & $stream, $url) {
+	        echo __METHOD__ . ':' . __LINE__ . PHP_EOL;
 
             $result['rounds']++;
             $result['bytes'] += strlen($str);
 
             $stream->write($str);
 
             try {
+            	echo "TRY: " . __METHOD__ . ':' . __LINE__ . PHP_EOL;
                 // store the type in the result array by looking at the bits
                 $result['type'] = $parser->parseType();
 
@@ -143,8 +151,10 @@ protected function handle($url, & $result)
                  * for the size.
                  */
                 $result['size'] = $parser->parseSize() ?: 'failed';
+                print_r($result);
             }
             catch (StreamBufferTooSmallException $e) {
+            	echo "StreamBufferTooSmallException\n";
                 /*
                  * If this exception is thrown, we don't have enough of the stream buffered
                  * so in order to tell curl to keep streaming we need to return the number
@@ -159,6 +169,7 @@ protected function handle($url, & $result)
                 return strlen($str);
             }
             catch (InvalidImageException $e) {
+	            echo "InvalidImageException\n";
 
                 /*
                  * This means we've determined that we're lost and don't know

diff --git a/includes/lib/fasterimage/ImageParser.php b/includes/lib/fasterimage/ImageParser.php
@@ -136,7 +136,11 @@ public function parseType()
                     return $this->type = 'tiff';
                 default:
                     $this->stream->resetPointer();
-                    if ( false !== strpos( $this->stream->read( 1024 ), '<svg' ) ) {
+                    $markup = $this->stream->read( 1024 );
+	                echo( __METHOD__ . ':' . $markup . "\n" );
+	                echo( ( false !== strpos( $markup, '<svg' ) ? 'YES SVG' : 'NOPE' ) . "\n" );
+
+                    if ( false !== strpos( $markup, '<svg' ) ) {
                         $this->type = 'svg';
                     } else {
                         return false;
@@ -359,7 +363,9 @@ protected function parseSizeForWebp()
      */
     protected function parseSizeForSvg()
     {
+        $this->stream->resetPointer();
         $markup = $this->stream->read( 1024 );
+	    echo( __METHOD__ . ':' . $markup . "\n" );
         if ( ! preg_match( '#<svg.*?>#s', $markup, $matches ) ) {
             return null;
         }
@@ -375,6 +381,12 @@ protected function parseSizeForSvg()
         if ( $width && $height ) {
             return [ $width, $height ];
         }
+        if ( preg_match( '/\sviewBox=([\'"])[^\1]*(?:,|\s)+(?P<width>\d+(?:\.\d+)?)(?:px)?(?:,|\s)+(?P<height>\d+(?:\.\d+)?)(?:px)?\s*\1/', $svg_start_tag, $matches ) ) {
+            return [
+                floatval( $matches['width'] ),
+                floatval( $matches['height'] )
+            ];
+        }
         return null;
     }
 
@@ -407,4 +419,4 @@ private function readInt($str)
 
         return ($size[1] << 8) + $size[2];
     }
-}
+}
diff --git a/includes/lib/fastimage/class-fastimage.php b/includes/lib/fastimage/class-fastimage.php
@@ -71,7 +71,11 @@ public function getType()
                     return $this->type = 'png';
                 default:
                     $this->strpos = 0;
-                    if ( false !== strpos( $this->getChars( 1024 ), '<svg' ) ) {
+	                $markup = $this->getChars( 1024 );
+	                $this->strpos = 0;
+	                echo( __METHOD__ . ':' . $markup . "\n" );
+	                echo( ( false !== strpos( $markup, '<svg' ) ? 'YES SVG' : 'NOPE' ) . "\n" );
+                    if ( false !== strpos( $markup, '<svg' ) ) {
                         $this->type = 'svg';
                     } else {
                         return false;
@@ -170,21 +174,36 @@ private function parseSizeForSVG()
     {
         $this->strpos = 0;
         $markup = $this->getChars( 1024 );
+        echo( __METHOD__ . ':' . $markup . "\n" );
         if ( ! preg_match( '#<svg.*?>#s', $markup, $matches ) ) {
             return null;
         }
+        echo( __METHOD__ . ':' . __LINE__ );
         $svg_start_tag = $matches[0];
         $width = null;
         $height = null;
+	    echo( __METHOD__ . ':' . __LINE__ );
         if ( preg_match( '/\swidth=([\'"])(\d+(\.\d+)?)(px)?\1/', $svg_start_tag, $matches ) ) {
             $width = floatval( $matches[2] );
         }
+	    echo( __METHOD__ . ':' . __LINE__ );
         if ( preg_match( '/\sheight=([\'"])(\d+(\.\d+)?)(px)?\1/', $svg_start_tag, $matches ) ) {
             $height = floatval( $matches[2] );
         }
+	    echo( __METHOD__ . ':' . __LINE__ );
         if ( $width && $height ) {
+	        echo( __METHOD__ . ':' . __LINE__ );
             return array( $width, $height );
         }
+	    echo( __METHOD__ . ':' . __LINE__ );
+        if ( preg_match( '/\sviewBox=([\'"])[^\1]*(?:,|\s)+(?P<width>\d+(?:\.\d+)?)(?:px)?(?:,|\s)+(?P<height>\d+(?:\.\d+)?)(?:px)?\s*\1/', $svg_start_tag, $matches ) ) {
+	        echo( __METHOD__ . ':' . __LINE__ );
+            return array(
+                floatval( $matches['width'] ),
+                floatval( $matches['height'] )
+            );
+        }
+	    echo( __METHOD__ . ':' . __LINE__ );
         return null;
     }
 
@@ -228,4 +247,4 @@ public function __destruct()
     {
         $this->close();
     }
-}
+}
diff --git a/includes/utils/class-amp-image-dimension-extractor.php b/includes/utils/class-amp-image-dimension-extractor.php
@@ -172,6 +172,7 @@ private static function fetch_images( $urls_to_fetch, &$images, $mode ) {
 	 * @param array $images Array to populate with results of image/dimension inspection.
 	 */
 	private static function fetch_images_via_fast_image( $urls_to_fetch, &$images ) {
+		echo __METHOD__ . PHP_EOL; // WPCS: XSS OK.
 
 		$image = new FastImage();
 		$urls  = array_keys( $urls_to_fetch );
@@ -195,6 +196,7 @@ private static function fetch_images_via_fast_image( $urls_to_fetch, &$images )
 	 * @param array $images Array to populate with results of image/dimension inspection.
 	 */
 	private static function fetch_images_via_faster_image( $urls_to_fetch, &$images ) {
+		echo __METHOD__ . PHP_EOL; // WPCS: XSS OK.
 		$urls = array_keys( $urls_to_fetch );
 
 		if ( ! function_exists( 'amp_get_fasterimage_client' ) ) {

diff --git a/tests/test-amp-image-dimension-extractor.php b/tests/test-amp-image-dimension-extractor.php
@@ -10,7 +10,8 @@
 // Not ideal to use remote URLs (since the remote service can change); mocking would be better.
 define( 'IMG_350', 'http://i0.wp.com/amptest.files.wordpress.com/2017/03/350x150.png' );
 define( 'IMG_1024', 'http://i0.wp.com/amptest.files.wordpress.com/2017/03/1024x768.png' );
-define( 'IMG_SVG', 'https://www.ampproject.org/static/img/logo-blue.svg' );
+define( 'IMG_SVG', 'https://gist.githubusercontent.com/westonruter/90fbaaced3851bf6ef762996c8c4375d/raw/316f589ce7f0c809a8ff101745f7ba5ea94505fb/amp.svg' );
+define( 'IMG_SVG_VIEWPORT', 'https://gist.githubusercontent.com/westonruter/90fbaaced3851bf6ef762996c8c4375d/raw/316f589ce7f0c809a8ff101745f7ba5ea94505fb/google.svg' );
 
 /**
  * Tests for AMP_Image_Dimension_Extractor.
@@ -156,28 +157,41 @@ public function test__valid_image_file_synchronous() {
 	 * Test multiple valid image files.
 	 */
 	public function test__multiple_valid_image_files() {
+		echo "\n>>>>>>>>>>>>>>>>>>>>>> " . __METHOD__ . ">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n";
+
 		$sources  = array(
-			IMG_350  => false,
-			IMG_1024 => false,
-			IMG_SVG  => false,
+			IMG_350          => false,
+			IMG_1024         => false,
+			IMG_SVG          => false,
+			IMG_SVG_VIEWPORT => false,
 		);
 		$expected = array(
-			IMG_350  => array(
+			IMG_350          => array(
 				'width'  => 350,
 				'height' => 150,
 			),
-			IMG_1024 => array(
+			IMG_1024         => array(
 				'width'  => 1024,
 				'height' => 768,
 			),
-			IMG_SVG  => array(
+			IMG_SVG          => array(
 				'width'  => 175,
 				'height' => 60,
 			),
+			IMG_SVG_VIEWPORT => array(
+				'width'  => 251,
+				'height' => 80,
+			),
 		);
 
+		// Temp start.
+		$sources  = wp_array_slice_assoc( $sources, array( IMG_SVG ) );
+		$expected = wp_array_slice_assoc( $expected, array( IMG_SVG ) );
+
+		// Temp end.
 		$dimensions = AMP_Image_Dimension_Extractor::extract_by_downloading_images( $sources );
 
+		echo "\n<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n";
 		$this->assertEquals( $expected, $dimensions );
 	}
 
@@ -186,23 +200,28 @@ public function test__multiple_valid_image_files() {
 	 */
 	public function test__multiple_valid_image_files_synchronous() {
 		$sources  = array(
-			IMG_350  => false,
-			IMG_1024 => false,
-			IMG_SVG  => false,
+			IMG_350          => false,
+			IMG_1024         => false,
+			IMG_SVG          => false,
+			IMG_SVG_VIEWPORT => false,
 		);
 		$expected = array(
-			IMG_350  => array(
+			IMG_350          => array(
 				'width'  => 350,
 				'height' => 150,
 			),
-			IMG_1024 => array(
+			IMG_1024         => array(
 				'width'  => 1024,
 				'height' => 768,
 			),
-			IMG_SVG  => array(
+			IMG_SVG          => array(
 				'width'  => 175,
 				'height' => 60,
 			),
+			IMG_SVG_VIEWPORT => array(
+				'width'  => 251,
+				'height' => 80,
+			),
 		);
 
 		$dimensions = AMP_Image_Dimension_Extractor::extract_by_downloading_images( $sources, 'synchronous' );
@@ -250,7 +269,6 @@ public function test__mix_of_valid_and_invalid_image_file() {
 			IMG_350                             => false,
 			AMP_IMG_DIMENSION_TEST_INVALID_FILE => false,
 			IMG_1024                            => false,
-			IMG_SVG                             => false,
 		);
 		$expected = array(
 			IMG_350                             => array(
@@ -262,10 +280,6 @@ public function test__mix_of_valid_and_invalid_image_file() {
 				'width'  => 1024,
 				'height' => 768,
 			),
-			IMG_SVG                             => array(
-				'width'  => 175,
-				'height' => 60,
-			),
 		);
 
 		$dimensions = AMP_Image_Dimension_Extractor::extract_by_downloading_images( $sources );
@@ -281,7 +295,6 @@ public function test__mix_of_valid_and_invalid_image_file_synchronous() {
 			IMG_350                             => false,
 			AMP_IMG_DIMENSION_TEST_INVALID_FILE => false,
 			IMG_1024                            => false,
-			IMG_SVG                             => false,
 		);
 		$expected = array(
 			IMG_350                             => array(
@@ -293,10 +306,6 @@ public function test__mix_of_valid_and_invalid_image_file_synchronous() {
 				'width'  => 1024,
 				'height' => 768,
 			),
-			IMG_SVG                             => array(
-				'width'  => 175,
-				'height' => 60,
-			),
 		);
 
 		$dimensions = AMP_Image_Dimension_Extractor::extract_by_downloading_images( $sources, 'synchronous' );