Commit 822023c6 authored by Alexis Reigel's avatar Alexis Reigel

Add a '?' to the custom favicon's urls

Without the '?' at the end of the favicon url the custom favicon (i.e.
the favicons that are served through `UploadController`) are not shown
in the browser. It may have something to do with how `#send_file` /
`#send_data` set http headers. When serving the same icon file from the
public directory everything is fine.
parent bf27c684
...@@ -2,7 +2,7 @@ module Gitlab ...@@ -2,7 +2,7 @@ module Gitlab
class Favicon class Favicon
class << self class << self
def default def default
return appearance_favicon.default.url if appearance_favicon.exists? return custom_favicon_url(appearance_favicon.default.url) if appearance_favicon.exists?
return 'favicon-yellow.ico' if Gitlab::Utils.to_boolean(ENV['CANARY']) return 'favicon-yellow.ico' if Gitlab::Utils.to_boolean(ENV['CANARY'])
return 'favicon-blue.ico' if Rails.env.development? return 'favicon-blue.ico' if Rails.env.development?
...@@ -11,7 +11,7 @@ module Gitlab ...@@ -11,7 +11,7 @@ module Gitlab
def status(status_name) def status(status_name)
if appearance_favicon.exists? if appearance_favicon.exists?
appearance_favicon.public_send("status_#{status_name}").url custom_favicon_url(appearance_favicon.public_send("status_#{status_name}").url) # rubocop:disable GitlabSecurity/PublicSend
else else
dir = 'ci_favicons' dir = 'ci_favicons'
dir = File.join(dir, 'dev') if Rails.env.development? dir = File.join(dir, 'dev') if Rails.env.development?
...@@ -30,6 +30,13 @@ module Gitlab ...@@ -30,6 +30,13 @@ module Gitlab
def appearance_favicon def appearance_favicon
appearance.favicon appearance.favicon
end end
# Without the '?' at the end of the favicon url the custom favicon (i.e.
# the favicons that are served through `UploadController`) are not shown
# in the browser.
def custom_favicon_url(url)
"#{url}?"
end
end end
end end
end end
...@@ -19,7 +19,7 @@ RSpec.describe Gitlab::Favicon do ...@@ -19,7 +19,7 @@ RSpec.describe Gitlab::Favicon do
it 'uses the custom favicon if a favicon appearance is present' do it 'uses the custom favicon if a favicon appearance is present' do
create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png')) create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
expect(described_class.default).to match %r{/uploads/-/system/appearance/favicon/\d+/default_dk.ico} expect(described_class.default).to match %r{/uploads/-/system/appearance/favicon/\d+/default_dk.ico\?}
end end
end end
...@@ -32,7 +32,7 @@ RSpec.describe Gitlab::Favicon do ...@@ -32,7 +32,7 @@ RSpec.describe Gitlab::Favicon do
it 'uses the custom favicon if a favicon appearance is present' do it 'uses the custom favicon if a favicon appearance is present' do
create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png')) create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
expect(subject).to match(%r{/uploads/-/system/appearance/favicon/\d+/status_created_dk.ico}) expect(subject).to match(%r{/uploads/-/system/appearance/favicon/\d+/status_created_dk.ico\?})
end end
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