Merge branch 'dm-remove-copy-link-on-svgs' into 'master'

Don't show 'Copy content' button on text files that are not rendered as text

See merge request !10563

10 files changed, 63 insertions, 45 deletions

diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index b0ac1623fbe..6c3f3a61e0a 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -118,6 +118,10 @@ module BlobHelper
     blob && blob.text? && !blob.lfs_pointer? && !blob.only_display_raw?
   end
 
+  def blob_rendered_as_text?(blob)
+    blob_text_viewable?(blob) && blob.to_partial_path(@project) == 'text'
+  end
+
   def blob_size(blob)
     if blob.lfs_pointer?
       blob.lfs_size
diff --git a/app/models/blob.rb b/app/models/blob.rb
index 801d3442803..55872acef51 100644
--- a/app/models/blob.rb
+++ b/app/models/blob.rb
@@ -42,12 +42,16 @@ class Blob < SimpleDelegator
     size && truncated?
   end
 
+  def extension
+    extname.downcase.delete('.')
+  end
+
   def svg?
     text? && language && language.name == 'SVG'
   end
 
   def pdf?
-    name && File.extname(name) == '.pdf'
+    extension == 'pdf'
   end
 
   def ipython_notebook?
@@ -55,11 +59,15 @@ class Blob < SimpleDelegator
   end
 
   def sketch?
-    binary? && extname.downcase.delete('.') == 'sketch'
+    binary? && extension == 'sketch'
   end
 
   def stl?
-    extname.downcase.delete('.') == 'stl'
+    extension == 'stl'
+  end
+
+  def markup?
+    text? && Gitlab::MarkupHelper.markup?(name)
   end
 
   def size_within_svg_limits?
@@ -77,8 +85,10 @@ class Blob < SimpleDelegator
       else
         'text'
       end
-    elsif image? || svg?
+    elsif image?
       'image'
+    elsif svg?
+      'svg'
     elsif pdf?
       'pdf'
     elsif ipython_notebook?
@@ -87,8 +97,18 @@ class Blob < SimpleDelegator
       'sketch'
     elsif stl?
       'stl'
+    elsif markup?
+      if only_display_raw?
+        'too_large'
+      else
+        'markup'
+      end
     elsif text?
-      'text'
+      if only_display_raw?
+        'too_large'
+      else
+        'text'
+      end
     else
       'download'
     end
diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml
index aa9b852035e..fd0ae42571e 100644
--- a/app/views/projects/blob/_blob.html.haml
+++ b/app/views/projects/blob/_blob.html.haml
@@ -32,4 +32,10 @@
         = link_to 'Fork', fork_path, method: :post, class: 'btn btn-grouped btn-inverted btn-new'
         %button.js-cancel-fork-suggestion.btn.btn-grouped{ type: 'button' }
           Cancel
-    = render blob.to_partial_path(@project), blob: blob
+
+    - if blob.empty?
+      .file-content.code
+        .nothing-here-block
+          Empty file
+    - else
+      = render blob.to_partial_path(@project), blob: blob
diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml
index 6c7d389e707..c42bf3c324a 100644
--- a/app/views/projects/blob/_header.html.haml
+++ b/app/views/projects/blob/_header.html.haml
@@ -13,7 +13,7 @@
 
   .file-actions.hidden-xs
     .btn-group{ role: "group" }<
-      = copy_blob_content_button(blob) if !blame && blob_text_viewable?(blob)
+      = copy_blob_content_button(blob) if !blame && blob_rendered_as_text?(blob)
       = open_raw_file_button(namespace_project_raw_path(@project.namespace, @project, @id))
       = view_on_environment_button(@commit.sha, @path, @environment) if @environment
 
diff --git a/app/views/projects/blob/_image.html.haml b/app/views/projects/blob/_image.html.haml
index ea3cecb86a9..73877d730f5 100644
--- a/app/views/projects/blob/_image.html.haml
+++ b/app/views/projects/blob/_image.html.haml
@@ -1,15 +1,2 @@
 .file-content.image_file
-  - if blob.svg?
-    - if blob.size_within_svg_limits?
-      -# We need to scrub SVG but we cannot do so in the RawController: it would
-      -# be wrong/strange if RawController modified the data.
-      - blob.load_all_data!(@repository)
-      - blob = sanitize_svg(blob)
-      %img{ src: "data:#{blob.mime_type};base64,#{Base64.encode64(blob.data)}", alt: "#{blob.name}" }
-    - else
-      .nothing-here-block
-        The SVG could not be displayed as it is too large, you can
-        #{link_to('view the raw file', namespace_project_raw_path(@project.namespace, @project, @id), target: '_blank', rel: 'noopener noreferrer')}
-        instead.
-  - else
-    %img{ src: namespace_project_raw_path(@project.namespace, @project, tree_join(@commit.id, blob.path)), alt: "#{blob.name}" }
+  %img{ src: namespace_project_raw_path(@project.namespace, @project, @id), alt: blob.name }
diff --git a/app/views/projects/blob/_markup.html.haml b/app/views/projects/blob/_markup.html.haml
new file mode 100644
index 00000000000..4ee4b03ff04
--- /dev/null
+++ b/app/views/projects/blob/_markup.html.haml
@@ -0,0 +1,4 @@
+- blob.load_all_data!(@repository)
+
+.file-content.wiki
+  = render_markup(blob.name, blob.data)
diff --git a/app/views/projects/blob/_svg.html.haml b/app/views/projects/blob/_svg.html.haml
new file mode 100644
index 00000000000..93be58fc658
--- /dev/null
+++ b/app/views/projects/blob/_svg.html.haml
@@ -0,0 +1,9 @@
+- if blob.size_within_svg_limits?
+  -# We need to scrub SVG but we cannot do so in the RawController: it would
+  -# be wrong/strange if RawController modified the data.
+  - blob.load_all_data!(@repository)
+  - blob = sanitize_svg(blob)
+  .file-content.image_file
+    %img{ src: "data:#{blob.mime_type};base64,#{Base64.encode64(blob.data)}", alt: blob.name }
+- else
+  = render 'too_large'
diff --git a/app/views/projects/blob/_text.html.haml b/app/views/projects/blob/_text.html.haml
index 7b16d266982..20638f6961d 100644
--- a/app/views/projects/blob/_text.html.haml
+++ b/app/views/projects/blob/_text.html.haml
@@ -1,19 +1,2 @@
-- if blob.only_display_raw?
-  .file-content.code
-    .nothing-here-block
-      File too large, you can
-      = succeed '.' do
-        = link_to 'view the raw file', namespace_project_raw_path(@project.namespace, @project, @id), target: '_blank', rel: 'noopener noreferrer'
-
-- else
-  - blob.load_all_data!(@repository)
-
-  - if blob.empty?
-    .file-content.code
-      .nothing-here-block Empty file
-  - else
-    - if markup?(blob.name)
-      .file-content.wiki
-        = render_markup(blob.name, blob.data)
-    - else
-      = render 'shared/file_highlight', blob: blob, repository: @repository
+- blob.load_all_data!(@repository)
+= render 'shared/file_highlight', blob: blob, repository: @repository
diff --git a/app/views/projects/blob/_too_large.html.haml b/app/views/projects/blob/_too_large.html.haml
new file mode 100644
index 00000000000..a505f87df40
--- /dev/null
+++ b/app/views/projects/blob/_too_large.html.haml
@@ -0,0 +1,5 @@
+.file-content.code
+  .nothing-here-block
+    The file could not be displayed as it is too large, you can
+    #{link_to('view the raw file', namespace_project_raw_path(@project.namespace, @project, @id), target: '_blank', rel: 'noopener noreferrer')}
+    instead.
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb
index 0f29766db41..e5dd57fc4bb 100644
--- a/spec/models/blob_spec.rb
+++ b/spec/models/blob_spec.rb
@@ -55,13 +55,13 @@ describe Blob do
 
   describe '#pdf?' do
     it 'is falsey when file extension is not .pdf' do
-      git_blob = double(name: 'git_blob.txt')
+      git_blob = Gitlab::Git::Blob.new(name: 'git_blob.txt')
 
       expect(described_class.decorate(git_blob)).not_to be_pdf
     end
 
     it 'is truthy when file extension is .pdf' do
-      git_blob = double(name: 'git_blob.pdf')
+      git_blob = Gitlab::Git::Blob.new(name: 'git_blob.pdf')
 
       expect(described_class.decorate(git_blob)).to be_pdf
     end
@@ -140,7 +140,7 @@ describe Blob do
         stl?: false
       )
 
-      described_class.decorate(double).tap do |blob|
+      described_class.decorate(Gitlab::Git::Blob.new({})).tap do |blob|
         allow(blob).to receive_messages(overrides)
       end
     end
@@ -158,7 +158,7 @@ describe Blob do
 
     it 'handles SVGs' do
       blob = stubbed_blob(text?: true, svg?: true)
-      expect(blob.to_partial_path(project)).to eq 'image'
+      expect(blob.to_partial_path(project)).to eq 'svg'
     end
 
     it 'handles images' do
@@ -167,7 +167,7 @@ describe Blob do
     end
 
     it 'handles text' do
-      blob = stubbed_blob(text?: true)
+      blob = stubbed_blob(text?: true, name: 'test.txt')
       expect(blob.to_partial_path(project)).to eq 'text'
     end