FIX: Do not downsize or crop GIF images (#10989)
It was a problem because during this operation only the first frame is kept. This commit removes the alternative solution to check if a GIF image is animated.
diff --git a/Gemfile.lock b/Gemfile.lock
index 6c568cf..b805492 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -134,7 +134,7 @@ GEM
faraday-net_http (1.0.0)
fast_blank (1.0.0)
fast_xs (0.8.0)
- fastimage (2.2.0)
+ fastimage (2.2.1)
ffi (1.14.2)
fspath (3.1.2)
gc_tracer (1.5.1)
diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index 5bd0eac..9c2f4ab 100644
--- a/lib/upload_creator.rb
+++ b/lib/upload_creator.rb
@@ -126,7 +126,7 @@ class UploadCreator
if is_image
@upload.thumbnail_width, @upload.thumbnail_height = ImageSizer.resize(*@image_info.size)
@upload.width, @upload.height = @image_info.size
- @upload.animated = animated?(@file)
+ @upload.animated = FastImage.animated?(@file)
end
add_metadata!
@@ -177,25 +177,6 @@ class UploadCreator
end
end
- def animated?(file)
- OptimizedImage.ensure_safe_paths!(file.path)
-
- # TODO - find out why:
- # FastImage.animated?(@file)
- # doesn't seem to identify all animated gifs
- @frame_count ||= begin
- command = [
- "identify",
- "-format", "%n\\n",
- file.path
- ]
-
- frames = Discourse::Utils.execute_command(*command).to_i rescue 1
- end
-
- @frame_count > 1
- end
-
MIN_PIXELS_TO_CONVERT_TO_JPEG ||= 1280 * 720
def convert_png_to_jpeg?
@@ -286,13 +267,13 @@ class UploadCreator
end
def should_alter_quality?
- return false if animated?(@file)
+ return false if FastImage.animated?(@file)
@upload.target_image_quality(@file.path, SiteSetting.recompress_original_jpg_quality).present?
end
def should_downsize?
- max_image_size > 0 && filesize >= max_image_size
+ max_image_size > 0 && filesize >= max_image_size && !FastImage.animated?(@file)
end
def downsize!
@@ -302,14 +283,13 @@ class UploadCreator
from = @file.path
to = down_tempfile.path
- scale = (from =~ /\.GIF$/i) ? "0.5" : "50%"
OptimizedImage.ensure_safe_paths!(from, to)
OptimizedImage.downsize(
from,
to,
- scale,
+ "50%",
scale_image: true,
raise_on_error: true
)
@@ -371,7 +351,7 @@ class UploadCreator
end
def should_crop?
- return false if @opts[:type] == 'custom_emoji' && animated?(@file)
+ return false if ['profile_background', 'card_background', 'custom_emoji'].include?(@opts[:type]) && FastImage.animated?(@file)
TYPES_TO_CROP.include?(@opts[:type])
end
diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb
index f1f0787..23442bc 100644
--- a/spec/lib/upload_creator_spec.rb
+++ b/spec/lib/upload_creator_spec.rb
@@ -497,4 +497,20 @@ RSpec.describe UploadCreator do
end
end
end
+
+ describe '#should_downsize?' do
+ context "GIF image" do
+ let(:gif_file) { file_from_fixtures("animated.gif") }
+
+ before do
+ SiteSetting.max_image_size_kb = 1
+ end
+
+ it "is not downsized" do
+ creator = UploadCreator.new(gif_file, "animated.gif")
+ creator.extract_image_info!
+ expect(creator.should_downsize?).to eq(false)
+ end
+ end
+ end
end
GitHub sha: 499a5947