Fix chain_transformations empty list handling

Fix handling of legacy srcset string option in cl_image_tag
cloudinary · Sep 6, 2018 · 5472d34 · 5472d34
1 parent c384577
commit 5472d34
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 18 deletions.
diff --git a/src/Cloudinary.php b/src/Cloudinary.php
@@ -697,17 +697,13 @@ public static function generate_transformation_string(&$options = array())
      *
      * The result of this function is an updated $options parameter
      *
-     * @param string    $source          The Public ID of the resource
      * @param array     $options         Original options
      * @param array     $transformations Transformations to chain at the end
      *
      * @return array Resulting options
      */
     public static function chain_transformations($options, $transformations)
     {
-        if (empty($transformations)) {
-            return $options;
-        }
         $transformations = \Cloudinary::build_array($transformations);
         // preserve url options
         $url_options = self::array_subset($options, self::$URL_KEYS);

diff --git a/src/Helpers.php b/src/Helpers.php
@@ -420,20 +420,22 @@ function generate_image_responsive_attributes($public_id, $attributes, $srcset_d
      */
     function cl_image_tag($public_id, $options = array())
     {
-        $original_options = null;
+        $original_options = $options;
 
-        $srcset_data = array_merge(
-            Cloudinary::config_get("srcset", []),
-            Cloudinary::option_consume($options, 'srcset', [])
-        );
+        $attributes = Cloudinary::option_consume($options, 'attributes', array());
 
-        if (!empty($srcset_data)) {
-            // Since cloudinary_url is destructive, we need to save a copy of original options passed to this function
-            $original_options =  Cloudinary::array_copy($options);
+        $srcset_option = Cloudinary::option_consume($options, 'srcset', []);
+
+        $srcset_data = [];
+
+        if (!is_array($srcset_option)) {
+            $attributes = array_merge(["srcset" => $srcset_option], $attributes);
+        }
+        else {
+            $srcset_data = array_merge(Cloudinary::config_get("srcset", []), $srcset_option);
         }
 
         $source = cloudinary_url_internal($public_id, $options);
-        $attributes = Cloudinary::option_consume($options, 'attributes', array());
         if (isset($options["html_width"])) {
             $options["width"] = Cloudinary::option_consume($options, "html_width");
         }
@@ -743,7 +745,7 @@ function cl_source_tag($public_id, $options = [])
 
         // `source` tag under `picture` tag uses `srcset` attribute for both `srcset` and `src` urls
         if (!array_key_exists("srcset", $attributes)) {
-            $attributes["srcset"] = cloudinary_url_internal($public_id, $options);
+            $attributes["srcset"] = cloudinary_url($public_id, $options);
         }
 
         $media_attr = generate_media_attr(Cloudinary::option_get($options, "media"));
@@ -770,8 +772,11 @@ function cl_picture_tag($public_id, $options = [], $sources = [])
         $public_id = Cloudinary::check_cloudinary_field($public_id, $options);
         Cloudinary::patch_fetch_format($options);
         foreach ($sources as $source) {
-            $source_options =  Cloudinary::array_copy($options);
-            $source_options = Cloudinary::chain_transformations($source_options, Cloudinary::option_get( $source, "transformation"));
+            $source_options =  $options;
+            $source_options = Cloudinary::chain_transformations(
+                $source_options,
+                Cloudinary::option_get($source, "transformation")
+            );
             $source_options["media"] = Cloudinary::array_subset($source, ['min_width', 'max_width']);
 
             $tag .= cl_source_tag($public_id, $source_options);

diff --git a/tests/CloudinaryTest.php b/tests/CloudinaryTest.php
@@ -11,7 +11,7 @@ class CloudinaryTest extends TestCase
     const DEFAULT_FETCH_PATH = 'http://res.cloudinary.com/test123/image/fetch/';
     const VIDEO_UPLOAD_PATH = 'http://res.cloudinary.com/test123/video/upload/';
     const TEST_ID = 'test';
-    
+
     const FETCH_URL = "http://cloudinary.com/images/logo.png";
 
     protected static $test_id = "test";
@@ -371,6 +371,12 @@ public function test_chain_transformations()
             $message
         );
 
+        $message = "Should handle empty list of chained transformations";
+        $actual_options = Cloudinary::chain_transformations($options, []);
+        $actual_transformation_str = Cloudinary::generate_transformation_string($actual_options);
+
+        $this->assertEquals("e_art:incognito", $actual_transformation_str, $message);
+
         $message = "Should handle empty options and empty list of chained transformations";
         $actual_options = Cloudinary::chain_transformations([], []);
         $actual_transformation_str = Cloudinary::generate_transformation_string($actual_options);

diff --git a/tests/TagTest.php b/tests/TagTest.php
@@ -79,7 +79,7 @@ public function test_cl_image_tag()
             $tag
         );
     }
-    
+
     /**
      * Should create a meta tag with client hints
      */
@@ -665,6 +665,29 @@ public function test_create_a_tag_with_custom_attributes_legacy_approach()
         $this->assertEquals($expected_custom_attributes_tag, $tag_with_custom_legacy_attribute);
     }
 
+    public function test_create_a_tag_with_legacy_srcset_attribute()
+    {
+        $srcset_attribute = ['srcset' =>'http://custom.srcset.attr/sample.jpg 100w'];
+        $tag_with_custom_srcset_attribute = cl_image_tag(
+            self::$public_id,
+            array_merge(
+                self::$common_image_options,
+                $srcset_attribute
+            )
+        );
+
+        $expected_custom_attributes_tag = self::get_expected_cl_image_tag(
+            self::$public_id,
+            self::$common_transformation_str,
+            '',
+            array(),
+            $srcset_attribute
+        );
+
+        $this->assertEquals($expected_custom_attributes_tag, $tag_with_custom_srcset_attribute);
+    }
+
+
     public function test_consume_custom_attributes_from_attributes_key()
     {
         $tag_with_custom_attribute = cl_image_tag(