From: Tom Hughes <tom@compton.nu>
Date: Wed, 13 Oct 2021 18:22:38 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/3340'
X-Git-Tag: live~2397
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/0b445cf7165dc9b71ad08d7bae6147137f9b016e?hp=36179746309d7659d3e20512c34bc2742b094a04

Merge remote-tracking branch 'upstream/pull/3340'
---

diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb
index 69b0f6d6b..eed55cd9d 100644
--- a/app/helpers/user_helper.rb
+++ b/app/helpers/user_helper.rb
@@ -8,9 +8,9 @@ module UserHelper
     if user.image_use_gravatar
       user_gravatar_tag(user, options)
     elsif user.avatar.attached?
-      image_tag user_avatar_variant(user, :resize_to_limit => [100, 100]), options
+      user_avatar_variant_tag(user, { :resize_to_limit => [100, 100] }, options)
     else
-      image_tag "avatar_large.png", options
+      image_tag "avatar_large.png", options.merge(:width => 100, :height => 100)
     end
   end
 
@@ -19,11 +19,11 @@ module UserHelper
     options[:alt] ||= ""
 
     if user.image_use_gravatar
-      user_gravatar_tag(user, options)
+      user_gravatar_tag(user, options.merge(:size => 50))
     elsif user.avatar.attached?
-      image_tag user_avatar_variant(user, :resize_to_limit => [50, 50]), options
+      user_avatar_variant_tag(user, { :resize_to_limit => [50, 50] }, options)
     else
-      image_tag "avatar_small.png", options
+      image_tag "avatar_small.png", options.merge(:width => 50, :height => 50)
     end
   end
 
@@ -32,11 +32,11 @@ module UserHelper
     options[:alt] ||= ""
 
     if user.image_use_gravatar
-      user_gravatar_tag(user, options)
+      user_gravatar_tag(user, options.merge(:size => 50))
     elsif user.avatar.attached?
-      image_tag user_avatar_variant(user, :resize_to_limit => [50, 50]), options
+      user_avatar_variant_tag(user, { :resize_to_limit => [50, 50] }, options)
     else
-      image_tag "avatar_small.png", options
+      image_tag "avatar_small.png", options.merge(:width => 50, :height => 50)
     end
   end
 
@@ -69,6 +69,22 @@ module UserHelper
   private
 
   # Local avatar support
+  def user_avatar_variant_tag(user, variant_options, options)
+    if user.avatar.variable?
+      variant = user.avatar.variant(variant_options)
+      # https://stackoverflow.com/questions/61893089/get-metadata-of-active-storage-variant/67228171
+      if variant.processed?
+        metadata = variant.processed.send(:record).image.blob.metadata
+        if metadata["width"]
+          options[:width] = metadata["width"]
+          options[:height] = metadata["height"]
+        end
+      end
+      image_tag variant, options
+    else
+      image_tag user.avatar, options
+    end
+  end
 
   def user_avatar_variant(user, options)
     if user.avatar.variable?
@@ -90,7 +106,7 @@ module UserHelper
 
   def user_gravatar_tag(user, options = {})
     url = user_gravatar_url(user, options)
-    options.delete(:size)
+    options[:height] = options[:width] = options.delete(:size) || 100
     image_tag url, options
   end
 end
diff --git a/test/helpers/user_helper_test.rb b/test/helpers/user_helper_test.rb
index 758baa1cb..8cef4d2dc 100644
--- a/test/helpers/user_helper_test.rb
+++ b/test/helpers/user_helper_test.rb
@@ -12,7 +12,6 @@ class UserHelperTest < ActionView::TestCase
 
     image = user_image(user, :class => "foo")
     assert_match %r{^<img class="foo" .* src="/images/avatar_large.png" />$}, image
-
     image = user_image(gravatar_user)
     assert_match %r{^<img class="user_image" .* src="http://www.gravatar.com/avatar/.*" />$}, image
 
@@ -66,6 +65,50 @@ class UserHelperTest < ActionView::TestCase
     assert_match %r{^http://www.gravatar.com/avatar/}, url
   end
 
+  def test_user_image_sizes_default_image
+    user = create(:user)
+
+    image = user_image(user)
+    assert_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+  end
+
+  def test_user_image_sizes_avatar
+    user = create(:user)
+    user.avatar.attach(:io => File.open("test/gpx/fixtures/a.gif"), :filename => "a.gif")
+
+    # first time access, no width or height is found
+    image = user_image(user)
+    assert_no_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_no_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+
+    # Small hacks to simulate what happens when the images have been fetched at least once before
+    variant = user.avatar.variant(:resize_to_limit => [100, 100])
+    variant.processed.send(:record).image.blob.analyze
+    variant = user.avatar.variant(:resize_to_limit => [50, 50])
+    variant.processed.send(:record).image.blob.analyze
+
+    image = user_image(user)
+    assert_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+  end
+
+  def test_user_image_sizes_gravatar
+    user = create(:user, :image_use_gravatar => true)
+
+    image = user_image(user)
+    assert_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+  end
+
   def test_openid_logo
     logo = openid_logo
     assert_match %r{^<img .* class="openid_logo" src="/images/openid_small.png" />$}, logo