UX: Use dominant color as image loading placeholder (#18248)

We previously had a system which would generate a 10x10px preview of images and add their URLs in a data-small-upload attribute. The client would then use that as the background-image of the `<img>` element. This works reasonably well on fast connections, but on slower connections it can take a few seconds for the placeholders to appear. The act of loading the placeholders can also break or delay the loading of the 'real' images.

This commit replaces the placeholder logic with a new approach. Instead of a 10x10px preview, we use imagemagick to calculate the average color of an image and store it in the database. The hex color value then added as a `data-dominant-color` attribute on the `<img>` element, and the client can use this as a `background-color` on the element while the real image is loading. That means no extra HTTP request is required, and so the placeholder color can appear instantly.

Dominant color will be calculated:
1. When a new upload is created
2. During a post rebake, if the dominant color is missing from an upload, it will be calculated and stored
3. Every 15 minutes, 25 old upload records are fetched and their dominant color calculated and stored. (part of the existing PeriodicalUpdates job)

Existing posts will continue to use the old 10x10px placeholder system until they are next rebaked
This commit is contained in:
David Taylor 2022-09-20 10:28:17 +01:00 committed by GitHub
parent e7091d2f59
commit d0243f741e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 232 additions and 78 deletions

View File

@ -1,14 +1,3 @@
// Min size in pixels for consideration for lazy loading
const MINIMUM_SIZE = 150;
function forEachImage(post, callback) {
post.querySelectorAll("img").forEach((img) => {
if (img.width >= MINIMUM_SIZE && img.height >= MINIMUM_SIZE) {
callback(img);
}
});
}
function isLoaded(img) { function isLoaded(img) {
// In Safari, img.complete sometimes returns true even when the image is not loaded. // In Safari, img.complete sometimes returns true even when the image is not loaded.
// naturalHeight seems to be a more reliable check // naturalHeight seems to be a more reliable check
@ -16,40 +5,48 @@ function isLoaded(img) {
} }
export function nativeLazyLoading(api) { export function nativeLazyLoading(api) {
api.decorateCookedElement(
(post) =>
post.querySelectorAll("img").forEach((img) => (img.loading = "lazy")),
{
id: "discourse-lazy-load",
}
);
api.decorateCookedElement( api.decorateCookedElement(
(post) => { (post) => {
const siteSettings = api.container.lookup("service:site-settings"); const siteSettings = api.container.lookup("service:site-settings");
forEachImage(post, (img) => { post.querySelectorAll("img").forEach((img) => {
img.loading = "lazy"; // Support for smallUpload should be maintained until Post::BAKED_VERSION is bumped higher than 2
const { smallUpload, dominantColor } = img.dataset;
if (siteSettings.secure_media) { if (siteSettings.secure_media && smallUpload) {
// Secure media requests go through the app. In topics with many images, // Secure media requests go through the app. In topics with many images,
// this makes it very easy to hit rate limiters. Skipping the low-res // this makes it very easy to hit rate limiters. Skipping the low-res
// placeholders reduces the chance of this problem occuring. // placeholders reduces the chance of this problem occuring.
return; return;
} }
if (img.dataset.smallUpload) { if ((smallUpload || dominantColor) && !isLoaded(img)) {
if (!isLoaded(img)) { if (!img.onload) {
if (!img.onload) { img.onload = () => {
img.onload = () => { img.style.removeProperty("background-image");
img.style.removeProperty("background-image"); img.style.removeProperty("background-size");
img.style.removeProperty("background-size"); img.style.removeProperty("background-color");
}; };
} }
img.style.setProperty( if (smallUpload) {
"background-image", img.style.setProperty("background-image", `url(${smallUpload})`);
`url(${img.dataset.smallUpload})`
);
img.style.setProperty("background-size", "cover"); img.style.setProperty("background-size", "cover");
} else {
img.style.setProperty("background-color", `#${dominantColor}`);
} }
} }
}); });
}, },
{ {
onlyStream: true,
id: "discourse-lazy-load-after-adopt", id: "discourse-lazy-load-after-adopt",
afterAdopt: true, afterAdopt: true,
} }

View File

@ -48,6 +48,8 @@ module Jobs
Category.auto_bump_topic! Category.auto_bump_topic!
Upload.backfill_dominant_colors!(25)
nil nil
end end

View File

@ -10,6 +10,7 @@ class Upload < ActiveRecord::Base
SEEDED_ID_THRESHOLD = 0 SEEDED_ID_THRESHOLD = 0
URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/ URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/
MAX_IDENTIFY_SECONDS = 5 MAX_IDENTIFY_SECONDS = 5
DOMINANT_COLOR_COMMAND_TIMEOUT_SECONDS = 5
belongs_to :user belongs_to :user
belongs_to :access_control_post, class_name: 'Post' belongs_to :access_control_post, class_name: 'Post'
@ -316,6 +317,71 @@ class Upload < ActiveRecord::Base
get_dimension(:thumbnail_height) get_dimension(:thumbnail_height)
end end
def dominant_color(calculate_if_missing: false)
val = read_attribute(:dominant_color)
if val.nil? && calculate_if_missing
calculate_dominant_color!
read_attribute(:dominant_color)
else
val
end
end
def calculate_dominant_color!(local_path = nil)
color = nil
if !FileHelper.is_supported_image?("image.#{extension}") || extension == "svg"
color = ""
end
if color.nil?
local_path ||=
if local?
Discourse.store.path_for(self)
else
Discourse.store.download(self).path
end
color = begin
data = Discourse::Utils.execute_command(
"nice",
"-n",
"10",
"convert",
local_path,
"-resize",
"1x1",
"-define",
"histogram:unique-colors=true",
"-format",
"%c",
"histogram:info:",
timeout: DOMINANT_COLOR_COMMAND_TIMEOUT_SECONDS
)
# Output format:
# 1: (110.873,116.226,93.8821) #6F745E srgb(43.4798%,45.5789%,36.8165%)
color = data[/#([0-9A-F]{6})/, 1]
raise "Calculated dominant color but unable to parse output:\n#{data}" if color.nil?
color
rescue Discourse::Utils::CommandError => e
# Timeout or unable to parse image
# This can happen due to bad user input - ignore and save
# an empty string to prevent re-evaluation
""
end
end
if persisted?
self.update_column(:dominant_color, color)
else
self.dominant_color = color
end
end
def target_image_quality(local_path, test_quality) def target_image_quality(local_path, test_quality)
@file_quality ||= Discourse::Utils.execute_command("identify", "-format", "%Q", local_path, timeout: MAX_IDENTIFY_SECONDS).to_i rescue 0 @file_quality ||= Discourse::Utils.execute_command("identify", "-format", "%Q", local_path, timeout: MAX_IDENTIFY_SECONDS).to_i rescue 0
@ -522,6 +588,12 @@ class Upload < ActiveRecord::Base
Upload.where(sha1: sha1s.uniq).pluck(:id) Upload.where(sha1: sha1s.uniq).pluck(:id)
end end
def self.backfill_dominant_colors!(count)
Upload.where(dominant_color: nil).order("id desc").first(count).each do |upload|
upload.calculate_dominant_color!
end
end
private private
def short_url_basename def short_url_basename
@ -553,10 +625,11 @@ end
# secure :boolean default(FALSE), not null # secure :boolean default(FALSE), not null
# access_control_post_id :bigint # access_control_post_id :bigint
# original_sha1 :string # original_sha1 :string
# verification_status :integer default(1), not null
# animated :boolean # animated :boolean
# verification_status :integer default(1), not null
# security_last_changed_at :datetime # security_last_changed_at :datetime
# security_last_changed_reason :string # security_last_changed_reason :string
# dominant_color :text
# #
# Indexes # Indexes
# #
@ -564,6 +637,7 @@ end
# index_uploads_on_access_control_post_id (access_control_post_id) # index_uploads_on_access_control_post_id (access_control_post_id)
# index_uploads_on_etag (etag) # index_uploads_on_etag (etag)
# index_uploads_on_extension (lower((extension)::text)) # index_uploads_on_extension (lower((extension)::text))
# index_uploads_on_id (id) WHERE (dominant_color IS NULL)
# index_uploads_on_id_and_url (id,url) # index_uploads_on_id_and_url (id,url)
# index_uploads_on_original_sha1 (original_sha1) # index_uploads_on_original_sha1 (original_sha1)
# index_uploads_on_sha1 (sha1) UNIQUE # index_uploads_on_sha1 (sha1) UNIQUE

View File

@ -13,7 +13,8 @@ class UploadSerializer < ApplicationSerializer
:short_url, :short_url,
:short_path, :short_path,
:retain_hours, :retain_hours,
:human_filesize :human_filesize,
:dominant_color
def url def url
object.for_site_setting ? object.url : UrlHelper.cook_url(object.url, secure: SiteSetting.secure_media? && object.secure) object.for_site_setting ? object.url : UrlHelper.cook_url(object.url, secure: SiteSetting.secure_media? && object.secure)

View File

@ -0,0 +1,8 @@
# frozen_string_literal: true
class AddDominantColorToUploads < ActiveRecord::Migration[7.0]
def change
add_column :uploads, :dominant_color, :text, limit: 6, null: true
add_index :uploads, :id, where: "dominant_color IS NULL"
end
end

View File

@ -7,8 +7,6 @@ class CookedPostProcessor
include CookedProcessorMixin include CookedProcessorMixin
LIGHTBOX_WRAPPER_CSS_CLASS = "lightbox-wrapper" LIGHTBOX_WRAPPER_CSS_CLASS = "lightbox-wrapper"
LOADING_SIZE = 10
LOADING_COLORS = 32
GIF_SOURCES_REGEXP = /(giphy|tenor)\.com\// GIF_SOURCES_REGEXP = /(giphy|tenor)\.com\//
attr_reader :cooking_options, :doc attr_reader :cooking_options, :doc
@ -32,7 +30,7 @@ class CookedPostProcessor
@has_oneboxes = post.post_analyzer.found_oneboxes? @has_oneboxes = post.post_analyzer.found_oneboxes?
@size_cache = {} @size_cache = {}
@disable_loading_image = !!opts[:disable_loading_image] @disable_dominant_color = !!opts[:disable_dominant_color]
@omit_nofollow = post.omit_nofollow? @omit_nofollow = post.omit_nofollow?
end end
@ -193,10 +191,6 @@ class CookedPostProcessor
end end
end end
unless @disable_loading_image
upload.create_thumbnail!(LOADING_SIZE, LOADING_SIZE, format: 'png', colors: LOADING_COLORS)
end
return if upload.animated? return if upload.animated?
if img.ancestors('.onebox, .onebox-body, .quote').blank? && !img.classes.include?("onebox") if img.ancestors('.onebox, .onebox-body, .quote').blank? && !img.classes.include?("onebox")
@ -207,10 +201,6 @@ class CookedPostProcessor
end end
end end
def loading_image(upload)
upload.thumbnail(LOADING_SIZE, LOADING_SIZE)
end
def each_responsive_ratio def each_responsive_ratio
SiteSetting SiteSetting
.responsive_post_image_sizes .responsive_post_image_sizes
@ -223,7 +213,7 @@ class CookedPostProcessor
def optimize_image!(img, upload, cropped: false) def optimize_image!(img, upload, cropped: false)
w, h = img["width"].to_i, img["height"].to_i w, h = img["width"].to_i, img["height"].to_i
# note: optimize_urls cooks the src and data-small-upload further after this # note: optimize_urls cooks the src further after this
thumbnail = upload.thumbnail(w, h) thumbnail = upload.thumbnail(w, h)
if thumbnail && thumbnail.filesize.to_i < upload.filesize if thumbnail && thumbnail.filesize.to_i < upload.filesize
img["src"] = thumbnail.url img["src"] = thumbnail.url
@ -248,8 +238,8 @@ class CookedPostProcessor
img["src"] = upload.url img["src"] = upload.url
end end
if small_upload = loading_image(upload) if !@disable_dominant_color && (color = upload.dominant_color(calculate_if_missing: true).presence)
img["data-small-upload"] = small_upload.url img["data-dominant-color"] = color
end end
end end
@ -329,7 +319,7 @@ class CookedPostProcessor
end end
end end
%w{src data-small-upload}.each do |selector| %w{src}.each do |selector|
@doc.css("img[#{selector}]").each do |img| @doc.css("img[#{selector}]").each do |img|
custom_emoji = img["class"]&.include?("emoji-custom") && Emoji.custom?(img["title"]) custom_emoji = img["class"]&.include?("emoji-custom") && Emoji.custom?(img["title"])
img[selector] = UrlHelper.cook_url( img[selector] = UrlHelper.cook_url(

View File

@ -109,6 +109,16 @@ module Discourse
nil nil
end end
class CommandError < RuntimeError
attr_reader :status, :stdout, :stderr
def initialize(message, status: nil, stdout: nil, stderr: nil)
super(message)
@status = status
@stdout = stdout
@stderr = stderr
end
end
private private
class CommandRunner class CommandRunner
@ -145,7 +155,12 @@ module Discourse
if !status.exited? || !success_status_codes.include?(status.exitstatus) if !status.exited? || !success_status_codes.include?(status.exitstatus)
failure_message = "#{failure_message}\n" if !failure_message.blank? failure_message = "#{failure_message}\n" if !failure_message.blank?
raise "#{caller[0]}: #{failure_message}#{stderr}" raise CommandError.new(
"#{caller[0]}: #{failure_message}#{stderr}",
stdout: stdout,
stderr: stderr,
status: status
)
end end
stdout stdout

View File

@ -168,6 +168,7 @@ class UploadCreator
@upload.thumbnail_width, @upload.thumbnail_height = ImageSizer.resize(w, h) @upload.thumbnail_width, @upload.thumbnail_height = ImageSizer.resize(w, h)
@upload.width, @upload.height = w, h @upload.width, @upload.height = w, h
@upload.animated = animated? @upload.animated = animated?
@upload.calculate_dominant_color!(@file.path)
end end
add_metadata! add_metadata!

View File

@ -22,9 +22,11 @@ Fabricator(:upload) do
end end
Fabricator(:image_upload, from: :upload) do Fabricator(:image_upload, from: :upload) do
after_create do |upload| transient color: "white"
after_create do |upload, transients|
file = Tempfile.new(['fabricated', '.png']) file = Tempfile.new(['fabricated', '.png'])
`convert -size #{upload.width}x#{upload.height} xc:white "#{file.path}"` `convert -size #{upload.width}x#{upload.height} xc:#{transients[:color]} "#{file.path}"`
upload.url = Discourse.store.store_upload(file, upload) upload.url = Discourse.store.store_upload(file, upload)
upload.sha1 = Upload.generate_digest(file.path) upload.sha1 = Upload.generate_digest(file.path)

View File

@ -14,7 +14,7 @@ RSpec.describe CookedPostProcessor do
RAW RAW
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
let(:post_process) { sequence("post_process") } let(:post_process) { sequence("post_process") }
it "post process in sequence" do it "post process in sequence" do
@ -258,7 +258,7 @@ RSpec.describe CookedPostProcessor do
before { SiteSetting.responsive_post_image_sizes = "1|1.5|3" } before { SiteSetting.responsive_post_image_sizes = "1|1.5|3" }
it "includes responsive images on demand" do it "includes responsive images on demand" do
upload.update!(width: 2000, height: 1500, filesize: 10000) upload.update!(width: 2000, height: 1500, filesize: 10000, dominant_color: "FFFFFF")
post = Fabricate(:post, raw: "hello <img src='#{upload.url}'>") post = Fabricate(:post, raw: "hello <img src='#{upload.url}'>")
# fake some optimized images # fake some optimized images
@ -284,17 +284,6 @@ RSpec.describe CookedPostProcessor do
filesize: 800 filesize: 800
) )
# Fake a loading image
_optimized_image = OptimizedImage.create!(
url: "/#{upload_path}/10x10.png",
width: CookedPostProcessor::LOADING_SIZE,
height: CookedPostProcessor::LOADING_SIZE,
upload_id: upload.id,
sha1: SecureRandom.hex,
extension: '.png',
filesize: 123
)
cpp = CookedPostProcessor.new(post) cpp = CookedPostProcessor.new(post)
cpp.add_to_size_cache(upload.url, 2000, 1500) cpp.add_to_size_cache(upload.url, 2000, 1500)
@ -302,7 +291,7 @@ RSpec.describe CookedPostProcessor do
html = cpp.html html = cpp.html
expect(html).to include(%Q|data-small-upload="//test.localhost/#{upload_path}/10x10.png"|) expect(html).to include(%Q|data-dominant-color="FFFFFF"|)
# 1.5x is skipped cause we have a missing thumb # 1.5x is skipped cause we have a missing thumb
expect(html).to include("srcset=\"//test.localhost/#{upload_path}/666x500.jpg, //test.localhost/#{upload_path}/1998x1500.jpg 3x\"") expect(html).to include("srcset=\"//test.localhost/#{upload_path}/666x500.jpg, //test.localhost/#{upload_path}/1998x1500.jpg 3x\"")
expect(html).to include("src=\"//test.localhost/#{upload_path}/666x500.jpg\"") expect(html).to include("src=\"//test.localhost/#{upload_path}/666x500.jpg\"")
@ -316,7 +305,7 @@ RSpec.describe CookedPostProcessor do
html = cpp.html html = cpp.html
expect(html).to include(%Q|data-small-upload="//cdn.localhost/#{upload_path}/10x10.png"|) expect(html).to include(%Q|data-dominant-color="FFFFFF"|)
expect(html).to include("srcset=\"//cdn.localhost/#{upload_path}/666x500.jpg, //cdn.localhost/#{upload_path}/1998x1500.jpg 3x\"") expect(html).to include("srcset=\"//cdn.localhost/#{upload_path}/666x500.jpg, //cdn.localhost/#{upload_path}/1998x1500.jpg 3x\"")
expect(html).to include("src=\"//cdn.localhost/#{upload_path}/666x500.jpg\"") expect(html).to include("src=\"//cdn.localhost/#{upload_path}/666x500.jpg\"")
end end
@ -416,7 +405,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.max_image_height = 2000 SiteSetting.max_image_height = 2000
@ -556,7 +545,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.create_thumbnails = true SiteSetting.create_thumbnails = true
@ -580,7 +569,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.create_thumbnails = true SiteSetting.create_thumbnails = true
@ -604,7 +593,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.create_thumbnails = true SiteSetting.create_thumbnails = true
@ -631,7 +620,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
set_subfolder "/subfolder" set_subfolder "/subfolder"
@ -671,7 +660,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.max_image_height = 2000 SiteSetting.max_image_height = 2000
@ -699,7 +688,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.max_image_height = 2000 SiteSetting.max_image_height = 2000
@ -727,7 +716,7 @@ RSpec.describe CookedPostProcessor do
HTML HTML
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
before do before do
SiteSetting.max_image_height = 2000 SiteSetting.max_image_height = 2000
@ -817,7 +806,7 @@ RSpec.describe CookedPostProcessor do
![alttext|1750x2000|thumbnail](#{upload2.url}) ![alttext|1750x2000|thumbnail](#{upload2.url})
MD MD
CookedPostProcessor.new(post, disable_loading_image: true).post_process CookedPostProcessor.new(post, disable_dominant_color: true).post_process
expect(post.reload.image_upload_id).to eq(upload2.id) expect(post.reload.image_upload_id).to eq(upload2.id)
end end
@ -959,7 +948,7 @@ RSpec.describe CookedPostProcessor do
it "adds lightbox and optimizes images" do it "adds lightbox and optimizes images" do
post = Fabricate(:post, raw: "![image|1024x768, 50%](#{upload.short_url})") post = Fabricate(:post, raw: "![image|1024x768, 50%](#{upload.short_url})")
cpp = CookedPostProcessor.new(post, disable_loading_image: true) cpp = CookedPostProcessor.new(post, disable_dominant_color: true)
cpp.post_process cpp.post_process
doc = Nokogiri::HTML5::fragment(cpp.html) doc = Nokogiri::HTML5::fragment(cpp.html)
@ -974,7 +963,7 @@ RSpec.describe CookedPostProcessor do
upload.update!(animated: true) upload.update!(animated: true)
post = Fabricate(:post, raw: "![image|1024x768, 50%](#{upload.short_url})") post = Fabricate(:post, raw: "![image|1024x768, 50%](#{upload.short_url})")
cpp = CookedPostProcessor.new(post, disable_loading_image: true) cpp = CookedPostProcessor.new(post, disable_dominant_color: true)
cpp.post_process cpp.post_process
doc = Nokogiri::HTML5::fragment(cpp.html) doc = Nokogiri::HTML5::fragment(cpp.html)
@ -992,7 +981,7 @@ RSpec.describe CookedPostProcessor do
it "marks giphy images as animated" do it "marks giphy images as animated" do
post = Fabricate(:post, raw: "![tennis-gif|311x280](https://media2.giphy.com/media/7Oifk90VrCdNe/giphy.webp)") post = Fabricate(:post, raw: "![tennis-gif|311x280](https://media2.giphy.com/media/7Oifk90VrCdNe/giphy.webp)")
cpp = CookedPostProcessor.new(post, disable_loading_image: true) cpp = CookedPostProcessor.new(post, disable_dominant_color: true)
cpp.post_process cpp.post_process
doc = Nokogiri::HTML5::fragment(cpp.html) doc = Nokogiri::HTML5::fragment(cpp.html)
@ -1001,7 +990,7 @@ RSpec.describe CookedPostProcessor do
it "marks giphy images as animated" do it "marks giphy images as animated" do
post = Fabricate(:post, raw: "![cat](https://media1.tenor.com/images/20c7ddd5e84c7427954f430439c5209d/tenor.gif)") post = Fabricate(:post, raw: "![cat](https://media1.tenor.com/images/20c7ddd5e84c7427954f430439c5209d/tenor.gif)")
cpp = CookedPostProcessor.new(post, disable_loading_image: true) cpp = CookedPostProcessor.new(post, disable_dominant_color: true)
cpp.post_process cpp.post_process
doc = Nokogiri::HTML5::fragment(cpp.html) doc = Nokogiri::HTML5::fragment(cpp.html)
@ -1016,7 +1005,7 @@ RSpec.describe CookedPostProcessor do
[/quote] [/quote]
MD MD
cpp = CookedPostProcessor.new(post, disable_loading_image: true) cpp = CookedPostProcessor.new(post, disable_dominant_color: true)
cpp.post_process cpp.post_process
doc = Nokogiri::HTML5::fragment(cpp.html) doc = Nokogiri::HTML5::fragment(cpp.html)
@ -1031,7 +1020,7 @@ RSpec.describe CookedPostProcessor do
post = Fabricate(:post, raw: "https://discourse.org") post = Fabricate(:post, raw: "https://discourse.org")
cpp = CookedPostProcessor.new(post, disable_loading_image: true) cpp = CookedPostProcessor.new(post, disable_dominant_color: true)
cpp.post_process cpp.post_process
doc = Nokogiri::HTML5::fragment(cpp.html) doc = Nokogiri::HTML5::fragment(cpp.html)
@ -1591,7 +1580,7 @@ RSpec.describe CookedPostProcessor do
RAW RAW
end end
let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) }
it "does remove user ids" do it "does remove user ids" do
cpp.remove_user_ids cpp.remove_user_ids

View File

@ -618,4 +618,76 @@ RSpec.describe Upload do
expect(Upload.secure_media_url?(url)).to eq(false) expect(Upload.secure_media_url?(url)).to eq(false)
end end
end end
describe "#dominant_color" do
let(:white_image) { Fabricate(:image_upload, color: "white") }
let(:red_image) { Fabricate(:image_upload, color: "red") }
let(:not_an_image) {
upload = Fabricate(:upload)
file = Tempfile.new(['invalid', '.txt'])
file << "Not really an image"
file.rewind
upload.update(url: Discourse.store.store_upload(file, upload), extension: "txt")
upload
}
let(:invalid_image) {
upload = Fabricate(:upload)
file = Tempfile.new(['invalid', '.png'])
file << "Not really an image"
file.rewind
upload.update(url: Discourse.store.store_upload(file, upload))
upload
}
it "correctly identifies and stores an image's dominant color" do
expect(white_image.dominant_color).to eq(nil)
expect(white_image.dominant_color(calculate_if_missing: true)).to eq("FFFFFF")
expect(white_image.dominant_color).to eq("FFFFFF")
expect(red_image.dominant_color).to eq(nil)
expect(red_image.dominant_color(calculate_if_missing: true)).to eq("FF0000")
expect(red_image.dominant_color).to eq("FF0000")
end
it "can be backfilled" do
expect(white_image.dominant_color).to eq(nil)
expect(red_image.dominant_color).to eq(nil)
Upload.backfill_dominant_colors!(5)
white_image.reload
red_image.reload
expect(white_image.dominant_color).to eq("FFFFFF")
expect(red_image.dominant_color).to eq("FF0000")
end
it "stores an empty string for non-image uploads" do
expect(not_an_image.dominant_color).to eq(nil)
expect(not_an_image.dominant_color(calculate_if_missing: true)).to eq("")
expect(not_an_image.dominant_color).to eq("")
end
it "correctly handles invalid image files" do
expect(invalid_image.dominant_color).to eq(nil)
expect(invalid_image.dominant_color(calculate_if_missing: true)).to eq("")
expect(invalid_image.dominant_color).to eq("")
end
it "correctly handles unparsable ImageMagick output" do
Discourse::Utils.stubs(:execute_command).returns('someinvalidoutput')
expect(invalid_image.dominant_color).to eq(nil)
expect {
invalid_image.dominant_color(calculate_if_missing: true)
}.to raise_error(/Calculated dominant color but unable to parse output/)
expect(invalid_image.dominant_color).to eq(nil)
end
end
end end

View File

@ -39,6 +39,9 @@
}, },
"human_filesize": { "human_filesize": {
"type": "string" "type": "string"
},
"dominant_color": {
"type": ["string", "null"]
} }
}, },
"required": [ "required": [