FIX: don't use srcset on cropped thumbnails

lib/cooked_post_processor.rb

@@ -309,7 +309,7 @@ class CookedPostProcessor
       end
     end
 
-    add_lightbox!(img, original_width, original_height, upload)
+    add_lightbox!(img, original_width, original_height, upload, cropped: crop)
   end
 
   def is_a_hyperlink?(img)
@@ -330,7 +330,7 @@ class CookedPostProcessor
       .each { |r| yield r if r > 1 }
   end
 
-  def add_lightbox!(img, original_width, original_height, upload = nil)
+  def add_lightbox!(img, original_width, original_height, upload, cropped: false)
     # first, create a div to hold our lightbox
     lightbox = create_node("div", "lightbox-wrapper")
     img.add_next_sibling(lightbox)
@@ -352,7 +352,7 @@ class CookedPostProcessor
     if upload
       thumbnail = upload.thumbnail(w, h)
       if thumbnail && thumbnail.filesize.to_i < upload.filesize
-        img["src"] = upload.thumbnail(w, h).url
+        img["src"] = thumbnail.url
 
         srcset = +""
 
@@ -360,19 +360,16 @@ class CookedPostProcessor
           resized_w = (w * ratio).to_i
           resized_h = (h * ratio).to_i
 
-          if upload.width && resized_w > upload.width
+          if !cropped && upload.width && resized_w > upload.width
             cooked_url = UrlHelper.cook_url(upload.url)
-            srcset << ", #{cooked_url} #{ratio}x"
-          else
-            if t = upload.thumbnail(resized_w, resized_h)
-              cooked_url = UrlHelper.cook_url(t.url)
-              srcset << ", #{cooked_url} #{ratio}x"
-            end
+            srcset << ", #{cooked_url} #{ratio.to_s.sub(/\.0$/, "")}x"
+          elsif t = upload.thumbnail(resized_w, resized_h)
+            cooked_url = UrlHelper.cook_url(t.url)
+            srcset << ", #{cooked_url} #{ratio.to_s.sub(/\.0$/, "")}x"
           end
+
+          img["srcset"] = "#{UrlHelper.cook_url(img["src"])}#{srcset}" if srcset.present?
         end
-
-        img["srcset"] = "#{UrlHelper.cook_url(img["src"])}#{srcset}" if srcset.length > 0
-
       else
         img["src"] = upload.url
       end

spec/components/cooked_post_processor_spec.rb

@@ -58,10 +58,10 @@ describe CookedPostProcessor do
     end
 
     context "responsive images" do
+
+      before { SiteSetting.responsive_post_image_sizes = "1|1.5|3" }
+
       it "includes responsive images on demand" do
-
-        SiteSetting.responsive_post_image_sizes = "1|1.5|3"
-
         upload = Fabricate(:upload, width: 2000, height: 1500, filesize: 10000)
         post = Fabricate(:post, raw: "hello <img src='#{upload.url}'>")
 
@@ -93,8 +93,29 @@ describe CookedPostProcessor do
         cpp.post_process_images
 
         # 1.5x is skipped cause we have a missing thumb
-        expect(cpp.html).to include('srcset="http://a.b.c/666x500.jpg, http://a.b.c/1998x1500.jpg 3.0x"')
+        expect(cpp.html).to include('srcset="http://a.b.c/666x500.jpg, http://a.b.c/1998x1500.jpg 3x"')
+      end
 
+      it "doesn't include response images for cropped images" do
+        upload = Fabricate(:upload, width: 200, height: 4000, filesize: 12345)
+        post = Fabricate(:post, raw: "hello <img src='#{upload.url}'>")
+
+        # fake some optimized images
+        OptimizedImage.create!(
+          url: 'http://a.b.c/200x500.jpg',
+          width: 200,
+          height: 500,
+          upload_id: upload.id,
+          sha1: SecureRandom.hex,
+          extension: '.jpg',
+          filesize: 500
+        )
+
+        cpp = CookedPostProcessor.new(post)
+        cpp.add_to_size_cache(upload.url, 200, 4000)
+        cpp.post_process_images
+
+        expect(cpp.html).to_not include('srcset="')
       end
     end