handle link title properly and add specs48869-wiki-slugs-with-spaces

3 files changed, 85 insertions, 9 deletions

diff --git a/changelogs/unreleased/48869-wiki-slugs-with-spaces.yml b/changelogs/unreleased/48869-wiki-slugs-with-spaces.yml
new file mode 100644
index 00000000000..88ba8028e2c
--- /dev/null
+++ b/changelogs/unreleased/48869-wiki-slugs-with-spaces.yml
@@ -0,0 +1,5 @@
+---
+title: Allow spaces in wiki markdown links when using CommonMark
+merge_request: 20417
+author:
+type: fixed
diff --git a/lib/banzai/filter/spaced_link_filter.rb b/lib/banzai/filter/spaced_link_filter.rb
index 1d21b8174ab..574a8a6c7a5 100644
--- a/lib/banzai/filter/spaced_link_filter.rb
+++ b/lib/banzai/filter/spaced_link_filter.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'uri'
 
 module Banzai
@@ -9,7 +11,7 @@ module Banzai
     # CommonMark does not allow spaces in the url portion of a link.
     # For example, `[example](page slug)` is not valid.  However,
     # in our wikis, we support (via RedCarpet) this type of link, allowing
-    # wiki pages to be easily linked bby their title.  This filter adds that functionality.
+    # wiki pages to be easily linked by their title.  This filter adds that functionality.
     # The intent is for this to only be used in Wikis - in general, we want
     # to adhere to CommonMark's spec.
     #
@@ -52,14 +54,17 @@ module Banzai
 
       def spaced_link_match(link)
         match = LINK_PATTERN.match(link)
-        if match && match[1] && match[2]
-          # escape the spaces in the url so that it's a valid markdown link,
-          # then run it through the markdown processor again, let it do it's magic
-          new_link = "[#{match[1]}](#{match[2].gsub(' ', '%20')})"
-          Banzai::Filter::MarkdownFilter.new(new_link, context).call
-        else
-          link
-        end
+        return link unless match && match[1] && match[2]
+
+        # escape the spaces in the url so that it's a valid markdown link,
+        # then run it through the markdown processor again, let it do its magic
+        text     = match[1]
+        new_link = match[2].gsub(' ', '%20')
+        title    = match[3] ? " \"#{match[3]}\"" : ''
+        html     = Banzai::Filter::MarkdownFilter.call("[#{text}](#{new_link}#{title})", context)
+
+        # link is wrapped in a <p>, so strip that off
+        html.sub('<p>', '').chomp('</p>')
       end
 
       def spaced_link_filter(text)
diff --git a/spec/lib/banzai/filter/spaced_link_filter_spec.rb b/spec/lib/banzai/filter/spaced_link_filter_spec.rb
new file mode 100644
index 00000000000..4463c011522
--- /dev/null
+++ b/spec/lib/banzai/filter/spaced_link_filter_spec.rb
@@ -0,0 +1,66 @@
+require 'spec_helper'
+
+describe Banzai::Filter::SpacedLinkFilter do
+  include FilterSpecHelper
+
+  let(:link) { '[example](page slug)' }
+
+  it 'converts slug with spaces to a link' do
+    doc = filter("See #{link}")
+
+    expect(doc.at_css('a').text).to eq 'example'
+    expect(doc.at_css('a')['href']).to eq 'page%20slug'
+    expect(doc.at_css('p')).to eq nil
+  end
+
+  it 'converts slug with spaces and a title to a link' do
+    link = '[example](page slug "title")'
+    doc  = filter("See #{link}")
+
+    expect(doc.at_css('a').text).to eq 'example'
+    expect(doc.at_css('a')['href']).to eq 'page%20slug'
+    expect(doc.at_css('a')['title']).to eq 'title'
+    expect(doc.at_css('p')).to eq nil
+  end
+
+  it 'does nothing when markdown_engine is redcarpet' do
+    exp = act = link
+    expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp
+  end
+
+  it 'does nothing with empty text' do
+    link = '[](page slug)'
+    doc  = filter("See #{link}")
+
+    expect(doc.at_css('a')).to eq nil
+  end
+
+  it 'does nothing with an empty slug' do
+    link = '[example]()'
+    doc  = filter("See #{link}")
+
+    expect(doc.at_css('a')).to eq nil
+  end
+
+  it 'converts multiple URLs' do
+    link1 = '[first](slug one)'
+    link2 = '[second](http://example.com/slug two)'
+    doc   = filter("See #{link1} and #{link2}")
+
+    found_links = doc.css('a')
+
+    expect(found_links.size).to eq(2)
+    expect(found_links[0].text).to eq 'first'
+    expect(found_links[0]['href']).to eq 'slug%20one'
+    expect(found_links[1].text).to eq 'second'
+    expect(found_links[1]['href']).to eq 'http://example.com/slug%20two'
+  end
+
+  described_class::IGNORE_PARENTS.each do |elem|
+    it "ignores valid links contained inside '#{elem}' element" do
+      exp = act = "<#{elem}>See #{link}</#{elem}>"
+
+      expect(filter(act).to_html).to eq exp
+    end
+  end
+end