FEATURE: reduce avatar sizes to 6 from 20

app/assets/javascripts/discourse/app/lib/utilities.js

@@ -20,17 +20,17 @@ export function splitString(str, separator = ",") {
 export function translateSize(size) {
   switch (size) {
     case "tiny":
-      return 20;
+      return 24;
     case "small":
-      return 25;
+      return 24;
     case "medium":
-      return 32;
+      return 48;
     case "large":
-      return 45;
+      return 48;
     case "extra_large":
-      return 60;
+      return 96;
     case "huge":
-      return 120;
+      return 144;
   }
   return size;
 }
@@ -62,11 +62,31 @@ export function avatarUrl(template, size, { customGetURL } = {}) {
   if (!template) {
     return "";
   }
-  const rawSize = getRawSize(translateSize(size));
+  const rawSize = getRawAvatarSize(translateSize(size));
   const templatedPath = template.replace(/\{size\}/g, rawSize);
   return (customGetURL || getURLWithCDN)(templatedPath);
 }
 
+let allowedSizes = null;
+
+export function getRawAvatarSize(size) {
+  allowedSizes ??=
+    helperContext()
+      .siteSettings["avatar_sizes"].split("|")
+      .map((s) => parseInt(s, 10))
+      .sort((a, b) => a - b);
+
+  size = getRawSize(size);
+
+  for (let i = 0; i < allowedSizes.length; i++) {
+    if (allowedSizes[i] >= size) {
+      return allowedSizes[i];
+    }
+  }
+
+  return allowedSizes[allowedSizes.length - 1];
+}
+
 export function getRawSize(size) {
   const pixelRatio = window.devicePixelRatio || 1;
   let rawSize = 1;

app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js

@@ -8,7 +8,7 @@ import {
   escapeExpression,
   extractDomainFromUrl,
   fillMissingDates,
-  getRawSize,
+  getRawAvatarSize,
   inCodeBlock,
   initializeDefaultHomepage,
   mergeSortedLists,
@@ -86,25 +86,36 @@ module("Unit | Utilities", function (hooks) {
     );
   });
 
+  test("getRawAvatarSize avoids redirects", function (assert) {
+    assert.strictEqual(
+      getRawAvatarSize(1),
+      24,
+      "returns the first size larger on the menu"
+    );
+
+    assert.strictEqual(getRawAvatarSize(2000), 288, "caps at highest");
+  });
+
   test("avatarUrl", function (assert) {
-    let rawSize = getRawSize;
     assert.blank(avatarUrl("", "tiny"), "no template returns blank");
     assert.strictEqual(
       avatarUrl("/fake/template/{size}.png", "tiny"),
-      "/fake/template/" + rawSize(20) + ".png",
+      "/fake/template/" + getRawAvatarSize(24) + ".png",
       "simple avatar url"
     );
     assert.strictEqual(
       avatarUrl("/fake/template/{size}.png", "large"),
-      "/fake/template/" + rawSize(45) + ".png",
+      "/fake/template/" + getRawAvatarSize(48) + ".png",
       "different size"
     );
 
     setupURL("https://app-cdn.example.com", "https://example.com", "");
 
     assert.strictEqual(
       avatarUrl("/fake/template/{size}.png", "large"),
-      "https://app-cdn.example.com/fake/template/" + rawSize(45) + ".png",
+      "https://app-cdn.example.com/fake/template/" +
+        getRawAvatarSize(48) +
+        ".png",
       "uses CDN if present"
     );
   });
@@ -124,7 +135,7 @@ module("Unit | Utilities", function (hooks) {
     let avatarTemplate = "/path/to/avatar/{size}.png";
     assert.strictEqual(
       avatarImg({ avatarTemplate, size: "tiny" }),
-      "<img loading='lazy' alt='' width='20' height='20' src='/path/to/avatar/40.png' class='avatar'>",
+      "<img loading='lazy' alt='' width='24' height='24' src='/path/to/avatar/48.png' class='avatar'>",
       "it returns the avatar html"
     );
 
@@ -134,7 +145,7 @@ module("Unit | Utilities", function (hooks) {
         size: "tiny",
         title: "evilest trout",
       }),
-      "<img loading='lazy' alt='' width='20' height='20' src='/path/to/avatar/40.png' class='avatar' title='evilest trout' aria-label='evilest trout'>",
+      "<img loading='lazy' alt='' width='24' height='24' src='/path/to/avatar/48.png' class='avatar' title='evilest trout' aria-label='evilest trout'>",
       "it adds a title if supplied"
     );
 
@@ -144,7 +155,7 @@ module("Unit | Utilities", function (hooks) {
         size: "tiny",
         extraClasses: "evil fish",
       }),
-      "<img loading='lazy' alt='' width='20' height='20' src='/path/to/avatar/40.png' class='avatar evil fish'>",
+      "<img loading='lazy' alt='' width='24' height='24' src='/path/to/avatar/48.png' class='avatar evil fish'>",
       "it adds extra classes if supplied"
     );
 

app/assets/javascripts/discourse/tests/unit/models/report-test.js

@@ -370,7 +370,7 @@ module("Unit | Model | report", function (hooks) {
     const computedUsernameLabel = usernameLabel.compute(row);
     assert.strictEqual(
       computedUsernameLabel.formattedValue,
-      "<a href='/admin/users/1/joffrey'><img loading='lazy' alt='' width='20' height='20' src='/' class='avatar' title='joffrey' aria-label='joffrey'><span class='username'>joffrey</span></a>"
+      "<a href='/admin/users/1/joffrey'><img loading='lazy' alt='' width='24' height='24' src='/' class='avatar' title='joffrey' aria-label='joffrey'><span class='username'>joffrey</span></a>"
     );
     assert.strictEqual(computedUsernameLabel.value, "joffrey");
 
@@ -459,7 +459,7 @@ module("Unit | Model | report", function (hooks) {
     const userLink = computedLabels[0].compute(row).formattedValue;
     assert.strictEqual(
       userLink,
-      "<a href='/forum/admin/users/1/joffrey'><img loading='lazy' alt='' width='20' height='20' src='/forum/' class='avatar' title='joffrey' aria-label='joffrey'><span class='username'>joffrey</span></a>"
+      "<a href='/forum/admin/users/1/joffrey'><img loading='lazy' alt='' width='24' height='24' src='/forum/' class='avatar' title='joffrey' aria-label='joffrey'><span class='username'>joffrey</span></a>"
     );
   });
 });

app/models/user_avatar.rb

@@ -150,7 +150,7 @@ def self.import_url_for_user(avatar_url, user, options = nil)
     tempfile.close! if tempfile && tempfile.respond_to?(:close!)
   end
 
-  def self.ensure_consistency!
+  def self.ensure_consistency!(max_optimized_avatars_to_remove: 20_000)
     DB.exec <<~SQL
       UPDATE user_avatars
       SET gravatar_upload_id = NULL
@@ -174,6 +174,29 @@ def self.ensure_consistency!
           up.id IS NULL
       )
     SQL
+
+    ids =
+      DB.query_single(<<~SQL, sizes: Discourse.avatar_sizes, limit: max_optimized_avatars_to_remove)
+      SELECT oi.id FROM user_avatars a
+      JOIN optimized_images oi ON oi.upload_id = a.custom_upload_id
+      LEFT JOIN upload_references ur ON ur.upload_id = a.custom_upload_id and ur.target_type <> 'UserAvatar'
+      WHERE oi.width not in (:sizes) AND oi.height not in (:sizes) AND ur.upload_id IS NULL
+      LIMIT :limit
+    SQL
+
+    warnings_reported = 0
+
+    ids.each do |id|
+      begin
+        OptimizedImage.find(id).destroy!
+      rescue ActiveRecord::RecordNotFound
+      rescue => e
+        if warnings_reported < 10
+          Discourse.warn_exception(e, message: "Failed to remove optimized image")
+          warnings_reported += 1
+        end
+      end
+    end
   end
 end
 

config/site_settings.yml

@@ -1467,9 +1467,10 @@ files:
     type: url_list
     client: true
   avatar_sizes:
-    default: "20|25|32|45|60|120"
+    default: "24|48|72|96|144|288"
     type: list
     list_type: compact
+    client: true
   external_system_avatars_enabled:
     default: true
     client: true

lib/discourse.rb

@@ -327,19 +327,12 @@ def self.anonymous_top_menu_items
     @anonymous_top_menu_items ||= Discourse.anonymous_filters + %i[categories top]
   end
 
+  # list of pixel ratios Discourse tries to optimize for
   PIXEL_RATIOS ||= [1, 1.5, 2, 3]
 
   def self.avatar_sizes
     # TODO: should cache these when we get a notification system for site settings
-    set = Set.new
-
-    SiteSetting
-      .avatar_sizes
-      .split("|")
-      .map(&:to_i)
-      .each { |size| PIXEL_RATIOS.each { |pixel_ratio| set << (size * pixel_ratio).to_i } }
-
-    set
+    Set.new(SiteSetting.avatar_sizes.split("|").map(&:to_i))
   end
 
   def self.activate_plugins!

lib/pretty_text.rb

@@ -208,6 +208,7 @@ def self.markdown(text, opts = {})
         __optInput.watchedWordsReplace = #{WordWatcher.word_matcher_regexps(:replace, engine: :js).to_json};
         __optInput.watchedWordsLink = #{WordWatcher.word_matcher_regexps(:link, engine: :js).to_json};
         __optInput.additionalOptions = #{Site.markdown_additional_options.to_json};
+        __optInput.avatar_sizes = #{SiteSetting.avatar_sizes.to_json};
       JS
 
       buffer << "__optInput.topicId = #{opts[:topic_id].to_i};\n" if opts[:topic_id]

lib/pretty_text/shims.js

@@ -16,10 +16,11 @@ define("I18n", ["exports"], function (exports) {
   exports.default = I18n;
 });
 
-// Formatting doesn't currently need any helper context
 define("discourse-common/lib/helpers", ["exports"], function (exports) {
   exports.helperContext = function () {
-    return {};
+    return {
+      siteSettings: { avatar_sizes: __optInput.avatar_sizes },
+    };
   };
 });
 

plugins/chat/spec/integration/post_chat_quote_spec.rb

@@ -237,7 +237,7 @@
 <div class="chat-transcript" data-message-id="#{message1.id}" data-username="#{message1.user.username}" data-datetime="#{message1.created_at.iso8601}" data-channel-name="#{channel.name}" data-channel-id="#{channel.id}">
 <div class="chat-transcript-user">
 <div class="chat-transcript-user-avatar">
-<img loading="lazy" alt="" width="20" height="20" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "40")}" class="avatar">
+<img loading="lazy" alt="" width="24" height="24" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "48")}" class="avatar">
 </div>
 <div class="chat-transcript-username">
 #{message1.user.username}</div>
@@ -251,7 +251,7 @@
 <div class="chat-transcript" data-message-id="#{message2.id}" data-username="#{message2.user.username}" data-datetime="#{message2.created_at.iso8601}" data-channel-name="#{channel.name}" data-channel-id="#{channel.id}">
 <div class="chat-transcript-user">
 <div class="chat-transcript-user-avatar">
-<img loading="lazy" alt="" width="20" height="20" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "40")}" class="avatar">
+<img loading="lazy" alt="" width="24" height="24" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "48")}" class="avatar">
 </div>
 <div class="chat-transcript-username">
 #{message2.user.username}</div>

plugins/chat/spec/models/chat/message_spec.rb

@@ -75,7 +75,7 @@
       post = Fabricate(:post, topic: topic)
       SiteSetting.external_system_avatars_enabled = false
       avatar_src =
-        "//test.localhost#{User.system_avatar_template(post.user.username).gsub("{size}", "40")}"
+        "//test.localhost#{User.system_avatar_template(post.user.username).gsub("{size}", "48")}"
 
       cooked = described_class.cook(<<~RAW)
       [quote="#{post.user.username}, post:#{post.post_number}, topic:#{topic.id}"]
@@ -87,7 +87,7 @@
       <aside class="quote no-group" data-username="#{post.user.username}" data-post="#{post.post_number}" data-topic="#{topic.id}">
       <div class="title">
       <div class="quote-controls"></div>
-      <img loading="lazy" alt="" width="20" height="20" src="#{avatar_src}" class="avatar"><a href="http://test.localhost/t/some-quotable-topic/#{topic.id}/#{post.post_number}">#{topic.title}</a>
+      <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src}" class="avatar"><a href="http://test.localhost/t/some-quotable-topic/#{topic.id}/#{post.post_number}">#{topic.title}</a>
       </div>
       <blockquote>
       <p>Mark me...this will go down in history.</p>
@@ -101,9 +101,9 @@
       user = Fabricate(:user, username: "chatbbcodeuser")
       user2 = Fabricate(:user, username: "otherbbcodeuser")
       avatar_src =
-        "//test.localhost#{User.system_avatar_template(user.username).gsub("{size}", "40")}"
+        "//test.localhost#{User.system_avatar_template(user.username).gsub("{size}", "48")}"
       avatar_src2 =
-        "//test.localhost#{User.system_avatar_template(user2.username).gsub("{size}", "40")}"
+        "//test.localhost#{User.system_avatar_template(user2.username).gsub("{size}", "48")}"
       msg1 =
         Fabricate(
           :chat_message,
@@ -135,7 +135,7 @@
         </div>
         <div class="chat-transcript-user">
         <div class="chat-transcript-user-avatar">
-        <img loading="lazy" alt="" width="20" height="20" src="#{avatar_src}" class="avatar">
+        <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src}" class="avatar">
         </div>
         <div class="chat-transcript-username">
         chatbbcodeuser</div>
@@ -150,7 +150,7 @@
         <div class="chat-transcript chat-transcript-chained" data-message-id="#{msg2.id}" data-username="otherbbcodeuser" data-datetime="#{msg2.created_at.iso8601}">
         <div class="chat-transcript-user">
         <div class="chat-transcript-user-avatar">
-        <img loading="lazy" alt="" width="20" height="20" src="#{avatar_src2}" class="avatar">
+        <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src2}" class="avatar">
         </div>
         <div class="chat-transcript-username">
         otherbbcodeuser</div>

spec/lib/discourse_spec.rb

@@ -13,28 +13,8 @@
 
   describe "avatar_sizes" do
     it "returns a list of integers" do
-      expect(Discourse.avatar_sizes).to contain_exactly(
-        20,
-        25,
-        30,
-        32,
-        37,
-        40,
-        45,
-        48,
-        50,
-        60,
-        64,
-        67,
-        75,
-        90,
-        96,
-        120,
-        135,
-        180,
-        240,
-        360,
-      )
+      SiteSetting.avatar_sizes = "10|20|30"
+      expect(Discourse.avatar_sizes).to contain_exactly(10, 20, 30)
     end
   end
 

spec/lib/oneboxer_spec.rb

@@ -94,7 +94,7 @@ def preview(url, user = nil, category = nil, topic = nil)
 
       onebox = preview(public_reply.url, user, public_category, public_topic)
       expect(onebox).not_to include(public_topic.title)
-      expect(onebox).to include(replier.avatar_template_url.sub("{size}", "40"))
+      expect(onebox).to include(replier.avatar_template_url.sub("{size}", "48"))
 
       expect(preview(public_hidden.url, user, public_category)).to match_html(
         link(public_hidden.url),

spec/lib/pretty_text_spec.rb

@@ -251,7 +251,7 @@ def cook(*args)
           <aside class="quote no-group" data-username="#{user.username}" data-post="123" data-topic="456" data-full="true">
           <div class="title">
           <div class="quote-controls"></div>
-          <img loading="lazy" alt="" width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"> #{user.username}:</div>
+          <img loading="lazy" alt="" width="24" height="24" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/48.png" class="avatar"> #{user.username}:</div>
           <blockquote>
           <p>ddd</p>
           </blockquote>
@@ -273,7 +273,7 @@ def cook(*args)
           <aside class="quote no-group" data-username="#{user.username}" data-post="123" data-topic="456" data-full="true">
           <div class="title">
           <div class="quote-controls"></div>
-          <img loading="lazy" alt="" width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"> #{user.username}:</div>
+          <img loading="lazy" alt="" width="24" height="24" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/48.png" class="avatar"> #{user.username}:</div>
           <blockquote>
           <p>ddd</p>
           </blockquote>
@@ -294,7 +294,7 @@ def cook(*args)
           <aside class="quote no-group" data-username="#{user.username}" data-post="555" data-topic="666">
           <div class="title">
           <div class="quote-controls"></div>
-          <img loading="lazy" alt="" width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"> #{user.username}:</div>
+          <img loading="lazy" alt="" width="24" height="24" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/48.png" class="avatar"> #{user.username}:</div>
           <blockquote>
           <p>ddd</p>
           </blockquote>
@@ -320,7 +320,7 @@ def cook(*args)
           <aside class="quote group-#{group.name}" data-username="#{user.username}" data-post="2" data-topic="#{topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
-          <img loading="lazy" alt="" width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"><a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic</a>
+          <img loading="lazy" alt="" width="24" height="24" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/48.png" class="avatar"><a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic</a>
           </div>
           <blockquote>
           <p>ddd</p>