summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Gemfile1
-rw-r--r--Gemfile.lock4
-rw-r--r--app/assets/javascripts/gl_dropdown.js18
-rw-r--r--app/models/blob.rb7
-rw-r--r--app/models/blob_viewer/base.rb14
-rw-r--r--app/views/projects/blob/_viewer.html.haml14
-rw-r--r--changelogs/unreleased/dm-blob-binaryness-change.yml5
-rw-r--r--changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml4
-rw-r--r--config/boot.rb12
-rw-r--r--lib/gitlab/git/blob.rb1
-rw-r--r--spec/features/projects/blobs/blob_show_spec.rb37
-rw-r--r--spec/javascripts/gl_dropdown_spec.js4
-rw-r--r--spec/models/blob_viewer/base_spec.rb4
13 files changed, 103 insertions, 22 deletions
diff --git a/Gemfile b/Gemfile
index e197f53d9b5..715ce2bc6c2 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,6 +2,7 @@ source 'https://rubygems.org'
gem 'rails', '4.2.8'
gem 'rails-deprecated_sanitizer', '~> 1.0.3'
+gem 'bootsnap', '~> 1.0.0'
# Responders respond_to and respond_with
gem 'responders', '~> 2.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index b5f9c3beca7..d34b84df5e6 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -82,6 +82,8 @@ GEM
bindata (2.3.5)
binding_of_caller (0.7.2)
debug_inspector (>= 0.0.1)
+ bootsnap (1.0.0)
+ msgpack (~> 1.0)
bootstrap-sass (3.3.6)
autoprefixer-rails (>= 5.2.1)
sass (>= 3.3.4)
@@ -459,6 +461,7 @@ GEM
minitest (5.7.0)
mmap2 (2.2.6)
mousetrap-rails (1.4.6)
+ msgpack (1.1.0)
multi_json (1.12.1)
multi_xml (0.6.0)
multipart-post (2.0.0)
@@ -888,6 +891,7 @@ DEPENDENCIES
benchmark-ips (~> 2.3.0)
better_errors (~> 2.1.0)
binding_of_caller (~> 0.7.2)
+ bootsnap (~> 1.0.0)
bootstrap-sass (~> 3.3.0)
brakeman (~> 3.6.0)
browser (~> 2.2)
diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js
index d34561e5512..3babe273100 100644
--- a/app/assets/javascripts/gl_dropdown.js
+++ b/app/assets/javascripts/gl_dropdown.js
@@ -248,7 +248,7 @@ GitLabDropdown = (function() {
return function(data) {
_this.fullData = data;
_this.parseData(_this.fullData);
- _this.focusTextInput();
+ _this.focusTextInput(true);
if (_this.options.filterable && _this.filter && _this.filter.input && _this.filter.input.val() && _this.filter.input.val().trim() !== '') {
return _this.filter.input.trigger('input');
}
@@ -728,8 +728,20 @@ GitLabDropdown = (function() {
return [selectedObject, isMarking];
};
- GitLabDropdown.prototype.focusTextInput = function() {
- if (this.options.filterable) { this.filterInput.focus(); }
+ GitLabDropdown.prototype.focusTextInput = function(triggerFocus = false) {
+ if (this.options.filterable) {
+ $(':focus').blur();
+
+ this.dropdown.one('transitionend', () => {
+ this.filterInput.focus();
+ });
+
+ if (triggerFocus) {
+ // This triggers after a ajax request
+ // in case of slow requests, the dropdown transition could already be finished
+ this.dropdown.trigger('transitionend');
+ }
+ }
};
GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) {
diff --git a/app/models/blob.rb b/app/models/blob.rb
index 3869e79ba75..954d4e4d779 100644
--- a/app/models/blob.rb
+++ b/app/models/blob.rb
@@ -191,9 +191,12 @@ class Blob < SimpleDelegator
rendered_as_text? && rich_viewer
end
+ def expanded?
+ !!@expanded
+ end
+
def expand!
- simple_viewer&.expanded = true
- rich_viewer&.expanded = true
+ @expanded = true
end
private
diff --git a/app/models/blob_viewer/base.rb b/app/models/blob_viewer/base.rb
index d2aa33d994e..35965d01692 100644
--- a/app/models/blob_viewer/base.rb
+++ b/app/models/blob_viewer/base.rb
@@ -6,15 +6,15 @@ module BlobViewer
self.loading_partial_name = 'loading'
- delegate :partial_path, :loading_partial_path, :rich?, :simple?, :text?, :binary?, to: :class
+ delegate :partial_path, :loading_partial_path, :rich?, :simple?, :load_async?, :text?, :binary?, to: :class
attr_reader :blob
- attr_accessor :expanded
delegate :project, to: :blob
def initialize(blob)
@blob = blob
+ @initially_binary = blob.binary?
end
def self.partial_path
@@ -57,14 +57,10 @@ module BlobViewer
false
end
- def load_async?
- self.class.load_async? && render_error.nil?
- end
-
def collapsed?
return @collapsed if defined?(@collapsed)
- @collapsed = !expanded && collapse_limit && blob.raw_size > collapse_limit
+ @collapsed = !blob.expanded? && collapse_limit && blob.raw_size > collapse_limit
end
def too_large?
@@ -73,6 +69,10 @@ module BlobViewer
@too_large = size_limit && blob.raw_size > size_limit
end
+ def binary_detected_after_load?
+ !@initially_binary && blob.binary?
+ end
+
# This method is used on the server side to check whether we can attempt to
# render the blob at all. Human-readable error messages are found in the
# `BlobHelper#blob_render_error_reason` helper.
diff --git a/app/views/projects/blob/_viewer.html.haml b/app/views/projects/blob/_viewer.html.haml
index 4252f27d007..013f1c267c8 100644
--- a/app/views/projects/blob/_viewer.html.haml
+++ b/app/views/projects/blob/_viewer.html.haml
@@ -1,13 +1,19 @@
- hidden = local_assigns.fetch(:hidden, false)
- render_error = viewer.render_error
-- load_async = local_assigns.fetch(:load_async, viewer.load_async?)
+- load_async = local_assigns.fetch(:load_async, viewer.load_async? && render_error.nil?)
- viewer_url = local_assigns.fetch(:viewer_url) { url_for(params.merge(viewer: viewer.type, format: :json)) } if load_async
.blob-viewer{ data: { type: viewer.type, url: viewer_url }, class: ('hidden' if hidden) }
- - if load_async
- = render viewer.loading_partial_path, viewer: viewer
- - elsif render_error
+ - if render_error
= render 'projects/blob/render_error', viewer: viewer
+ - elsif load_async
+ = render viewer.loading_partial_path, viewer: viewer
- else
- viewer.prepare!
+
+ -# In the rare case where the first kilobyte of the file looks like text,
+ -# but the file turns out to actually be binary after loading all data,
+ -# we fall back on the binary Download viewer.
+ - viewer = BlobViewer::Download.new(viewer.blob) if viewer.binary_detected_after_load?
+
= render viewer.partial_path, viewer: viewer
diff --git a/changelogs/unreleased/dm-blob-binaryness-change.yml b/changelogs/unreleased/dm-blob-binaryness-change.yml
new file mode 100644
index 00000000000..f3e3af26f12
--- /dev/null
+++ b/changelogs/unreleased/dm-blob-binaryness-change.yml
@@ -0,0 +1,5 @@
+---
+title: Detect if file that appears to be text in the first 1024 bytes is actually
+ binary afer loading all data
+merge_request:
+author:
diff --git a/changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml b/changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml
new file mode 100644
index 00000000000..05d52fcad0f
--- /dev/null
+++ b/changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml
@@ -0,0 +1,4 @@
+---
+title: Fixed dropdown filter input not focusing after transition
+merge_request:
+author:
diff --git a/config/boot.rb b/config/boot.rb
index f2830ae3166..17a71148370 100644
--- a/config/boot.rb
+++ b/config/boot.rb
@@ -4,3 +4,15 @@ require 'rubygems'
ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__)
require 'bundler/setup' if File.exist?(ENV['BUNDLE_GEMFILE'])
+
+# Default Bootsnap configuration from https://github.com/Shopify/bootsnap#usage
+require 'bootsnap'
+Bootsnap.setup(
+ cache_dir: 'tmp/cache',
+ development_mode: ENV['RAILS_ENV'] == 'development',
+ load_path_cache: true,
+ autoload_paths_cache: true,
+ disable_trace: false,
+ compile_cache_iseq: true,
+ compile_cache_yaml: true
+)
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index d60e607b02b..33a7624e303 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -123,6 +123,7 @@ module Gitlab
@loaded_all_data = true
@data = repository.lookup(id).content
@loaded_size = @data.bytesize
+ @binary = nil
end
def name
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb
index 82cfbfda157..45fdb36e506 100644
--- a/spec/features/projects/blobs/blob_show_spec.rb
+++ b/spec/features/projects/blobs/blob_show_spec.rb
@@ -3,8 +3,8 @@ require 'spec_helper'
feature 'File blob', :js, feature: true do
let(:project) { create(:project, :public) }
- def visit_blob(path, fragment = nil)
- visit namespace_project_blob_path(project.namespace, project, File.join('master', path), anchor: fragment)
+ def visit_blob(path, anchor: nil, ref: 'master')
+ visit namespace_project_blob_path(project.namespace, project, File.join(ref, path), anchor: anchor)
wait_for_requests
end
@@ -101,7 +101,7 @@ feature 'File blob', :js, feature: true do
context 'visiting with a line number anchor' do
before do
- visit_blob('files/markdown/ruby-style-guide.md', 'L1')
+ visit_blob('files/markdown/ruby-style-guide.md', anchor: 'L1')
end
it 'displays the blob using the simple viewer' do
@@ -352,6 +352,37 @@ feature 'File blob', :js, feature: true do
end
end
+ context 'binary file that appears to be text in the first 1024 bytes' do
+ before do
+ visit_blob('encoding/binary-1.bin', ref: 'binary-encoding')
+ end
+
+ it 'displays the blob' do
+ aggregate_failures do
+ # shows a download link
+ expect(page).to have_link('Download (23.8 KB)')
+
+ # does not show a viewer switcher
+ expect(page).not_to have_selector('.js-blob-viewer-switcher')
+
+ # The specs below verify an arguably incorrect result, but since we only
+ # learn that the file is not actually text once the text viewer content
+ # is loaded asynchronously, there is no straightforward way to get these
+ # synchronously loaded elements to display correctly.
+ #
+ # Clicking the copy button will result in nothing being copied.
+ # Clicking the raw button will result in the binary file being downloaded,
+ # as expected.
+
+ # shows an enabled copy button, incorrectly
+ expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)')
+
+ # shows a raw button, incorrectly
+ expect(page).to have_link('Open raw')
+ end
+ end
+ end
+
context '.gitlab-ci.yml' do
before do
project.add_master(project.creator)
diff --git a/spec/javascripts/gl_dropdown_spec.js b/spec/javascripts/gl_dropdown_spec.js
index 3292590b9ed..10fcc590c89 100644
--- a/spec/javascripts/gl_dropdown_spec.js
+++ b/spec/javascripts/gl_dropdown_spec.js
@@ -185,7 +185,7 @@ import '~/lib/utils/url_utility';
expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR));
});
- it('should focus on input when opening for the second time', () => {
+ it('should focus on input when opening for the second time after transition', () => {
remoteCallback();
this.dropdownContainerElement.trigger({
type: 'keyup',
@@ -193,6 +193,7 @@ import '~/lib/utils/url_utility';
keyCode: ARROW_KEYS.ESC
});
this.dropdownButtonElement.click();
+ this.dropdownContainerElement.trigger('transitionend');
expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR));
});
});
@@ -201,6 +202,7 @@ import '~/lib/utils/url_utility';
it('should focus input when passing array data to drop down', () => {
initDropDown.call(this, false, true);
this.dropdownButtonElement.click();
+ this.dropdownContainerElement.trigger('transitionend');
expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR));
});
});
diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb
index d56379eb59d..574438838d8 100644
--- a/spec/models/blob_viewer/base_spec.rb
+++ b/spec/models/blob_viewer/base_spec.rb
@@ -106,9 +106,9 @@ describe BlobViewer::Base, model: true do
end
describe '#render_error' do
- context 'when expanded' do
+ context 'when the blob is expanded' do
before do
- viewer.expanded = true
+ blob.expand!
end
context 'when the blob size is larger than the size limit' do
View Lorry Controller Status