diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3eb5f267b77..8e4c2b6ac15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/app/assets/javascripts/discourse/tests/helpers/site-settings.js b/app/assets/javascripts/discourse/tests/helpers/site-settings.js index ba52f3f9b53..d84b53e4c73 100644 --- a/app/assets/javascripts/discourse/tests/helpers/site-settings.js +++ b/app/assets/javascripts/discourse/tests/helpers/site-settings.js @@ -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, diff --git a/app/controllers/user_avatars_controller.rb b/app/controllers/user_avatars_controller.rb index be9118855f1..583fc81e653 100644 --- a/app/controllers/user_avatars_controller.rb +++ b/app/controllers/user_avatars_controller.rb @@ -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 diff --git a/app/jobs/regular/create_avatar_thumbnails.rb b/app/jobs/regular/create_avatar_thumbnails.rb index 540c5803d5c..51988a47642 100644 --- a/app/jobs/regular/create_avatar_thumbnails.rb +++ b/app/jobs/regular/create_avatar_thumbnails.rb @@ -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 diff --git a/app/jobs/scheduled/update_animated_uploads.rb b/app/jobs/scheduled/update_animated_uploads.rb new file mode 100644 index 00000000000..2938911b75a --- /dev/null +++ b/app/jobs/scheduled/update_animated_uploads.rb @@ -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 diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 589cc5f1562..42a62b98f7e 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -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 diff --git a/app/models/upload.rb b/app/models/upload.rb index 36fc6da0a27..27d8825fdd2 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -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)) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2ef198f219b..16455b5f21b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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." diff --git a/config/site_settings.yml b/config/site_settings.yml index 42722d958f1..0483fed283e 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -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 diff --git a/db/migrate/20200707183007_add_animated_to_uploads.rb b/db/migrate/20200707183007_add_animated_to_uploads.rb new file mode 100644 index 00000000000..3dec32c836b --- /dev/null +++ b/db/migrate/20200707183007_add_animated_to_uploads.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddAnimatedToUploads < ActiveRecord::Migration[6.0] + def change + add_column :uploads, :animated, :boolean + end +end diff --git a/db/migrate/20200714105026_delete_allow_animated_avatars_site_setting.rb b/db/migrate/20200714105026_delete_allow_animated_avatars_site_setting.rb new file mode 100644 index 00000000000..b1ab2f56a1a --- /dev/null +++ b/db/migrate/20200714105026_delete_allow_animated_avatars_site_setting.rb @@ -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 diff --git a/db/migrate/20200714105027_delete_allow_animated_thumbnails_site_setting.rb b/db/migrate/20200714105027_delete_allow_animated_thumbnails_site_setting.rb new file mode 100644 index 00000000000..e0e98f8289f --- /dev/null +++ b/db/migrate/20200714105027_delete_allow_animated_thumbnails_site_setting.rb @@ -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 diff --git a/docs/DEVELOPER-ADVANCED.md b/docs/DEVELOPER-ADVANCED.md index 59527a289a5..95f0295f4e2 100644 --- a/docs/DEVELOPER-ADVANCED.md +++ b/docs/DEVELOPER-ADVANCED.md @@ -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 - diff --git a/docs/DEVELOPMENT-OSX-NATIVE.md b/docs/DEVELOPMENT-OSX-NATIVE.md index 93721f51f39..f34cbcc922b 100644 --- a/docs/DEVELOPMENT-OSX-NATIVE.md +++ b/docs/DEVELOPMENT-OSX-NATIVE.md @@ -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 ``` diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index 69985a947f4..bdfb26cdf10 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -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 diff --git a/spec/jobs/update_animated_uploads_spec.rb b/spec/jobs/update_animated_uploads_spec.rb new file mode 100644 index 00000000000..65e2e8e39e7 --- /dev/null +++ b/spec/jobs/update_animated_uploads_spec.rb @@ -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