Fix duplicated alt and title attributes for attached images in Textile formatter...

Patch by Katsuya HIDAKA (@hidakatsuya).

git-svn-id: https://svn.redmine.org/redmine/trunk@22845 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 1dd96cbadf880452b6f59277ad5dc68ed8cf631f..37bfcf0de9c583046883e4dba1416eed654f189b 100644 (file)
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -944,16 +944,27 @@ module ApplicationHelper
       attachments += obj.attachments if obj.respond_to?(:attachments)
     end
     if attachments.present?
-      text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpe|jpeg|png|webp))"(\s+alt="([^"]*)")?/i) do |m|
-        filename, ext, alt, alttext = $1, $2, $3, $4
+      title_and_alt_re = /\s+(title|alt)="([^"]*)"/i
+
+      text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpe|jpeg|png|webp))"([^>]*)/i) do |m|
+        filename, ext, other_attrs = $1, $2, $3
+
         # search for the picture in attachments
         if found = Attachment.latest_attach(attachments, CGI.unescape(filename))
           image_url = download_named_attachment_url(found, found.filename, :only_path => only_path)
           desc = found.description.to_s.delete('"')
-          if !desc.blank? && alttext.blank?
-            alt = " title=\"#{desc}\" alt=\"#{desc}\""
-          end
-          "src=\"#{image_url}\"#{alt} loading=\"lazy\""
+
+          # remove title and alt attributes after extracting them
+          title_and_alt = other_attrs.scan(title_and_alt_re).to_h
+          other_attrs.gsub!(title_and_alt_re, '')
+
+          title_and_alt_attrs = if !desc.blank? && title_and_alt['alt'].blank?
+                                  " title=\"#{desc}\" alt=\"#{desc}\""
+                                else
+                                  # restore original title and alt attributes
+                                  " #{title_and_alt.map { |k, v| %[#{k}="#{v}"] }.join(' ')}"
+                                end
+          "src=\"#{image_url}\"#{title_and_alt_attrs} loading=\"lazy\"#{other_attrs}"
         else
           m
         end

diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb
index 443ad4698d6147e1f6cd5345006a645a74053372..70c11273284c3c325692d5a6067e361afeb4c5e1 100644 (file)
--- a/test/helpers/application_helper_test.rb
+++ b/test/helpers/application_helper_test.rb
@@ -202,11 +202,11 @@ class ApplicationHelperTest < Redmine::HelperTest
     attachments = Attachment.all
     with_settings text_formatting: 'textile' do
       # When alt text is set
-      assert_match %r[<img src=".+?" title="This is a logo" alt="This is a logo" loading=".+?" title="alt text" alt="alt text" />],
+      assert_match %r[<img src=".+?" title="alt text" alt="alt text" loading=".+?" />],
         textilizable('!logo.gif(alt text)!', attachments: attachments)
 
       # When alt text and style are set
-      assert_match %r[<img src=".+?" title="This is a logo" alt="This is a logo" loading=".+?" style="width:100px;" title="alt text" alt="alt text" />],
+      assert_match %r[<img src=".+?" title="alt text" alt="alt text" loading=".+?" style="width:100px;" />],
         textilizable('!{width:100px}logo.gif(alt text)!', attachments: attachments)
 
       # When alt text is not set