FIX: show lightbox for small images (#29140)

We want to allow lightboxing of smaller images, even if they are below the minimum size for image thumbnail generation.

This change sets a minimum threshold of 100 x 100 pixels for triggering the lightbox.

---------

Co-authored-by: Régis Hanol <regis@hanol.fr>

lib/cooked_post_processor.rb

@@ -8,6 +8,8 @@ class CookedPostProcessor
 
   LIGHTBOX_WRAPPER_CSS_CLASS = "lightbox-wrapper"
   GIF_SOURCES_REGEXP = %r{(giphy|tenor)\.com/}
+  MIN_LIGHTBOX_WIDTH = 100
+  MIN_LIGHTBOX_HEIGHT = 100
 
   attr_reader :cooking_options, :doc
 
@@ -181,10 +183,8 @@ class CookedPostProcessor
       img.add_class("animated")
     end
 
-    if original_width <= SiteSetting.max_image_width &&
-         original_height <= SiteSetting.max_image_height
-      return
-    end
+    generate_thumbnail =
+      original_width > SiteSetting.max_image_width || original_height > SiteSetting.max_image_height
 
     user_width, user_height = [original_width, original_height] if user_width.to_i <= 0 &&
       user_height.to_i <= 0
@@ -201,24 +201,26 @@ class CookedPostProcessor
     end
 
     if upload.present?
-      upload.create_thumbnail!(width, height, crop: crop)
+      if generate_thumbnail
+        upload.create_thumbnail!(width, height, crop: crop)
 
-      each_responsive_ratio do |ratio|
-        resized_w = (width * ratio).to_i
-        resized_h = (height * ratio).to_i
+        each_responsive_ratio do |ratio|
+          resized_w = (width * ratio).to_i
+          resized_h = (height * ratio).to_i
 
-        if upload.width && resized_w <= upload.width
-          upload.create_thumbnail!(resized_w, resized_h, crop: crop)
+          if upload.width && resized_w <= upload.width
+            upload.create_thumbnail!(resized_w, resized_h, crop: crop)
+          end
         end
       end
 
       return if upload.animated?
 
       if img.ancestors(".onebox, .onebox-body").blank? && !img.classes.include?("onebox")
-        add_lightbox!(img, original_width, original_height, upload, cropped: crop)
+        add_lightbox!(img, original_width, original_height, upload)
       end
 
-      optimize_image!(img, upload, cropped: crop)
+      optimize_image!(img, upload, cropped: crop) if generate_thumbnail
     end
   end
 
@@ -265,15 +267,18 @@ class CookedPostProcessor
     end
   end
 
-  def add_lightbox!(img, original_width, original_height, upload, cropped: false)
+  def add_lightbox!(img, original_width, original_height, upload)
+    return if original_width < MIN_LIGHTBOX_WIDTH || original_height < MIN_LIGHTBOX_HEIGHT
+
     # first, create a div to hold our lightbox
     lightbox = create_node("div", LIGHTBOX_WRAPPER_CSS_CLASS)
     img.add_next_sibling(lightbox)
     lightbox.add_child(img)
 
     # then, the link to our larger image
-    src_url = Upload.secure_uploads_url?(img["src"]) ? upload&.url : img["src"]
-    src = UrlHelper.cook_url(src_url || img["src"], secure: @should_secure_uploads)
+    src_url = Upload.secure_uploads_url?(img["src"]) ? upload&.url || img["src"] : img["src"]
+    src = UrlHelper.cook_url(src_url, secure: @should_secure_uploads)
+
     a = create_link_node("lightbox", src)
     img.add_next_sibling(a)
 

spec/lib/cooked_post_processor_spec.rb

@@ -401,6 +401,40 @@ RSpec.describe CookedPostProcessor do
         end
       end
 
+      context "with small images" do
+        fab!(:upload) { Fabricate(:image_upload, width: 150, height: 150) }
+        fab!(:post) { Fabricate(:post, user: user_with_auto_groups, raw: <<~HTML) }
+          <img src="#{upload.url}">
+          HTML
+        let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
+
+        before { SiteSetting.create_thumbnails = true }
+
+        it "shows the lightbox when both dimensions are above the minimum" do
+          cpp.post_process
+          expect(cpp.html).to match(/<div class="lightbox-wrapper">/)
+        end
+
+        it "does not show lightbox when both dimensions are below the minimum" do
+          upload.update!(width: 50, height: 50)
+          cpp.post_process
+
+          expect(cpp.html).not_to match(/<div class="lightbox-wrapper">/)
+        end
+
+        it "does not show lightbox when either dimension is below the minimum" do
+          upload.update!(width: 50, height: 150)
+          cpp.post_process
+
+          expect(cpp.html).not_to match(/<div class="lightbox-wrapper">/)
+        end
+
+        it "does not create thumbnails for small images" do
+          Upload.any_instance.expects(:create_thumbnail!).never
+          cpp.post_process
+        end
+      end
+
       context "with large images" do
         fab!(:upload) { Fabricate(:image_upload, width: 1750, height: 2000) }
 

spec/lib/email/sender_spec.rb

@@ -636,7 +636,7 @@ RSpec.describe Email::Sender do
           @secure_image_2 =
             UploadCreator.new(
               file_from_fixtures("logo-dev.png", "images"),
-              "secuere_logo_2.png",
+              "secure_logo_2.png",
             ).create_for(Discourse.system_user.id)
           @secure_image_2.update_secure_status(override: true)
           @secure_image_2.update(access_control_post_id: reply.id)
@@ -666,7 +666,7 @@ RSpec.describe Email::Sender do
 
           @secure_image_2 =
             UploadCreator.new(
-              file_from_fixtures("logo-dev.png", "images"),
+              file_from_fixtures("logo.png", "images"),
               "something-cool.png",
             ).create_for(Discourse.system_user.id)
           @secure_image_2.update_secure_status(override: true)
@@ -701,6 +701,7 @@ RSpec.describe Email::Sender do
           expect(summary.attachments.map(&:filename)).to include(
             *[@secure_image, @secure_image_2, @secure_image_3].map(&:original_filename),
           )
+          expect(summary.attachments.size).to eq(3)
           expect(summary.to_s.scan("Content-Type: text/html;").length).to eq(1)
           expect(summary.to_s.scan("Content-Type: text/plain;").length).to eq(1)
           expect(summary.to_s.scan(/cid:[\w\-@.]+/).length).to eq(3)