FIX: Ensure 'crop' always returns requested dimensions (#16437)

Previously, 'crop' would resize the image to have the requested width, then crop the height to the requested value. This works when cropping images vertically, but not when cropping them horizontally.

For example, trying to crop a 500x500 image to 200x500 was actually resulting in a 200x200 image. Having an OptimizedImage with width/height columns mismatching the actual OptimizedImage width/height causes some unusual issues.

This commit ensures that a call to `OptimizedImage.crop(from, to, width, height)` will always return an image of the requested width/height. The `w x h^` syntax defines minimum width/height, while maintaining aspect ratio.

app/models/optimized_image.rb

@@ -252,7 +252,7 @@ class OptimizedImage < ActiveRecord::Base
       -auto-orient
       -gravity north
       -background transparent
-      -#{thumbnail_or_resize} #{opts[:width]}
+      -#{thumbnail_or_resize} #{dimensions}^
       -crop #{dimensions}+0+0
       -unsharp 2x0.5+0.7+0
       -interlace none
@@ -290,7 +290,6 @@ class OptimizedImage < ActiveRecord::Base
   end
 
   def self.crop(from, to, width, height, opts = {})
-    opts[:width] = width
     optimize("crop", from, to, "#{width}x#{height}", opts)
   end
 

spec/models/optimized_image_spec.rb

@@ -7,22 +7,23 @@ describe OptimizedImage do
   describe '.crop' do
     it 'should produce cropped images (requires ImageMagick 7)' do
       tmp_path = "/tmp/cropped.png"
+      desired_width = 5
+      desired_height = 5
 
       begin
         OptimizedImage.crop(
           "#{Rails.root}/spec/fixtures/images/logo.png",
           tmp_path,
-          5,
-          5
+          desired_width,
+          desired_height
         )
 
-        # we don't want to deal with something new here every time image magick
-        # is upgraded or pngquant is upgraded, lets just test the basics ...
-        # cropped image should be less than 120 bytes
+        w, h = FastImage.size(tmp_path)
+        expect(w).to eq(desired_width)
+        expect(h).to eq(desired_height)
 
         cropped_size = File.size(tmp_path)
-
-        expect(cropped_size).to be < 120
+        expect(cropped_size).to be < 200
         expect(cropped_size).to be > 50
 
       ensure
@@ -30,6 +31,52 @@ describe OptimizedImage do
       end
     end
 
+    it 'should correctly crop images vertically' do
+      tmp_path = "/tmp/cropped.png"
+      desired_width = 100
+      desired_height = 66
+
+      begin
+        OptimizedImage.crop(
+          "#{Rails.root}/spec/fixtures/images/logo.png", # 244x66px
+          tmp_path,
+          desired_width,
+          desired_height
+        )
+
+        w, h = FastImage.size(tmp_path)
+
+        expect(w).to eq(desired_width)
+        expect(h).to eq(desired_height)
+
+      ensure
+        File.delete(tmp_path) if File.exist?(tmp_path)
+      end
+    end
+
+    it 'should correctly crop images horizontally' do
+      tmp_path = "/tmp/cropped.png"
+      desired_width = 244
+      desired_height = 500
+
+      begin
+        OptimizedImage.crop(
+          "#{Rails.root}/spec/fixtures/images/logo.png", # 244x66px
+          tmp_path,
+          desired_width,
+          desired_height
+        )
+
+        w, h = FastImage.size(tmp_path)
+
+        expect(w).to eq(desired_width)
+        expect(h).to eq(desired_height)
+
+      ensure
+        File.delete(tmp_path) if File.exist?(tmp_path)
+      end
+    end
+
     describe ".resize_instructions" do
       let(:image) { "#{Rails.root}/spec/fixtures/images/logo.png" }