Remove VideoJS and clean the integration

Handle videos in:
- MD preview in notes: commit, issue/MR, MR diff
- New notes in: commit, issue/MR, MR diff
- Persisted notes in: commit, issue/MR, MR diff
Signed-off-by: Rémy Coutable <remy@rymai.me>

app/assets/javascripts/application.js.coffee

@@ -53,7 +53,6 @@
 #= require_directory ./u2f
 #= require_directory .
 #= require fuzzaldrin-plus
-#= require u2f
 
 window.slugify = (text) ->
   text.replace(/[^-a-zA-Z0-9]+/g, '_').toLowerCase()

app/assets/javascripts/video_setup.js.coffee

-# Disables dynamic sizing of video tag, which defaults to 300px.
-# Without this the video naturally fills the container.
-window.VIDEOJS_NO_DYNAMIC_STYLE = true

app/assets/stylesheets/application.scss

@@ -29,11 +29,6 @@
  */
 @import "font-awesome";
 
-/*
- * Styles for VideoJS.
- */
-@import "videojs/video-js";
-
 /*
  * Page specific styles (issues, projects etc):
  */

app/assets/stylesheets/pages/issuable.scss

@@ -268,7 +268,7 @@
   .issuable-header-btn {
     background: $gray-normal;
     border: 1px solid $border-gray-normal;
-    
+
     &:hover {
       background: $gray-dark;
       border: 1px solid $border-gray-dark;

app/controllers/help_controller.rb

@@ -30,7 +30,7 @@ class HelpController < ApplicationController
       end
 
       # Allow access to images in the doc folder
-      format.any(:png, :gif, :jpeg) do
+      format.any(:png, :gif, :jpeg, :mp4) do
         # Note: We are purposefully NOT using `Rails.root.join`
         path = File.join(Rails.root, 'doc', "#{@path}.#{params[:format]}")
 

app/uploaders/uploader_helper.rb

@@ -3,8 +3,8 @@ module UploaderHelper
   IMAGE_EXT = %w[png jpg jpeg gif bmp tiff]
   # We recommend using the .mp4 format over .mov. Videos in .mov format can
   # still be used but you really need to make sure they are served with the
-  # proper MIME type video/mp4 and not video/quicktime or your videos won’t play
-  # on IE ≥ 9.
+  # proper MIME type video/mp4 and not video/quicktime or your videos won't play
+  # on IE >= 9.
   # http://archive.sublimevideo.info/20150912/docs.sublimevideo.net/troubleshooting.html
   VIDEO_EXT = %w[mp4 m4v mov webm ogv]
 
@@ -21,6 +21,8 @@ module UploaderHelper
   end
 
   def extension_match?(extensions)
+    return false unless file
+
     extension =
       if file.respond_to?(:extension)
         file.extension

doc/markdown/markdown.md

@@ -13,6 +13,7 @@
 * [Emoji](#emoji)
 * [Special GitLab references](#special-gitlab-references)
 * [Task Lists](#task-lists)
+* [Videos](#videos)
 
 **[Standard Markdown](#standard-markdown)**
 
@@ -281,6 +282,20 @@ You can add task lists to issues, merge requests and comments. To create a task
 
 Task lists can only be created in descriptions, not in titles. Task item state can be managed by editing the description's Markdown or by toggling the rendered check boxes.
 
+## Videos
+
+Image tags with a video extension are automatically converted to a video player.
+
+The valid video extensions are `.mp4`, `.m4v`, `.mov`, `.webm`, `.ogv`, and `.ogg`.
+
+    Here's a sample video:
+
+    ![Sample Video](img/video.mp4)
+
+Here's a sample video:
+
+![Sample Video](img/video.mp4)
+
 # Standard Markdown
 
 ## Headers

lib/banzai/filter/video_link_filter.rb

 module Banzai
   module Filter
 
-    # HTML Filter that handles video uploads.
-
+    # Find every image that isn't already wrapped in an `a` tag, and that has
+    # a `src` attribute ending with a video extension, add a new video node and
+    # a "Download" link in the case the video cannot be played.
     class VideoLinkFilter < HTML::Pipeline::Filter
-      include ActionView::Helpers::TagHelper
-      include ActionView::Context
 
       def call
         doc.xpath(query).each do |el|
@@ -27,8 +26,6 @@ module Banzai
         end
       end
 
-      # Return a video tag Nokogiri node
-      #
       def video_node(doc, element)
         container = doc.document.create_element(
           'div',
@@ -38,16 +35,16 @@ module Banzai
         video = doc.document.create_element(
           'video',
           src: element['src'],
-          class: 'video-js vjs-sublime-skin',
+          width: '400',
           controls: true,
-          "data-setup": '{}')
+          'data-setup' => '{}')
 
         link = doc.document.create_element(
           'a',
           element['title'] || element['alt'],
           href: element['src'],
           target: '_blank',
-          title: "Downlad '#{element['title'] || element['alt']}'")
+          title: "Download '#{element['title'] || element['alt']}'")
         download_paragraph = doc.document.create_element('p')
         download_paragraph.children = link
 

spec/features/markdown_spec.rb

@@ -236,6 +236,14 @@ describe 'GitLab Markdown', feature: true do
     it 'includes TaskListFilter' do
       expect(doc).to parse_task_lists
     end
+
+    it 'includes InlineDiffFilter' do
+      expect(doc).to parse_inline_diffs
+    end
+
+    it 'includes VideoLinkFilter' do
+      expect(doc).to parse_video_links
+    end
   end
 
   context 'wiki pipeline' do
@@ -293,6 +301,10 @@ describe 'GitLab Markdown', feature: true do
     it 'includes InlineDiffFilter' do
       expect(doc).to parse_inline_diffs
     end
+
+    it 'includes VideoLinkFilter' do
+      expect(doc).to parse_video_links
+    end
   end
 
   # Fake a `current_user` helper

spec/fixtures/markdown.md.erb

@@ -256,3 +256,7 @@ However the wrapping tags can not be mixed as such -
 - [+ additions +}
 - {- delletions -]
 - [- delletions -}
+
+### Videos
+
+![My Video](/assets/videos/gitlab-demo.mp4)

spec/lib/banzai/filter/video_link_filter_spec.rb

@@ -18,24 +18,33 @@ describe Banzai::Filter::VideoLinkFilter, lib: true do
   context 'when the element src has a video extension' do
     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
+        container = filter(link_to_image("/path/video.#{ext}")).children.first
 
-        expect(element.name).to eq 'video'
-        expect(element['src']).to eq "/path/video.#{ext}"
+        expect(container.name).to eq 'div'
+        expect(container['class']).to eq 'video-container'
 
-        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'
+        video, paragraph = container.children
+
+        expect(video.name).to eq 'video'
+        expect(video['src']).to eq "/path/video.#{ext}"
+
+        expect(paragraph.name).to eq 'p'
+
+        link = paragraph.children.first
+
+        expect(link.name).to eq 'a'
+        expect(link['href']).to eq "/path/video.#{ext}"
+        expect(link['target']).to eq '_blank'
       end
     end
   end
 
   context 'when the element src is an image' do
     it 'leaves the document unchanged' do
-      element = filter(link_to_image("/path/my_image.jpg")).children.first
+      element = filter(link_to_image('/path/my_image.jpg')).children.first
 
       expect(element.name).to eq 'img'
+      expect(element['src']).to eq '/path/my_image.jpg'
     end
   end
 

spec/support/matchers/markdown_matchers.rb

@@ -178,6 +178,17 @@ module MarkdownMatchers
       expect(actual).to have_selector('span.idiff.deletion', count: 2)
     end
   end
+
+  # VideoLinkFilter
+  matcher :parse_video_links do
+    set_default_markdown_messages
+
+    match do |actual|
+      video = actual.at_css('video')
+
+      expect(video['src']).to end_with('/assets/videos/gitlab-demo.mp4')
+    end
+  end
 end
 
 # Monkeypatch the matcher DSL so that we can reduce some noisy duplication for

spec/uploaders/file_uploader_spec.rb

-require "spec_helper"
-
-# provides matchers like `have_dimensions`
-# https://github.com/carrierwaveuploader/carrierwave#testing-with-carrierwave
-# require "carrierwave/test/matchers"
-
+require 'spec_helper'
 
 describe FileUploader do
-  # include CarrierWave::Test::Matchers
-
-  let(:project){ create(:project) }
-
-  let(:image_file){ File.new Rails.root.join("spec", "fixtures", "rails_sample.jpg") }
-  let(:video_file){ File.new Rails.root.join("spec", "fixtures", "video_sample.mp4") }
-  let(:text_file) { File.new Rails.root.join("spec", "fixtures", "doc_sample.txt") }
+  let(:project) { create(:project) }
 
   before do
+    @previous_enable_processing = FileUploader.enable_processing
     FileUploader.enable_processing = false
     @uploader = FileUploader.new(project)
   end
 
   after do
-    FileUploader.enable_processing = true
+    FileUploader.enable_processing = @previous_enable_processing
     @uploader.remove!
   end
 
-  it "should detect an image based on file extension" do
-    @uploader.store!(image_file)
-    expect(@uploader.image_or_video?).to be true
+  describe '#image_or_video?' do
+    context 'given an image file' do
+      before do
+        @uploader.store!(File.new(Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')))
+      end
+
+      it 'detects an image based on file extension' do
+        expect(@uploader.image_or_video?).to be true
+      end
+    end
+
+    context 'given an video file' do
+      before do
+        video_file = File.new(Rails.root.join('spec', 'fixtures', 'video_sample.mp4'))
+        @uploader.store!(video_file)
+      end
+
+      it 'detects a video based on file extension' do
+        expect(@uploader.image_or_video?).to be true
+      end
+    end
+
+    it 'does not return image_or_video? for other types' do
+      @uploader.store!(File.new(Rails.root.join('spec', 'fixtures', 'doc_sample.txt')))
+
+      expect(@uploader.image_or_video?).to be false
+    end
   end
-
-  it "should detect a video based on file extension" do
-    @uploader.store!(video_file)
-    expect(@uploader.image_or_video?).to be true
-  end
-
-  it "should not return image_or_video? for other types" do
-    @uploader.store!(text_file)
-    expect(@uploader.image_or_video?).to be false
-  end
-
 end