Merge pull request #164 from GuideToIceland/upstream-migration

Migrating multiple fixes upstream
Lullabot · Feb 13, 2017 · a9bb7fe · a9bb7fe
2 parents b91b246 + 0e89046
commit a9bb7fe
Show file tree

Hide file tree

Showing 18 changed files with 372 additions and 34 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -12,3 +12,7 @@ before_script:
 
 script:
   - vendor/bin/phpunit tests
+
+matrix:
+  allow_failures:
+    - php: nightly
diff --git a/src/AMP.php b/src/AMP.php
@@ -174,6 +174,17 @@ public function loadHtml($html, $options = [])
             $options['use_html5_parser'] = true;
         }
 
+        // By default the convertion of img into amp-anim is disabled (because of ressource cost)
+        //  => they will be converted into amp-img instead
+        if (!isset($options['use_img_anim_tag'])) {
+            $options['use_img_anim_tag'] = false;
+        }
+
+        // By default img that can't be converted are kept as it is and not removed
+        if (!isset($options['remove_non_converted_img_tag'])) {
+            $options['remove_non_converted_img_tag'] = false;
+        }
+
         $this->options = $options;
         $this->scope = !empty($options['scope']) ? $options['scope'] : Scope::BODY_SCOPE;
 

diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php
@@ -75,6 +75,17 @@ function pass()
                     $error->resolved = false;
                 }
             }
+            elseif (in_array($error->code, [ValidationErrorCode::MANDATORY_TAG_ANCESTOR_WITH_HINT]) &&
+                !$error->resolved &&
+                !empty($error->dom_tag) &&
+                strtolower($error->dom_tag->tagName) == 'img' &&
+                !empty($this->options['remove_non_converted_img_tag'])
+            ) {
+                 // Remove the offending tag
+                $error->dom_tag->parentNode->removeChild($error->dom_tag);
+                $error->addActionTaken(new ActionTakenLine('img', ActionTakenType::TAG_REMOVED));
+                $error->resolved = TRUE;
+            }
         }
     }
 }
diff --git a/src/Pass/IframeYouTubeTagTransformPass.php b/src/Pass/IframeYouTubeTagTransformPass.php
@@ -113,37 +113,25 @@ protected function isYouTubeIframe(DOMQuery $el)
      *
      * @param DOMQuery $el
      * @return string
+     *
      */
     protected function getYouTubeCode(DOMQuery $el)
     {
         $matches = [];
         $youtube_code = '';
         $href = $el->attr('src');
 
-        // @todo there seem to be a lot of ways to embed a youtube video. We probably need to capture all patterns here
-        // The next one is the embed code that youtube gives you
-        if (preg_match('&(*UTF8)/embed/([^/?\&]+)&i', $href, $matches)) {
-            if (!empty($matches[1])) {
-                $youtube_code = $matches[1];
-                return $youtube_code;
-            }
-        }
-
-        if (preg_match('&(*UTF8)youtu\.be/([^/?\&]+)&i', $href, $matches)) {
-            if (!empty($matches[1])) {
-                $youtube_code = $matches[1];
-                return $youtube_code;
-            }
-        }
-
-        if (preg_match('!(*UTF8)watch\?v=([^&]+)!i', $href, $matches)) {
-            if (!empty($matches[1])) {
+        // This regex is supposed to catch all possible way to embed youtube video
+        if (preg_match('%(?:youtube(?:-nocookie)?\.com/(?:[^/]+/.+/|(?:v|e(?:mbed)?)/|.*[?&]v=)|youtu\.be/)([^"&?/ ]{11})%i', $href, $matches))
+        {
+            if (!empty($matches[1]))
+            {
                 $youtube_code = $matches[1];
-                return $youtube_code;
+                return htmlspecialchars($youtube_code);
             }
         }
 
-        return $youtube_code;
+        return htmlspecialchars($youtube_code);
     }
 
     /**

diff --git a/src/Pass/ImgTagTransformPass.php b/src/Pass/ImgTagTransformPass.php
@@ -79,8 +79,9 @@ function pass()
             }
             if ($this->isPixel($el)) {
                 $new_dom_el = $this->convertAmpPixel($el, $lineno, $context_string);
-            }
-            else {
+            } else if (!empty($this->options['use_amp_anim_tag']) && $this->isAnimatedImg($dom_el)) {
+                $new_dom_el = $this->convertAmpAnim($el, $lineno, $context_string);
+            } else {
                 $new_dom_el = $this->convertAmpImg($el, $lineno, $context_string);
             }
             $this->context->addLineAssociation($new_dom_el, $lineno);
@@ -139,6 +140,24 @@ protected function getLayout($el) {
             ? 'fixed' : 'responsive';
     }
 
+    /**
+     * Given an animated image element returns an amp-anim element with the same attributes and children
+     *
+     * @param DOMQuery $el
+     * @param int $lineno
+     * @param string $context_string
+     * @return DOMElement
+     */
+    protected function convertAmpAnim($el, $lineno, $context_string)
+    {
+        $dom_el = $el->get(0);
+        $new_dom_el = $this->cloneAndRenameDomElement($dom_el, 'amp-anim');
+        $new_el = $el->prev();
+        $this->setLayoutIfNoLayout($new_el, 'responsive');
+        $this->addActionTaken(new ActionTakenLine('img', ActionTakenType::IMG_ANIM_CONVERTED, $lineno, $context_string));
+        return $new_dom_el;
+    }
+
     /**
      * Given an image src attribute, try to get its dimensions
      * Returns false on failure
@@ -188,6 +207,87 @@ protected function isPixel(DOMQuery $el)
         return $el->attr('width') === '1' && $el->attr('height') === '1';
     }
 
+    /**
+     * Detects if the img is animated. In that case we convert to <amp-anim> instead of <amp-img>
+     * @param \DOMElement $el
+     * @return bool
+     */
+    protected function isAnimatedImg(\DOMElement $el)
+    {
+        $animated_type = ['gif', 'png'];
+        if (!$el->hasAttribute('src')) {
+            return true;
+        }
+
+        $src = trim($el->getAttribute('src'));
+        if (preg_match('/\.([a-z0-9]+)$/i', parse_url($src,PHP_URL_PATH), $match)) {
+            if (!empty($match[1]) && in_array(strtolower($match[1]), $animated_type)) {
+                if ($match[1] === "gif") {
+                    if ($this->isAnimatedGif($src)) {
+                        return true;
+                    } else {
+                        return false;
+                    }
+                }
+                if ($this->isApng($src)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Identifies APNGs
+     * Written by Coda, functionified by Foone/Popcorn Mariachi#!9i78bPeIxI
+     * This code is in the public domain
+     *
+     * @see http://stackoverflow.com/a/4525194
+     * @see http://foone.org/apng/identify_apng.php
+     *
+     * @param  string  $src    The filename
+     * @return bool    true if the file is an APMG
+     */
+    function isApng($src)
+    {
+        $img_bytes = @file_get_contents($src);
+        if ($img_bytes) {
+            if (strpos(substr($img_bytes, 0, strpos($img_bytes, 'IDAT')), 'acTL') !== false) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Detects if the gif image is animated or not
+     * source: http://php.net/manual/en/function.imagecreatefromgif.php#104473
+     *
+     * @param  string  $filename
+     * @return bool
+     */
+    function isAnimatedGif($filename) {
+        if (!($fh = @fopen($filename, 'rb')))
+            return FALSE;
+        $count = 0;
+        //an animated gif contains multiple "frames", with each frame having a
+        //header made up of:
+        // * a static 4-byte sequence (\x00\x21\xF9\x04)
+        // * 4 variable bytes
+        // * a static 2-byte sequence (\x00\x2C) (some variants may use \x00\x21 ?)
+
+        // We read through the file til we reach the end of the file, or we've found
+        // at least 2 frame headers
+        while (!feof($fh) && $count < 2) {
+            $chunk = fread($fh, 1024 * 100); //read 100kb at a time
+            $count += preg_match_all('#\x00\x21\xF9\x04.{4}\x00(\x2C|\x21)#s', $chunk, $matches);
+       }
+
+        fclose($fh);
+        return $count > 1;
+    }
+
     /**
      * @param string $src
      * @return boolean|string

diff --git a/src/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php
@@ -25,6 +25,7 @@ class ActionTakenType
     const PROPERTY_REMOVED_ATTRIBUTE_REMOVED = 'property value pair was removed from attribute due to validation issues. The resulting attribute was empty and was also removed.';
     const IMG_CONVERTED = 'tag was converted to the amp-img tag.';
     const IMG_PIXEL_CONVERTED = 'tag was converted to the amp-pixel tag.';
+    const IMG_ANIM_CONVERTED = 'tag was converted to the amp-anim tag.';
     const IMG_COULD_NOT_BE_CONVERTED = 'tag could NOT be converted to the amp-img tag as the image is not accessible.';
     const INSTAGRAM_CONVERTED = 'instagram embed code was converted to the amp-instagram tag.';
     const PINTEREST_CONVERTED = 'pinterest embed code was converted to the amp-pinterest tag.';

diff --git a/tests/test-data/fragment-html/img-anim-test-fragment.html b/tests/test-data/fragment-html/img-anim-test-fragment.html
@@ -0,0 +1,14 @@
+<!-- should transform to amp-anim-->
+<img src="https://upload.wikimedia.org/wikipedia/commons/thumb/2/2c/Rotating_earth_%28large%29.gif/200px-Rotating_earth_%28large%29.gif">
+
+<!-- should transform to amp-img because this gif is not animated -->
+<img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif">
+
+<!-- should transform to amp-anim-->
+<img src="https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png">
+
+<!-- should transform to amp-img because this png is not animated -->
+<img src="https://upload.wikimedia.org/wikipedia/commons/thumb/5/53/Wikimedia-logo.png/240px-Wikimedia-logo.png">
+
+<!-- nonexistent image, should refuse to convert to amp-img and delete it -->
+<img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg">
diff --git a/tests/test-data/fragment-html/img-anim-test-fragment.html.options.json b/tests/test-data/fragment-html/img-anim-test-fragment.html.options.json
@@ -0,0 +1,4 @@
+{
+  "use_amp_anim_tag" : true,
+  "remove_non_converted_img_tag" : true
+}
diff --git a/tests/test-data/fragment-html/img-anim-test-fragment.html.out b/tests/test-data/fragment-html/img-anim-test-fragment.html.out
@@ -0,0 +1,67 @@
+<!-- should transform to amp-anim-->
+<amp-anim src="https://upload.wikimedia.org/wikipedia/commons/thumb/2/2c/Rotating_earth_%28large%29.gif/200px-Rotating_earth_%28large%29.gif" width="200" height="200" layout="responsive"></amp-anim>
+
+<!-- should transform to amp-img because this gif is not animated -->
+<amp-img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif" width="46" height="46" layout="responsive"></amp-img>
+
+<!-- should transform to amp-anim-->
+<amp-anim src="https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png" width="100" height="100" layout="responsive"></amp-anim>
+
+<!-- should transform to amp-img because this png is not animated -->
+<amp-img src="https://upload.wikimedia.org/wikipedia/commons/thumb/5/53/Wikimedia-logo.png/240px-Wikimedia-logo.png" width="240" height="240" layout="responsive"></amp-img>
+
+<!-- nonexistent image, should refuse to convert to amp-img and delete it -->
+
+
+
+ORIGINAL HTML
+---------------
+Line  1: <!-- should transform to amp-anim-->
+Line  2: <img src="https://upload.wikimedia.org/wikipedia/commons/thumb/2/2c/Rotating_earth_%28large%29.gif/200px-Rotating_earth_%28large%29.gif">
+Line  3: 
+Line  4: <!-- should transform to amp-img because this gif is not animated -->
+Line  5: <img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif">
+Line  6: 
+Line  7: <!-- should transform to amp-anim-->
+Line  8: <img src="https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png">
+Line  9: 
+Line 10: <!-- should transform to amp-img because this png is not animated -->
+Line 11: <img src="https://upload.wikimedia.org/wikipedia/commons/thumb/5/53/Wikimedia-logo.png/240px-Wikimedia-logo.png">
+Line 12: 
+Line 13: <!-- nonexistent image, should refuse to convert to amp-img and delete it -->
+Line 14: <img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg">
+Line 15: 
+
+
+Transformations made from HTML tags to AMP custom tags
+-------------------------------------------------------
+
+<img src="https://upload.wikimedia.org/wikipedia/commons/thumb/2/2c/Rotating_earth_%28large%29.gif/200px-Rotating_earth_%28large%29.gif"> at line 2
+ ACTION TAKEN: img tag was converted to the amp-anim tag.
+
+<img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif"> at line 5
+ ACTION TAKEN: img tag was converted to the amp-img tag.
+
+<img src="https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png"> at line 8
+ ACTION TAKEN: img tag was converted to the amp-anim tag.
+
+<img src="https://upload.wikimedia.org/wikipedia/commons/thumb/5/53/Wikimedia-logo.png/240px-Wikimedia-logo.png"> at line 11
+ ACTION TAKEN: img tag was converted to the amp-img tag.
+
+<img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg"> at line 14
+ ACTION TAKEN: img tag could NOT be converted to the amp-img tag as the image is not accessible.
+
+
+AMP-HTML Validation Issues and Fixes
+-------------------------------------
+FAIL
+
+<img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg"> on line 14
+- The tag 'img' may only appear as a descendant of tag 'noscript'. Did you mean 'amp-img'?
+   [code: MANDATORY_TAG_ANCESTOR_WITH_HINT  category: DISALLOWED_HTML_WITH_AMP_EQUIVALENT see: https://www.ampproject.org/docs/reference/amp-img.html]
+   ACTION TAKEN: img tag was removed due to validation issues.
+
+COMPONENT NAMES WITH JS PATH
+------------------------------
+'amp-anim', include path 'https://cdn.ampproject.org/v0/amp-anim-0.1.js'
+
diff --git a/tests/test-data/fragment-html/img-test-fragment.html b/tests/test-data/fragment-html/img-test-fragment.html
@@ -6,7 +6,7 @@
 <!-- should transform to amp-img with fixed layout, preserving the width and height -->
 <img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/%22Birdcatcher%22_with_jockey_up.jpg" width="125" height="96">
 
-<!-- nonexistent image, should refuse to convert to amp-img -->
+<!-- nonexistent image, should refuse to convert to amp-img and keep it as it is -->
 <img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg">
 
 <!-- should provide layout and height width and make layout responsive -->
@@ -22,4 +22,7 @@
 <amp-img layout="responsive" src="https://upload.wikimedia.org/wikipedia/commons/e/ee/%22Birdcatcher%22_with_jockey_up.jpg" width="625rem" height="480"></amp-img>
 
 <!-- should transform to amp-pixel -->
-<img src="https://upload.wikimedia.org/wikipedia/commons/c/ce/Transparent.gif">
+<img src="https://upload.wikimedia.org/wikipedia/commons/c/ce/Transparent.gif">
+
+<!-- should transform to amp-img instead of amp-anim because of default option['use_amp_anim_tag'] = false -->
+<img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif">
diff --git a/tests/test-data/fragment-html/img-test-fragment.html.out b/tests/test-data/fragment-html/img-test-fragment.html.out
@@ -6,7 +6,7 @@
 <!-- should transform to amp-img with fixed layout, preserving the width and height -->
 <amp-img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/%22Birdcatcher%22_with_jockey_up.jpg" width="125" height="96" layout="fixed"></amp-img>
 
-<!-- nonexistent image, should refuse to convert to amp-img -->
+<!-- nonexistent image, should refuse to convert to amp-img and keep it as it is -->
 <img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg">
 
 <!-- should provide layout and height width and make layout responsive -->
@@ -24,6 +24,10 @@
 <!-- should transform to amp-pixel -->
 <amp-pixel src="https://upload.wikimedia.org/wikipedia/commons/c/ce/Transparent.gif"></amp-pixel>
 
+<!-- should transform to amp-img instead of amp-anim because of default option['use_amp_anim_tag'] = false -->
+<amp-img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif" width="46" height="46" layout="fixed"></amp-img>
+
+
 ORIGINAL HTML
 ---------------
 Line  1: <!-- Note: this image is in the public domain. https://commons.wikimedia.org/wiki/File:"Birdcatcher"_with_jockey_up.jpg -->
@@ -34,7 +38,7 @@ Line  5:
 Line  6: <!-- should transform to amp-img with fixed layout, preserving the width and height -->
 Line  7: <img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/%22Birdcatcher%22_with_jockey_up.jpg" width="125" height="96">
 Line  8: 
-Line  9: <!-- nonexistent image, should refuse to convert to amp-img -->
+Line  9: <!-- nonexistent image, should refuse to convert to amp-img and keep it as it is -->
 Line 10: <img src="https://upload.wikimedia.org/wikipedia/commons/e/ee/non-existent-image1234.jpg">
 Line 11: 
 Line 12: <!-- should provide layout and height width and make layout responsive -->
@@ -51,6 +55,10 @@ Line 22: <amp-img layout="responsive" src="https://upload.wikimedia.org/wikipedi
 Line 23: 
 Line 24: <!-- should transform to amp-pixel -->
 Line 25: <img src="https://upload.wikimedia.org/wikipedia/commons/c/ce/Transparent.gif">
+Line 26: 
+Line 27: <!-- should transform to amp-img instead of amp-anim because of default option['use_amp_anim_tag'] = false -->
+Line 28: <img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif">
+Line 29: 
 
 
 Transformations made from HTML tags to AMP custom tags
@@ -68,6 +76,9 @@ Transformations made from HTML tags to AMP custom tags
 <img src="https://upload.wikimedia.org/wikipedia/commons/c/ce/Transparent.gif"> at line 25
  ACTION TAKEN: img tag was converted to the amp-pixel tag.
 
+<img src="https://upload.wikimedia.org/wikipedia/commons/b/bb/Quilt_design_as_46x46_uncompressed_GIF.gif"> at line 28
+ ACTION TAKEN: img tag was converted to the amp-img tag.
+
 
 AMP-HTML Validation Issues and Fixes
 -------------------------------------

diff --git a/tests/test-data/fragment-html/youtube-bad-fragment.html b/tests/test-data/fragment-html/youtube-bad-fragment.html
@@ -0,0 +1,4 @@
+<iframe data-priority="high" width="560" height="315"
+        src="https://www.youtube.com/embed/MnR9AVs6Q_c&amp;list=PLiGedaJzwg4zAaznvmnz66U6K1-LFpBYk"
+        frameborder="0" allowfullscreen>
+</iframe>