diff options
author | Brett Walker <bwalker@gitlab.com> | 2018-09-12 17:47:21 -0500 |
---|---|---|
committer | Brett Walker <bwalker@gitlab.com> | 2018-09-13 10:33:06 -0500 |
commit | 635d901288d60f73c56859c131e360ddf82d8e34 (patch) | |
tree | 58e56bfc9a7eb0aa11ac492d8481b5162714d228 | |
parent | c7d1eef671dbf598814a6c2ff1f81b924583ae8a (diff) | |
download | gitlab-ce-635d901288d60f73c56859c131e360ddf82d8e34.tar.gz |
Remove images in 'first_line_in_markdown'
By default, we now strip images in the 'first_line_in_markdown'
method. This keeps images from being displayed in the
one-liner of both todo and project activity panels.
Although not currently used, we allow images to be preserved
with the allow_images: true options.
-rw-r--r-- | app/helpers/markup_helper.rb | 23 | ||||
-rw-r--r-- | changelogs/unreleased/31887-remove-images-from-todos.yml | 5 | ||||
-rw-r--r-- | spec/helpers/markup_helper_spec.rb | 22 |
3 files changed, 43 insertions, 7 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, diff --git a/changelogs/unreleased/31887-remove-images-from-todos.yml b/changelogs/unreleased/31887-remove-images-from-todos.yml new file mode 100644 index 00000000000..36388f66514 --- /dev/null +++ b/changelogs/unreleased/31887-remove-images-from-todos.yml @@ -0,0 +1,5 @@ +--- +title: Images are no longer displayed in Todo descriptions +merge_request: 21704 +author: +type: fixed diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 9a29ac26eff..a0c0af94fa5 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -339,11 +339,25 @@ describe MarkupHelper do expect(first_line_in_markdown(object, attribute, 150, project: project)).to eq(expected) end - it 'preserves data-src for lazy images' do - object = create_object("![ImageTest](/uploads/test.png)") - image_url = "data-src=\".*/uploads/test.png\"" + context 'when images are allowed' do + it 'preserves data-src for lazy images' do + object = create_object("![ImageTest](/uploads/test.png)") + image_url = "data-src=\".*/uploads/test.png\"" + text = first_line_in_markdown(object, attribute, 150, project: project, allow_images: true) + + expect(text).to match(image_url) + expect(text).to match('<a') + end + end - expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(image_url) + context 'when images are not allowed' do + it 'removes any images' do + object = create_object("![ImageTest](/uploads/test.png)") + text = first_line_in_markdown(object, attribute, 150, project: project) + + expect(text).not_to match('<img') + expect(text).not_to match('<a') + end end context 'labels formatting' do |