DEV: Remove gifsicle dependency (#10357)

Dependency on gifsicle, allow_animated_avatars and allow_animated_thumbnails
site settings were all removed. Animated GIF images are still allowed, but
the generated optimized images are no longer animated for those (which were
used for avatars and thumbnails).

The added 'animated' is populated by extracting information using FastImage.
This field was used to selectively reoptimize old animations. This process
happens in the background.
This commit is contained in:
Bianca Nenciu 2020-10-16 13:41:27 +03:00 committed by GitHub
parent 14cb587b7e
commit 43e52a7dc1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 85 additions and 64 deletions

View File

@ -65,7 +65,7 @@ jobs:
if: env.BUILD_TYPE != 'LINT'
run: |
sudo apt-get update
sudo apt-get -yqq install postgresql-client libpq-dev gifsicle jpegoptim optipng jhead
sudo apt-get -yqq install postgresql-client libpq-dev jpegoptim optipng jhead
wget -qO- https://raw.githubusercontent.com/discourse/discourse_docker/master/image/base/install-pngquant | sudo sh
- name: Update imagemagick

View File

@ -63,7 +63,6 @@ const ORIGINAL_SETTINGS = {
max_image_height: 500,
allow_profile_backgrounds: true,
allow_uploaded_avatars: true,
allow_animated_avatars: false,
tl1_requires_read_posts: 30,
enable_long_polling: true,
polling_interval: 3000,

View File

@ -189,7 +189,7 @@ class UserAvatarsController < ApplicationController
return if !upload
return upload if upload.extension == "svg"
upload.get_optimized_image(size, size, allow_animation: SiteSetting.allow_animated_avatars)
upload.get_optimized_image(size, size)
# TODO decide if we want to detach here
end

View File

@ -14,12 +14,7 @@ module Jobs
return unless upload = Upload.find_by(id: upload_id)
Discourse.avatar_sizes.each do |size|
OptimizedImage.create_for(
upload,
size,
size,
allow_animation: SiteSetting.allow_animated_avatars
)
OptimizedImage.create_for(upload, size, size)
end
end

View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
module Jobs
class UpdateAnimatedUploads < ::Jobs::Scheduled
every 1.hour
MAX_PROCESSED_GIF_IMAGES ||= 200
def execute(args)
Upload
.where("extension = 'gif' OR (extension IS NULL AND original_filename LIKE '%.gif')")
.where(animated: nil)
.limit(MAX_PROCESSED_GIF_IMAGES)
.each do |upload|
uri = Discourse.store.path_for(upload) || upload.url
upload.update!(animated: FastImage.animated?(uri))
upload.optimized_images.destroy_all if upload.animated
end
nil
end
end
end

View File

@ -234,20 +234,6 @@ class OptimizedImage < ActiveRecord::Base
})
end
def self.resize_instructions_animated(from, to, dimensions, opts = {})
ensure_safe_paths!(from, to)
resize_method = opts[:scale_image] ? "scale" : "resize-fit"
%W{
gifsicle
--colors=#{opts[:colors] || 256}
--#{resize_method} #{dimensions}
--optimize=3
--output #{to}
#{from}
}
end
def self.crop_instructions(from, to, dimensions, opts = {})
ensure_safe_paths!(from, to)
@ -270,19 +256,6 @@ class OptimizedImage < ActiveRecord::Base
}
end
def self.crop_instructions_animated(from, to, dimensions, opts = {})
ensure_safe_paths!(from, to)
%W{
gifsicle
--crop 0,0+#{dimensions}
--colors=#{opts[:colors] || 256}
--optimize=3
--output #{to}
#{from}
}
end
def self.downsize_instructions(from, to, dimensions, opts = {})
ensure_safe_paths!(from, to)
@ -302,10 +275,6 @@ class OptimizedImage < ActiveRecord::Base
}
end
def self.downsize_instructions_animated(from, to, dimensions, opts = {})
resize_instructions_animated(from, to, dimensions, opts)
end
def self.resize(from, to, width, height, opts = {})
optimize("resize", from, to, "#{width}x#{height}", opts)
end
@ -322,10 +291,6 @@ class OptimizedImage < ActiveRecord::Base
def self.optimize(operation, from, to, dimensions, opts = {})
method_name = "#{operation}_instructions"
if !!opts[:allow_animation] && (from =~ /\.GIF$/i)
method_name += "_animated"
end
instructions = self.public_send(method_name.to_sym, from, to, dimensions, opts)
convert_with(instructions, to, opts)
end

View File

@ -78,7 +78,6 @@ class Upload < ActiveRecord::Base
def create_thumbnail!(width, height, opts = nil)
return unless SiteSetting.create_thumbnails?
opts ||= {}
opts[:allow_animation] = SiteSetting.allow_animated_thumbnails
if get_optimized_image(width, height, opts)
save(validate: false)
@ -86,7 +85,9 @@ class Upload < ActiveRecord::Base
end
# this method attempts to correct old incorrect extensions
def get_optimized_image(width, height, opts)
def get_optimized_image(width, height, opts = nil)
opts ||= {}
if (!extension || extension.length == 0)
fix_image_extension
end
@ -461,11 +462,12 @@ end
# secure :boolean default(FALSE), not null
# access_control_post_id :bigint
# original_sha1 :string
# verified :boolean
# verification_status :integer default(1), not null
# animated :boolean
#
# Indexes
#
# idx_uploads_on_verification_status (verification_status)
# index_uploads_on_access_control_post_id (access_control_post_id)
# index_uploads_on_etag (etag)
# index_uploads_on_extension (lower((extension)::text))

View File

@ -2000,8 +2000,6 @@ en:
logout_redirect: "Location to redirect browser to after logout (eg: https://example.com/logout)"
allow_uploaded_avatars: "Allow users to upload custom profile pictures."
allow_animated_avatars: "Allow users to use animated gif profile pictures. WARNING: run the avatars:refresh rake task after changing this setting."
allow_animated_thumbnails: "Generates animated thumbnails of animated gifs."
default_avatars: "URLs to avatars that will be used by default for new users until they change them."
automatically_download_gravatars: "Download Gravatars for users upon account creation or email change."
digest_topics: "The maximum number of popular topics to display in the email summary."

View File

@ -1308,10 +1308,6 @@ files:
allow_uploaded_avatars:
client: true
default: true
allow_animated_avatars:
client: true
default: false
allow_animated_thumbnails: true
default_avatars:
default: ""
type: url_list

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddAnimatedToUploads < ActiveRecord::Migration[6.0]
def change
add_column :uploads, :animated, :boolean
end
end

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class DeleteAllowAnimatedAvatarsSiteSetting < ActiveRecord::Migration[6.0]
def up
execute "DELETE FROM site_settings WHERE name = 'allow_animated_avatars'"
end
def down
raise ActiveRecord::IrreversibleMigration.new
end
end

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class DeleteAllowAnimatedThumbnailsSiteSetting < ActiveRecord::Migration[6.0]
def up
execute "DELETE FROM site_settings WHERE name = 'allow_animated_thumbnails'"
end
def down
raise ActiveRecord::IrreversibleMigration.new
end
end

View File

@ -12,7 +12,7 @@ To get your Ubuntu 16.04 or 18.04 LTS install up and running to develop Discours
whoami > /tmp/username
sudo add-apt-repository ppa:chris-lea/redis-server
sudo apt-get -yqq update
sudo apt-get -yqq install software-properties-common vim curl expect debconf-utils git-core build-essential zlib1g-dev libssl-dev openssl libcurl4-openssl-dev libreadline6-dev libpcre3 libpcre3-dev imagemagick redis-server advancecomp gifsicle jhead jpegoptim libjpeg-turbo-progs optipng pngcrush pngquant gnupg2
sudo apt-get -yqq install software-properties-common vim curl expect debconf-utils git-core build-essential zlib1g-dev libssl-dev openssl libcurl4-openssl-dev libreadline6-dev libpcre3 libpcre3-dev imagemagick redis-server advancecomp jhead jpegoptim libjpeg-turbo-progs optipng pngcrush pngquant gnupg2
# Ruby
curl -sSL https://rvm.io/mpapis.asc | gpg2 --import -

View File

@ -223,7 +223,7 @@ In addition to ImageMagick we also need to install some other image related
software:
```sh
brew install gifsicle jpegoptim optipng jhead
brew install jpegoptim optipng jhead
npm install -g svgo
```

View File

@ -126,6 +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 = FastImage.animated?(@file)
end
add_metadata!
@ -279,7 +280,6 @@ class UploadCreator
from,
to,
scale,
allow_animation: allow_animation,
scale_image: true,
raise_on_error: true
)
@ -351,17 +351,17 @@ class UploadCreator
case @opts[:type]
when "avatar"
width = height = Discourse.avatar_sizes.max
OptimizedImage.resize(@file.path, @file.path, width, height, filename: filename_with_correct_ext, allow_animation: allow_animation)
OptimizedImage.resize(@file.path, @file.path, width, height, filename: filename_with_correct_ext)
when "profile_background"
max_width = 850 * max_pixel_ratio
width, height = ImageSizer.resize(@image_info.size[0], @image_info.size[1], max_width: max_width, max_height: max_width)
OptimizedImage.downsize(@file.path, @file.path, "#{width}x#{height}\>", filename: filename_with_correct_ext, allow_animation: allow_animation)
OptimizedImage.downsize(@file.path, @file.path, "#{width}x#{height}\>", filename: filename_with_correct_ext)
when "card_background"
max_width = 590 * max_pixel_ratio
width, height = ImageSizer.resize(@image_info.size[0], @image_info.size[1], max_width: max_width, max_height: max_width)
OptimizedImage.downsize(@file.path, @file.path, "#{width}x#{height}\>", filename: filename_with_correct_ext, allow_animation: allow_animation)
OptimizedImage.downsize(@file.path, @file.path, "#{width}x#{height}\>", filename: filename_with_correct_ext)
when "custom_emoji"
OptimizedImage.downsize(@file.path, @file.path, "100x100\>", filename: filename_with_correct_ext, allow_animation: allow_animation)
OptimizedImage.downsize(@file.path, @file.path, "100x100\>", filename: filename_with_correct_ext)
end
extract_image_info!
@ -401,10 +401,6 @@ class UploadCreator
@image_info.size&.reduce(:*).to_i
end
def allow_animation
@allow_animation ||= @opts[:type] == "avatar" ? SiteSetting.allow_animated_avatars : SiteSetting.allow_animated_thumbnails
end
def svg_allowlist_xpath
@@svg_allowlist_xpath ||= "//*[#{ALLOWED_SVG_ELEMENTS.map { |e| "name()!='#{e}'" }.join(" and ") }]"
end

View File

@ -0,0 +1,18 @@
# frozen_string_literal: true
require 'rails_helper'
describe Jobs::UpdateAnimatedUploads do
let!(:upload) { Fabricate(:upload) }
let!(:gif_upload) { Fabricate(:upload, extension: "gif") }
it "affects only GIF uploads" do
url = Discourse.store.path_for(gif_upload) || gif_upload.url
FastImage.expects(:animated?).with(url).returns(true).once
described_class.new.execute({})
expect(upload.reload.animated).to eq(nil)
expect(gif_upload.reload.animated).to eq(true)
end
end