Commit 644296d6 authored by Brett Walker's avatar Brett Walker Committed by Douwe Maan

Resolve "Wiki page attachments not rendered properly"

parent 734b8f93
...@@ -8,8 +8,9 @@ module Banzai ...@@ -8,8 +8,9 @@ module Banzai
# #
# Based on Banzai::Filter::AutolinkFilter # Based on Banzai::Filter::AutolinkFilter
# #
# CommonMark does not allow spaces in the url portion of a link. # CommonMark does not allow spaces in the url portion of a link/url.
# For example, `[example](page slug)` is not valid. However, # For example, `[example](page slug)` is not valid.
# Neither is `![example](test image.jpg)`. However,
# in our wikis, we support (via RedCarpet) this type of link, allowing # in our wikis, we support (via RedCarpet) this type of link, allowing
# wiki pages to be easily linked by 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 # The intent is for this to only be used in Wikis - in general, we want
...@@ -20,10 +21,17 @@ module Banzai ...@@ -20,10 +21,17 @@ module Banzai
# Pattern to match a standard markdown link # Pattern to match a standard markdown link
# #
# Rubular: http://rubular.com/r/z9EAHxYmKI # Rubular: http://rubular.com/r/2EXEQ49rg5
LINK_PATTERN = /\[([^\]]+)\]\(([^)"]+)(?: \"([^\"]+)\")?\)/ LINK_OR_IMAGE_PATTERN = %r{
(?<preview_operator>!)?
# Text matching LINK_PATTERN inside these elements will not be linked \[(?<text>.+?)\]
\(
(?<new_link>.+?)
(?<title>\ ".+?")?
\)
}x
# Text matching LINK_OR_IMAGE_PATTERN inside these elements will not be linked
IGNORE_PARENTS = %w(a code kbd pre script style).to_set IGNORE_PARENTS = %w(a code kbd pre script style).to_set
# The XPath query to use for finding text nodes to parse. # The XPath query to use for finding text nodes to parse.
...@@ -38,7 +46,7 @@ module Banzai ...@@ -38,7 +46,7 @@ module Banzai
doc.xpath(TEXT_QUERY).each do |node| doc.xpath(TEXT_QUERY).each do |node|
content = node.to_html content = node.to_html
next unless content.match(LINK_PATTERN) next unless content.match(LINK_OR_IMAGE_PATTERN)
html = spaced_link_filter(content) html = spaced_link_filter(content)
...@@ -53,25 +61,37 @@ module Banzai ...@@ -53,25 +61,37 @@ module Banzai
private private
def spaced_link_match(link) def spaced_link_match(link)
match = LINK_PATTERN.match(link) match = LINK_OR_IMAGE_PATTERN.match(link)
return link unless match && match[1] && match[2] return link unless match
# escape the spaces in the url so that it's a valid markdown link, # 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 # then run it through the markdown processor again, let it do its magic
text = match[1] html = Banzai::Filter::MarkdownFilter.call(transform_markdown(match), context)
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 # link is wrapped in a <p>, so strip that off
html.sub('<p>', '').chomp('</p>') html.sub('<p>', '').chomp('</p>')
end end
def spaced_link_filter(text) def spaced_link_filter(text)
Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_PATTERN) do |link, left:, right:| Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_OR_IMAGE_PATTERN) do |link, left:, right:|
spaced_link_match(link) spaced_link_match(link)
end end
end end
def transform_markdown(match)
preview_operator, text, new_link, title = process_match(match)
"#{preview_operator}[#{text}](#{new_link}#{title})"
end
def process_match(match)
[
match[:preview_operator],
match[:text],
match[:new_link].gsub(' ', '%20'),
match[:title]
]
end
end end
end end
end end
...@@ -5,7 +5,7 @@ module Banzai ...@@ -5,7 +5,7 @@ module Banzai
@filters ||= begin @filters ||= begin
super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter) super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter)
.insert_before(Filter::TaskListFilter, Filter::WikiLinkFilter) .insert_before(Filter::TaskListFilter, Filter::WikiLinkFilter)
.insert_before(Filter::WikiLinkFilter, Filter::SpacedLinkFilter) .insert_before(Filter::VideoLinkFilter, Filter::SpacedLinkFilter)
end end
end end
end end
......
...@@ -3,49 +3,73 @@ require 'spec_helper' ...@@ -3,49 +3,73 @@ require 'spec_helper'
describe Banzai::Filter::SpacedLinkFilter do describe Banzai::Filter::SpacedLinkFilter do
include FilterSpecHelper include FilterSpecHelper
let(:link) { '[example](page slug)' } let(:link) { '[example](page slug)' }
let(:image) { '![example](img test.jpg)' }
it 'converts slug with spaces to a link' do context 'when a link is detected' do
doc = filter("See #{link}") 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').text).to eq 'example'
expect(doc.at_css('a')['href']).to eq 'page%20slug' expect(doc.at_css('a')['href']).to eq 'page%20slug'
expect(doc.at_css('p')).to eq nil expect(doc.at_css('a')['title']).to be_nil
end expect(doc.at_css('p')).to be_nil
end
it 'converts slug with spaces and a title to a link' do it 'converts slug with spaces and a title to a link' do
link = '[example](page slug "title")' link = '[example](page slug "title")'
doc = filter("See #{link}") doc = filter("See #{link}")
expect(doc.at_css('a').text).to eq 'example' expect(doc.at_css('a').text).to eq 'example'
expect(doc.at_css('a')['href']).to eq 'page%20slug' expect(doc.at_css('a')['href']).to eq 'page%20slug'
expect(doc.at_css('a')['title']).to eq 'title' expect(doc.at_css('a')['title']).to eq 'title'
expect(doc.at_css('p')).to eq nil expect(doc.at_css('p')).to be_nil
end end
it 'does nothing when markdown_engine is redcarpet' do it 'does nothing when markdown_engine is redcarpet' do
exp = act = link exp = act = link
expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp
end end
it 'does nothing with empty text' do
link = '[](page slug)'
doc = filter("See #{link}")
expect(doc.at_css('a')).to be_nil
end
it 'does nothing with empty text' do it 'does nothing with an empty slug' do
link = '[](page slug)' link = '[example]()'
doc = filter("See #{link}") doc = filter("See #{link}")
expect(doc.at_css('a')).to eq nil expect(doc.at_css('a')).to be_nil
end
end end
it 'does nothing with an empty slug' do context 'when an image is detected' do
link = '[example]()' it 'converts slug with spaces to an iamge' do
doc = filter("See #{link}") doc = filter("See #{image}")
expect(doc.at_css('img')['src']).to eq 'img%20test.jpg'
expect(doc.at_css('img')['alt']).to eq 'example'
expect(doc.at_css('p')).to be_nil
end
it 'converts slug with spaces and a title to an image' do
image = '![example](img test.jpg "title")'
doc = filter("See #{image}")
expect(doc.at_css('a')).to eq nil expect(doc.at_css('img')['src']).to eq 'img%20test.jpg'
expect(doc.at_css('img')['alt']).to eq 'example'
expect(doc.at_css('img')['title']).to eq 'title'
expect(doc.at_css('p')).to be_nil
end
end end
it 'converts multiple URLs' do it 'converts multiple URLs' do
link1 = '[first](slug one)' link1 = '[first](slug one)'
link2 = '[second](http://example.com/slug two)' link2 = '[second](http://example.com/slug two)'
doc = filter("See #{link1} and #{link2}") doc = filter("See #{link1} and #{image} and #{link2}")
found_links = doc.css('a') found_links = doc.css('a')
...@@ -54,6 +78,12 @@ describe Banzai::Filter::SpacedLinkFilter do ...@@ -54,6 +78,12 @@ describe Banzai::Filter::SpacedLinkFilter do
expect(found_links[0]['href']).to eq 'slug%20one' expect(found_links[0]['href']).to eq 'slug%20one'
expect(found_links[1].text).to eq 'second' expect(found_links[1].text).to eq 'second'
expect(found_links[1]['href']).to eq 'http://example.com/slug%20two' expect(found_links[1]['href']).to eq 'http://example.com/slug%20two'
found_images = doc.css('img')
expect(found_images.size).to eq(1)
expect(found_images[0]['src']).to eq 'img%20test.jpg'
expect(found_images[0]['alt']).to eq 'example'
end end
described_class::IGNORE_PARENTS.each do |elem| described_class::IGNORE_PARENTS.each do |elem|
......
...@@ -178,4 +178,25 @@ describe Banzai::Pipeline::WikiPipeline do ...@@ -178,4 +178,25 @@ describe Banzai::Pipeline::WikiPipeline do
end end
end end
end end
describe 'videos' do
let(:namespace) { create(:namespace, name: "wiki_link_ns") }
let(:project) { create(:project, :public, name: "wiki_link_project", namespace: namespace) }
let(:project_wiki) { ProjectWiki.new(project, double(:user)) }
let(:page) { build(:wiki_page, wiki: project_wiki, page: OpenStruct.new(url_path: 'nested/twice/start-page')) }
it 'generates video html structure' do
markdown = "![video_file](video_file_name.mp4)"
output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug)
expect(output).to include('<video src="/wiki_link_ns/wiki_link_project/wikis/nested/twice/video_file_name.mp4"')
end
it 'rewrites and replaces video links names with white spaces to %20' do
markdown = "![video file](video file name.mp4)"
output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug)
expect(output).to include('<video src="/wiki_link_ns/wiki_link_project/wikis/nested/twice/video%20file%20name.mp4"')
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment