Use a more powerful query to match videos in img tags

Also, always add a link to download videos since video playback is
tricky. Also, it solves the issue with email client not supporting
videos.

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 51 insertions, 20 deletions

diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb
index a8316e72a68..4dfbff8ec86 100644
--- a/lib/banzai/filter/video_link_filter.rb
+++ b/lib/banzai/filter/video_link_filter.rb
@@ -8,8 +8,8 @@ module Banzai
       include ActionView::Context
 
       def call
-        doc.search('img').each do |el|
-          el.replace(video_tag(doc, el)) if video?(el)
+        doc.xpath(query).each do |el|
+          el.replace(video_node(doc, el))
         end
 
         doc
@@ -17,19 +17,44 @@ module Banzai
 
       private
 
-      def video?(element)
-        extension = File.extname(element.attribute('src').value).delete('.')
-        UploaderHelper::VIDEO_EXT.include?(extension)
+      def query
+        @query ||= begin
+          src_query = UploaderHelper::VIDEO_EXT.map do |ext|
+            "'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})"
+          end
+
+          "descendant-or-self::img[not(ancestor::a) and (#{src_query.join(' or ')})]"
+        end
       end
 
       # Return a video tag Nokogiri node
       #
       def video_node(doc, element)
-        doc.document.create_element(
+        container = doc.document.create_element(
+          'div',
+          class: 'video-container'
+        )
+
+        video = doc.document.create_element(
           'video',
-          src: element.attribute('src').value,
-          class: 'video-js',
-          controls: true)
+          src: element['src'],
+          class: 'video-js vjs-sublime-skin',
+          controls: true,
+          "data-setup": '{}')
+
+        link = doc.document.create_element(
+          'a',
+          element['title'] || element['alt'],
+          href: element['src'],
+          target: '_blank',
+          title: "Downlad '#{element['title'] || element['alt']}'")
+        download_paragraph = doc.document.create_element('p')
+        download_paragraph.children = link
+
+        container.add_child(video)
+        container.add_child(download_paragraph)
+
+        container
       end
     end
 
diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb
index d9edca7046c..8d94b199c66 100644
--- a/lib/banzai/pipeline/gfm_pipeline.rb
+++ b/lib/banzai/pipeline/gfm_pipeline.rb
@@ -7,8 +7,8 @@ module Banzai
           Filter::SanitizationFilter,
 
           Filter::UploadLinkFilter,
-          Filter::ImageLinkFilter,
           Filter::VideoLinkFilter,
+          Filter::ImageLinkFilter,
           Filter::EmojiFilter,
           Filter::TableOfContentsFilter,
           Filter::AutolinkFilter,
diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb
index 8ccfb9c0fca..bdf76c458b0 100644
--- a/spec/lib/banzai/filter/video_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/video_link_filter_spec.rb
@@ -9,28 +9,34 @@ describe Banzai::Filter::VideoLinkFilter, lib: true do
     described_class.call(doc, contexts)
   end
 
-  def image(path)
+  def link_to_image(path)
     %(<img src="#{path}" />)
   end
 
   let(:project) { create(:project) }
 
   context 'when the element src has a video extension' do
-    it 'replaces the image tag with a video tag' do
-      doc = filter(image("/path/video.mov"))
-      element = doc.children.first
-      expect(element.name).to eq( "video" )
-      expect(element['src']).to eq( "/path/video.mov" )
+    UploaderHelper::VIDEO_EXT.each do |ext|
+      it "replaces the image tag 'path/video.#{ext}' with a video tag" do
+        element = filter(link_to_image("/path/video.#{ext}")).children.first
+
+        expect(element.name).to eq 'video'
+        expect(element['src']).to eq "/path/video.#{ext}"
+
+        fallback_link = element.children.first
+        expect(fallback_link.name).to eq 'a'
+        expect(fallback_link['href']).to eq "/path/video.#{ext}"
+        expect(fallback_link['target']).to eq '_blank'
+      end
     end
   end
 
   context 'when the element src is an image' do
     it 'leaves the document unchanged' do
-      doc = filter(image("/path/my_image.jpg"))
-      element = doc.children.first
-      expect(element.name).to eq( "img" )
+      element = filter(link_to_image("/path/my_image.jpg")).children.first
+
+      expect(element.name).to eq 'img'
     end
   end
 
-
 end