Merge branch '31887-remove-images-from-todos' into 'master'

Resolve "Remove images from todos"

Closes #31887

See merge request gitlab-org/gitlab-ce!21704

1 files changed, 20 insertions, 3 deletions

diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index f2cd676bb1b..0d638b850b4 100644
--- a/app/helpers/markup_helper.rb
+++ b/app/helpers/markup_helper.rb
@@ -74,14 +74,21 @@ module MarkupHelper
   # the tag contents are truncated without removing the closing tag.
   def first_line_in_markdown(object, attribute, max_chars = nil, options = {})
     md = markdown_field(object, attribute, options)
+    return nil unless md.present?
 
-    text = truncate_visible(md, max_chars || md.length) if md.present?
+    tags = %w(a gl-emoji b pre code p span)
+    tags << 'img' if options[:allow_images]
 
-    sanitize(
+    text = truncate_visible(md, max_chars || md.length)
+    text = sanitize(
       text,
-      tags: %w(a img gl-emoji b pre code p span),
+      tags: tags,
       attributes: Rails::Html::WhiteListSanitizer.allowed_attributes + ['style', 'data-src', 'data-name', 'data-unicode-version']
     )
+
+    # since <img> tags are stripped, this can leave empty <a> tags hanging around
+    # (as our markdown wraps images in links)
+    options[:allow_images] ? text : strip_empty_link_tags(text).html_safe
   end
 
   def markdown(text, context = {})
@@ -235,6 +242,16 @@ module MarkupHelper
     end
   end
 
+  def strip_empty_link_tags(text)
+    scrubber = Loofah::Scrubber.new do |node|
+      node.remove if node.name == 'a' && node.content.blank?
+    end
+
+    # Use `Loofah` directly instead of `sanitize`
+    # as we still use the `rails-deprecated_sanitizer` gem
+    Loofah.fragment(text).scrub!(scrubber).to_s
+  end
+
   def markdown_toolbar_button(options = {})
     data = options[:data].merge({ container: 'body' })
     content_tag :button,