From 3bc1d28d5dc4f671f861174048b9e352e094ba76 Mon Sep 17 00:00:00 2001
From: MAEDA Go <maeda@farend.jp>
Date: Tue, 4 Aug 2026 17:29:34 +0900
Subject: [PATCH 4/6] Show the same file type icon whether the content type
 comes from Marcel or Redmine::MimeType

---
 app/helpers/icons_helper.rb             | 77 +++++++++++++++----------
 app/views/attachments/_links.html.erb   |  2 +-
 app/views/attachments/edit_all.html.erb |  2 +-
 app/views/issues/_edit.html.erb         |  2 +-
 app/views/wiki/edit.html.erb            |  2 +-
 test/helpers/icons_helper_test.rb       | 22 +++++++
 6 files changed, 74 insertions(+), 33 deletions(-)

diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb
index 627f8c8cf..0e1e03520 100644
--- a/app/helpers/icons_helper.rb
+++ b/app/helpers/icons_helper.rb
@@ -23,6 +23,40 @@ module IconsHelper
   DEFAULT_ICON_SIZE = "18"
   DEFAULT_SPRITE = "icons"
 
+  # Content types covered by each icon. The same kind of file can be denoted by
+  # several types, depending on whether the type was detected from the file
+  # contents or taken from Redmine::MimeType, so list them all rather than
+  # deriving the icon name from the type. An entry with no subtype (eg. audio)
+  # covers every type of that top level type that is not listed elsewhere.
+  ICON_MIME_TYPES = {
+    'application-gzip' => %w(application/gzip),
+    'application-javascript' => %w(application/javascript text/javascript),
+    'application-pdf' => %w(application/pdf),
+    'application-zip' => %w(application/zip),
+    'file-music' => %w(audio),
+    'movie' => %w(video),
+    'photo' => %w(image),
+    'text-css' => %w(text/css),
+    'text-html' => %w(text/html),
+    'text-plain' => %w(text application/sql application/x-csh application/x-sh),
+    'text-x-c' => %w(text/x-c text/x-c++hdr text/x-c++src),
+    'text-x-csharp' => %w(text/x-csharp),
+    'text-x-java' => %w(text/x-java text/x-java-source),
+    'text-x-php' => %w(text/x-php),
+    'text-x-ruby' => %w(text/x-ruby),
+    'text-xml' => %w(text/xml application/xml),
+    # Microsoft Office Open XML documents
+    # Do not add legacy Office formats (.doc, .xls, .ppt) here because
+    # Redmine does not provide attachment preview for these formats.
+    'file-type-docx' => %w(application/vnd.openxmlformats-officedocument.wordprocessingml.document),
+    'file-type-ppt' => %w(application/vnd.openxmlformats-officedocument.presentationml.presentation),
+    'file-type-xls' => %w(application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
+  }.freeze
+
+  MIME_TYPE_ICONS = ICON_MIME_TYPES.each_with_object({}) do |(icon, types), map|
+    types.each {|type| map[type] = icon}
+  end.freeze
+
   def sprite_source(icon_name, sprite: DEFAULT_SPRITE, plugin: nil)
     if plugin
       "plugin_assets/#{plugin}/#{sprite}.svg"
@@ -102,9 +136,9 @@ module IconsHelper
     sprite_icon(icon_name, **)
   end
 
-  def file_type_icon(mime_type, ...)
-    icon_name = icon_for_mime_type(mime_type)
-    sprite_icon(icon_name, ...)
+  def file_type_icon(mime_type, label = nil, filename: nil, **)
+    icon_name = icon_for_mime_type(mime_type, filename)
+    sprite_icon(icon_name, label, **)
   end
 
   private
@@ -125,32 +159,17 @@ module IconsHelper
     )
   end
 
-  def icon_for_mime_type(mime)
-    if %w(text/x-c text/x-csharp text/x-java text/x-php
-          text/x-ruby text/xml text/css text/html text/css text/html
-          application/pdf application/zip application/gzip application/javascript).include?(mime)
-      icon_name = mime.tr('/', '-')
-    else
-      top_level_type, subtype = mime.to_s.split('/')
-      icon_name =
-        case top_level_type
-        when 'audio' then 'file-music'
-        when 'image' then 'photo'
-        when 'text'
-          %w(markdown plain x-textile).include?(subtype) ? 'text-plain' : nil
-        when 'video' then 'movie'
-        else
-          # MIME type mapping
-          {
-            # Microsoft Office Open XML documents
-            # Do not add legacy Office formats (.doc, .xls, .ppt) here because
-            # Redmine does not provide attachment preview for these formats.
-            'application/vnd.openxmlformats-officedocument.presentationml.presentation' => 'file-type-ppt',
-            'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => 'file-type-xls',
-            'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => 'file-type-docx'
-          }[mime.to_s]
-        end
+  def icon_for_mime_type(mime, filename = nil)
+    # Content type detection falls back to application/octet-stream when it
+    # cannot tell the type from the file contents. In that case only, guess
+    # the type from the filename, as AttachmentsController#detect_content_type
+    # does.
+    if filename.present? && (mime.blank? || mime == 'application/octet-stream')
+      mime = Redmine::MimeType.of(filename)
     end
-    icon_name || 'file'
+
+    MIME_TYPE_ICONS[mime] ||
+      MIME_TYPE_ICONS[mime.to_s.split('/').first] ||
+      'file'
   end
 end
diff --git a/app/views/attachments/_links.html.erb b/app/views/attachments/_links.html.erb
index d2341a4a9..8bfee8e95 100644
--- a/app/views/attachments/_links.html.erb
+++ b/app/views/attachments/_links.html.erb
@@ -15,7 +15,7 @@
 <% for attachment in attachments %>
 <tr>
   <td>
-    <%= link_to_attachment attachment, class: 'icon icon-attachment ', icon: icon_for_mime_type(attachment.content_type) -%>
+    <%= link_to_attachment attachment, class: 'icon icon-attachment ', icon: icon_for_mime_type(attachment.content_type, attachment.filename) -%>
     <span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
     <%= link_to_attachment attachment, class: 'icon-only icon-download ', title: l(:button_download), download: true, icon: 'download' -%>
   </td>
diff --git a/app/views/attachments/edit_all.html.erb b/app/views/attachments/edit_all.html.erb
index 3b1800c5b..20b4e670a 100644
--- a/app/views/attachments/edit_all.html.erb
+++ b/app/views/attachments/edit_all.html.erb
@@ -11,7 +11,7 @@
   <% @attachments.each do |attachment| %>
     <tr>
       <td colspan="2">
-        <span class="icon icon-attachment"><%= file_type_icon(attachment.content_type_was, attachment.filename_was) %></span>
+        <span class="icon icon-attachment"><%= file_type_icon(attachment.content_type_was, attachment.filename_was, filename: attachment.filename_was) %></span>
         <span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
         <span class="author"><%= attachment.author %>, <%= format_time(attachment.created_on) %></span>
       </td>
diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb
index b7e368596..f86fb07eb 100644
--- a/app/views/issues/_edit.html.erb
+++ b/app/views/issues/_edit.html.erb
@@ -55,7 +55,7 @@
         <div id="existing-attachments" style="<%= @issue.deleted_attachment_ids.blank? ? 'display:none;' : '' %>">
           <% @issue.attachments.each do |attachment| %>
           <span class="existing-attachment">
-            <%= file_type_icon(attachment.content_type, size: 12) %>
+            <%= file_type_icon(attachment.content_type, filename: attachment.filename, size: 12) %>
             <%= text_field_tag '', attachment.filename, :class => "icon icon-attachment filename", :disabled => true %>
             <label>
               <%= check_box_tag 'issue[deleted_attachment_ids][]',
diff --git a/app/views/wiki/edit.html.erb b/app/views/wiki/edit.html.erb
index 4255193ad..2d98e6ea1 100644
--- a/app/views/wiki/edit.html.erb
+++ b/app/views/wiki/edit.html.erb
@@ -41,7 +41,7 @@
 <div id="existing-attachments" style="<%= @page.deleted_attachment_ids.blank? ? 'display:none;' : '' %>">
   <% @page.attachments.each do |attachment| %>
   <span class="existing-attachment">
-    <%= file_type_icon(attachment.content_type, size: 12) %>
+    <%= file_type_icon(attachment.content_type, filename: attachment.filename, size: 12) %>
     <%= text_field_tag '', attachment.filename, :class => "icon icon-attachment filename", :disabled => true %>
     <label class='inline'>
       <%= check_box_tag 'wiki_page[deleted_attachment_ids][]',
diff --git a/test/helpers/icons_helper_test.rb b/test/helpers/icons_helper_test.rb
index a1c9cab6e..25b0e6e38 100644
--- a/test/helpers/icons_helper_test.rb
+++ b/test/helpers/icons_helper_test.rb
@@ -242,6 +242,28 @@ class IconsHelperTest < Redmine::HelperTest
     assert_equal 'movie', icon_for_mime_type('video/mp4')
   end
 
+  def test_icon_for_mime_type_should_return_the_same_icon_for_equivalent_mime_types
+    # Types detected from the file contents and their Redmine::MimeType counterparts
+    {
+      'text/javascript' => 'application/javascript',
+      'application/xml' => 'text/xml',
+      'text/x-c++src' => 'text/x-c',
+      'text/x-java-source' => 'text/x-java',
+      'text/x-diff' => 'text/plain',
+      'application/sql' => 'text/plain'
+    }.each do |detected, counterpart|
+      assert_equal icon_for_mime_type(counterpart), icon_for_mime_type(detected),
+                   "#{detected} should get the same icon as #{counterpart}"
+    end
+  end
+
+  def test_icon_for_mime_type_should_fall_back_to_filename_only_when_type_is_undetermined
+    assert_equal 'text-plain', icon_for_mime_type('application/octet-stream', 'readme.textile')
+    assert_equal 'application-pdf', icon_for_mime_type(nil, 'document.pdf')
+    # A type detected from the file contents takes precedence over the filename
+    assert_equal 'text-html', icon_for_mime_type('text/html', 'not-really-an-image.png')
+  end
+
   def test_icon_for_mime_type_should_return_generic_file_icon_for_unknown_mime_types
     assert_equal 'file', icon_for_mime_type('unknown-type')
   end
-- 
2.50.1