Merge branch 'feature/customizable-favicon' into 'master'

Customizable favicon

Closes #15661

See merge request gitlab-org/gitlab-ce!14497

.gitlab-ci.yml

@@ -1,4 +1,4 @@
-image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.17-chrome-65.0-node-8.x-yarn-1.2-postgresql-9.6"
+image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.17-chrome-65.0-node-8.x-yarn-1.2-postgresql-9.6-graphicsmagick-1.3.29"
 
 .dedicated-runner: &dedicated-runner
   retry: 1

Gemfile

@@ -108,6 +108,7 @@ gem 'hamlit', '~> 2.6.1'
 
 # Files attachments
 gem 'carrierwave', '~> 1.2'
+gem 'mini_magick'
 
 # Drag and Drop UI
 gem 'dropzonejs-rails', '~> 0.7.1'

Gemfile.lock

@@ -499,6 +499,7 @@ GEM
       mime-types-data (~> 3.2015)
     mime-types-data (3.2016.0521)
     mimemagic (0.3.0)
+    mini_magick (4.8.0)
     mini_mime (1.0.0)
     mini_portile2 (2.3.0)
     minitest (5.7.0)
@@ -1082,6 +1083,7 @@ DEPENDENCIES
   loofah (~> 2.2)
   mail_room (~> 0.9.1)
   method_source (~> 0.8)
+  mini_magick
   minitest (~> 5.7.0)
   mousetrap-rails (~> 1.4.6)
   mysql2 (~> 0.4.10)

app/assets/javascripts/lib/utils/common_utils.js

@@ -384,6 +384,49 @@ export const backOff = (fn, timeout = 60000) => {
   });
 };
 
+export const createOverlayIcon = (iconPath, overlayPath) => {
+  const faviconImage = document.createElement('img');
+
+  return new Promise((resolve) => {
+    faviconImage.onload = () => {
+      const size = 32;
+
+      const canvas = document.createElement('canvas');
+      canvas.width = size;
+      canvas.height = size;
+
+      const context = canvas.getContext('2d');
+      context.clearRect(0, 0, size, size);
+      context.drawImage(
+        faviconImage, 0, 0, faviconImage.width, faviconImage.height, 0, 0, size, size,
+      );
+
+      const overlayImage = document.createElement('img');
+      overlayImage.onload = () => {
+        context.drawImage(
+          overlayImage, 0, 0, overlayImage.width, overlayImage.height, 0, 0, size, size,
+        );
+
+        const faviconWithOverlayUrl = canvas.toDataURL();
+
+        resolve(faviconWithOverlayUrl);
+      };
+      overlayImage.src = overlayPath;
+    };
+    faviconImage.src = iconPath;
+  });
+};
+
+export const setFaviconOverlay = (overlayPath) => {
+  const faviconEl = document.getElementById('favicon');
+
+  if (!faviconEl) { return null; }
+
+  const iconPath = faviconEl.getAttribute('data-original-href');
+
+  return createOverlayIcon(iconPath, overlayPath).then(faviconWithOverlayUrl => faviconEl.setAttribute('href', faviconWithOverlayUrl));
+};
+
 export const setFavicon = (faviconPath) => {
   const faviconEl = document.getElementById('favicon');
   if (faviconEl && faviconPath) {
@@ -393,8 +436,9 @@ export const setFavicon = (faviconPath) => {
 
 export const resetFavicon = () => {
   const faviconEl = document.getElementById('favicon');
-  const originalFavicon = faviconEl ? faviconEl.getAttribute('href') : null;
+
   if (faviconEl) {
+    const originalFavicon = faviconEl.getAttribute('data-original-href');
     faviconEl.setAttribute('href', originalFavicon);
   }
 };
@@ -403,10 +447,9 @@ export const setCiStatusFavicon = pageUrl =>
   axios.get(pageUrl)
     .then(({ data }) => {
       if (data && data.favicon) {
-        setFavicon(data.favicon);
-      } else {
-        resetFavicon();
+        return setFaviconOverlay(data.favicon);
       }
+      return resetFavicon();
     })
     .catch(resetFavicon);
 

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -36,7 +36,7 @@ import {
   notify,
   SourceBranchRemovalStatus,
 } from './dependencies';
-import { setFavicon } from '../lib/utils/common_utils';
+import { setFaviconOverlay } from '../lib/utils/common_utils';
 
 export default {
   el: '#js-vue-mr-widget',
@@ -159,8 +159,9 @@ export default {
     },
     setFaviconHelper() {
       if (this.mr.ciStatusFaviconPath) {
-        setFavicon(this.mr.ciStatusFaviconPath);
+        return setFaviconOverlay(this.mr.ciStatusFaviconPath);
       }
+      return Promise.resolve();
     },
     fetchDeployments() {
       return this.service.fetchDeployments()

app/assets/javascripts/vue_shared/components/file_icon/file_icon_map.js

@@ -513,7 +513,7 @@ const fileNameIcons = {
   'credits.md': 'credits',
   'credits.md.rendered': 'credits',
   '.flowconfig': 'flow',
-  'favicon.ico': 'favicon',
+  'favicon.png': 'favicon',
   'karma.conf.js': 'karma',
   'karma.conf.ts': 'karma',
   'karma.conf.coffee': 'karma',

app/controllers/admin/appearances_controller.rb

@@ -41,6 +41,13 @@ class Admin::AppearancesController < Admin::ApplicationController
     redirect_to admin_appearances_path, notice: 'Header logo was succesfully removed.'
   end
 
+  def favicon
+    @appearance.remove_favicon!
+    @appearance.save
+
+    redirect_to admin_appearances_path, notice: 'Favicon was succesfully removed.'
+  end
+
   private
 
   # Use callbacks to share common setup or constraints between actions.
@@ -61,6 +68,8 @@ class Admin::AppearancesController < Admin::ApplicationController
       logo_cache
       header_logo
       header_logo_cache
+      favicon
+      favicon_cache
       new_project_guidelines
       updated_by
     ]

app/controllers/concerns/uploads_actions.rb

@@ -2,7 +2,7 @@ module UploadsActions
   include Gitlab::Utils::StrongMemoize
   include SendFileUpload
 
-  UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo).freeze
+  UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo favicon).freeze
 
   def create
     link_to_file = UploadService.new(model, params[:file], uploader_class).execute
@@ -31,6 +31,11 @@ module UploadsActions
 
     disposition = uploader.image_or_video? ? 'inline' : 'attachment'
 
+    uploaders = [uploader, *uploader.versions.values]
+    uploader = uploaders.find { |version| version.filename == params[:filename] }
+
+    return render_404 unless uploader
+
     send_upload(uploader, attachment: uploader.filename, disposition: disposition)
   end
 

app/helpers/favicon_helper.rb

@@ -0,0 +1,7 @@
+module FaviconHelper
+  def favicon_extension_whitelist
+    FaviconUploader::EXTENSION_WHITELIST
+      .map { |extension| "'.#{extension}'"}
+      .to_sentence
+  end
+end

app/helpers/page_layout_helper.rb

@@ -39,10 +39,7 @@ module PageLayoutHelper
   end
 
   def favicon
-    return 'favicon-yellow.ico' if Gitlab::Utils.to_boolean(ENV['CANARY'])
-    return 'favicon-blue.ico' if Rails.env.development?
-
-    'favicon.ico'
+    Gitlab::Favicon.main
   end
 
   def page_image

app/models/appearance.rb

@@ -14,6 +14,7 @@ class Appearance < ActiveRecord::Base
 
   mount_uploader :logo,         AttachmentUploader
   mount_uploader :header_logo,  AttachmentUploader
+  mount_uploader :favicon,      FaviconUploader
 
   # Overrides CacheableAttributes.current_without_cache
   def self.current_without_cache

app/models/project_services/jira_service.rb

@@ -265,7 +265,7 @@ class JiraService < IssueTrackerService
         title: title,
         status: status,
         icon: {
-          title: 'GitLab', url16x16: asset_url('favicon.ico', host: gitlab_config.url)
+          title: 'GitLab', url16x16: asset_url(Gitlab::Favicon.main, host: gitlab_config.url)
         }
       }
     }

app/serializers/status_entity.rb

@@ -7,16 +7,7 @@ class StatusEntity < Grape::Entity
   expose :details_path
 
   expose :favicon do |status|
-    dir =
-      if Gitlab::Utils.to_boolean(ENV['CANARY'])
-        File.join('ci_favicons', 'canary')
-      elsif Rails.env.development?
-        File.join('ci_favicons', 'dev')
-      else
-        'ci_favicons'
-      end
-
-    ActionController::Base.helpers.image_path(File.join(dir, "#{status.favicon}.ico"))
+    Gitlab::Favicon.status_overlay(status.favicon)
   end
 
   expose :action, if: -> (status, _) { status.has_action? } do

app/uploaders/favicon_uploader.rb

@@ -0,0 +1,24 @@
+class FaviconUploader < AttachmentUploader
+  EXTENSION_WHITELIST = %w[png ico].freeze
+
+  include CarrierWave::MiniMagick
+
+  version :favicon_main do
+    process resize_to_fill: [32, 32]
+    process convert: 'png'
+
+    def full_filename(filename)
+      filename_for_different_format(super(filename), 'png')
+    end
+  end
+
+  def extension_whitelist
+    EXTENSION_WHITELIST
+  end
+
+  private
+
+  def filename_for_different_format(filename, format)
+    filename.chomp(File.extname(filename)) + ".#{format}"
+  end
+end

app/uploaders/uploader_helper.rb

@@ -1,6 +1,6 @@
 # Extra methods for uploader
 module UploaderHelper
-  IMAGE_EXT = %w[png jpg jpeg gif bmp tiff].freeze
+  IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze
   # 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

app/views/admin/appearances/_form.html.haml

@@ -11,13 +11,32 @@
         = image_tag @appearance.header_logo_url, class: 'appearance-light-logo-preview'
         - if @appearance.persisted?
           %br
-          = link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-logo"
+          = link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo"
         %hr
       = f.hidden_field :header_logo_cache
       = f.file_field :header_logo, class: ""
       .hint
         Maximum file size is 1MB. Pages are optimized for a 28px tall header logo
 
+  %fieldset.app_logo
+    %legend
+      Favicon:
+  .form-group.row
+    = f.label :favicon, 'Favicon', class: 'col-sm-2 col-form-label'
+    .col-sm-10
+      - if @appearance.favicon?
+        = image_tag @appearance.favicon.favicon_main.url, class: 'appearance-light-logo-preview'
+        - if @appearance.persisted?
+          %br
+          = link_to 'Remove favicon', favicon_admin_appearances_path, data: { confirm: "Favicon will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo"
+        %hr
+      = f.hidden_field :favicon_cache
+      = f.file_field :favicon, class: ''
+      .hint
+        Maximum file size is 1MB. Allowed image formats are #{favicon_extension_whitelist}.
+        %br
+        The resulting favicons will be cropped to be square and scaled down to a size of 32x32 px.
+
   %fieldset.sign-in
     %legend
       Sign in/Sign up pages:
@@ -38,7 +57,7 @@
         = image_tag @appearance.logo_url, class: 'appearance-logo-preview'
         - if @appearance.persisted?
           %br
-          = link_to 'Remove logo', logo_admin_appearances_path, data: { confirm: "Logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-logo"
+          = link_to 'Remove logo', logo_admin_appearances_path, data: { confirm: "Logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo"
         %hr
       = f.hidden_field :logo_cache
       = f.file_field :logo, class: ""

app/views/layouts/_head.html.haml

@@ -25,7 +25,7 @@
   %title= page_title(site_name)
   %meta{ name: "description", content: page_description }
 
-  = favicon_link_tag favicon, id: 'favicon'
+  = favicon_link_tag favicon, id: 'favicon', data: { original_href: favicon }, type: 'image/png'
 
   = stylesheet_link_tag "application", media: "all"
   = stylesheet_link_tag "print",       media: "print"

changelogs/unreleased/feature-customizable-favicon.yml

@@ -0,0 +1,5 @@
+---
+title: Allow changing the default favicon to a custom icon.
+merge_request: 14497
+author: Alexis Reigel
+type: added

config/initializers/mime_types.rb

@@ -15,3 +15,5 @@ Mime::Type.register "video/ogg",  :ogv
 
 Mime::Type.unregister :json
 Mime::Type.register 'application/json', :json, [LfsRequest::CONTENT_TYPE, 'application/json']
+
+Mime::Type.register 'image/x-icon', :ico

config/initializers/mini_magick.rb

@@ -0,0 +1,3 @@
+MiniMagick.configure do |config|
+  config.cli = :graphicsmagick
+end

config/locales/carrierwave.en.yml

@@ -0,0 +1,14 @@
+en:
+  errors:
+    messages:
+      carrierwave_processing_error: failed to be processed
+      carrierwave_integrity_error: is not of an allowed file type
+      carrierwave_download_error: could not be downloaded
+      extension_whitelist_error: "You are not allowed to upload %{extension} files, allowed types: %{allowed_types}"
+      extension_blacklist_error: "You are not allowed to upload %{extension} files, prohibited types: %{prohibited_types}"
+      content_type_whitelist_error: "You are not allowed to upload %{content_type} files"
+      content_type_blacklist_error: "You are not allowed to upload %{content_type} files"
+      rmagick_processing_error: "Failed to manipulate with rmagick, maybe it is not an image?"
+      mini_magick_processing_error: "Failed to manipulate with MiniMagick, maybe it is not an image? Original Error: %{e}"
+      min_size_error: "File size should be greater than %{min_size}"
+      max_size_error: "File size should be less than %{max_size}"

config/routes/admin.rb

@@ -102,6 +102,7 @@ namespace :admin do
       get :preview_sign_in
       delete :logo
       delete :header_logos
+      delete :favicon
     end
   end
 

config/routes/uploads.rb

@@ -17,7 +17,7 @@ scope path: :uploads do
   # Appearance
   get "-/system/:model/:mounted_as/:id/:filename",
       to:           "uploads#show",
-      constraints:  { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ }
+      constraints:  { model: /appearance/, mounted_as: /logo|header_logo|favicon/, filename: /.+/ }
 
   # Project markdown uploads
   get ":namespace_id/:project_id/:secret/:filename",

db/migrate/20170925184228_add_favicon_to_appearances.rb

@@ -0,0 +1,7 @@
+class AddFaviconToAppearances < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    add_column :appearances, :favicon, :string
+  end
+end

db/schema.rb

@@ -38,6 +38,7 @@ ActiveRecord::Schema.define(version: 20180603190921) do
     t.integer "cached_markdown_version"
     t.text "new_project_guidelines"
     t.text "new_project_guidelines_html"
+    t.string "favicon"
   end
 
   create_table "application_setting_terms", force: :cascade do |t|

doc/administration/index.md

@@ -49,6 +49,7 @@ Learn how to install, configure, update, and maintain your GitLab instance.
 #### Customizing GitLab's appearance
 
 - [Header logo](../customization/branded_page_and_email_header.md): Change the logo on all pages and email headers.
+- [Favicon](../customization/favicon.md): Change the default favicon to your own logo.
 - [Branded login page](../customization/branded_login_page.md): Customize the login page with your own logo, title, and description.
 - [Welcome message](../customization/welcome_message.md): Add a custom welcome message to the sign-in page.
 - ["New Project" page](../customization/new_project_page.md): Customize the text to be displayed on the page that opens whenever your users create a new project.

doc/customization/favicon.md

@@ -0,0 +1,16 @@
+# Changing the favicon
+
+> [Introduced][ce-14497] in GitLab 11.0.
+
+[ce-14497]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14497
+
+Navigate to the **Admin** area and go to the **Appearance** page.
+
+Upload the custom favicon (**Favicon**) in the section **Favicon**.
+
+![appearance](favicon/appearance.png)
+
+After saving the page, the new favicon will be shown in the browser. The main
+favicon as well as the CI status icons will show the custom icon:
+
+![custom_favicon](favicon/custom_favicon.png)

doc/user/reserved_names.md

@@ -58,7 +58,7 @@ Currently the following names are reserved as top level groups:
 - dashboard
 - deploy.html
 - explore
-- favicon.ico
+- favicon.png
 - groups
 - header_logo_dark.png
 - header_logo_light.png

lib/gitlab/favicon.rb

@@ -0,0 +1,47 @@
+module Gitlab
+  class Favicon
+    class << self
+      def main
+        return appearance_favicon.favicon_main.url if appearance_favicon.exists?
+
+        image_name =
+          if Gitlab::Utils.to_boolean(ENV['CANARY'])
+            'favicon-yellow.png'
+          elsif Rails.env.development?
+            'favicon-blue.png'
+          else
+            'favicon.png'
+          end
+
+        ActionController::Base.helpers.image_path(image_name)
+      end
+
+      def status_overlay(status_name)
+        path = File.join(
+          'ci_favicons',
+          "#{status_name}.png"
+        )
+
+        ActionController::Base.helpers.image_path(path)
+      end
+
+      def available_status_names
+        @available_status_names ||= begin
+          Dir.glob(Rails.root.join('app', 'assets', 'images', 'ci_favicons', '*.png'))
+            .map { |file| File.basename(file, '.png') }
+            .sort
+        end
+      end
+
+      private
+
+      def appearance
+        RequestStore.store[:appearance] ||= (Appearance.current || Appearance.new)
+      end
+
+      def appearance_favicon
+        appearance.favicon
+      end
+    end
+  end
+end

lib/gitlab/path_regex.rb

@@ -30,7 +30,7 @@ module Gitlab
       dashboard
       deploy.html
       explore
-      favicon.ico
+      favicon.png
       files
       groups
       health_check

spec/controllers/projects/jobs_controller_spec.rb

@@ -265,7 +265,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       expect(json_response['text']).to eq status.text
       expect(json_response['label']).to eq status.label
       expect(json_response['icon']).to eq status.icon
-      expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico"
+      expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png"
     end
   end
 

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -701,7 +701,7 @@ describe Projects::MergeRequestsController do
         expect(json_response['text']).to eq status.text
         expect(json_response['label']).to eq status.label
         expect(json_response['icon']).to eq status.icon
-        expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico"
+        expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png"
       end
     end
 

spec/controllers/projects/pipelines_controller_spec.rb

@@ -253,7 +253,7 @@ describe Projects::PipelinesController do
       expect(json_response['text']).to eq status.text
       expect(json_response['label']).to eq status.label
       expect(json_response['icon']).to eq status.icon
-      expect(json_response['favicon']).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+      expect(json_response['favicon']).to match_asset_path("/assets/ci_favicons/#{status.favicon}.png")
     end
   end
 

spec/controllers/uploads_controller_spec.rb

@@ -136,7 +136,7 @@ describe UploadsController do
       context 'for PNG files' do
         it 'returns Content-Disposition: inline' do
           note = create(:note, :with_attachment, project: project)
-          get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
+          get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png'
 
           expect(response['Content-Disposition']).to start_with('inline;')
         end
@@ -145,7 +145,7 @@ describe UploadsController do
       context 'for SVG files' do
         it 'returns Content-Disposition: attachment' do
           note = create(:note, :with_svg_attachment, project: project)
-          get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.svg'
+          get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'unsanitized.svg'
 
           expect(response['Content-Disposition']).to start_with('attachment;')
         end
@@ -164,7 +164,7 @@ describe UploadsController do
           end
 
           it "redirects to the sign in page" do
-            get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "image.png"
+            get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png"
 
             expect(response).to redirect_to(new_user_session_path)
           end
@@ -172,14 +172,14 @@ describe UploadsController do
 
         context "when the user isn't blocked" do
           it "responds with status 200" do
-            get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "image.png"
+            get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'image.png'
+              get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png'
 
               response
             end
@@ -189,14 +189,14 @@ describe UploadsController do
 
       context "when not signed in" do
         it "responds with status 200" do
-          get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "image.png"
+          get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png"
 
           expect(response).to have_gitlab_http_status(200)
         end
 
         it_behaves_like 'content not cached without revalidation' do
           subject do
-            get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'image.png'
+            get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png'
 
             response
           end
@@ -214,14 +214,14 @@ describe UploadsController do
 
         context "when not signed in" do
           it "responds with status 200" do
-            get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png"
+            get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png'
+              get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'dk.png'
 
               response
             end
@@ -234,14 +234,14 @@ describe UploadsController do
           end
 
           it "responds with status 200" do
-            get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png"
+            get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png'
+              get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'dk.png'
 
               response
             end
@@ -256,7 +256,7 @@ describe UploadsController do
 
         context "when not signed in" do
           it "redirects to the sign in page" do
-            get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png"
+            get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png"
 
             expect(response).to redirect_to(new_user_session_path)
           end
@@ -279,7 +279,7 @@ describe UploadsController do
               end
 
               it "redirects to the sign in page" do
-                get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png"
+                get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png"
 
                 expect(response).to redirect_to(new_user_session_path)
               end
@@ -287,14 +287,14 @@ describe UploadsController do
 
             context "when the user isn't blocked" do
               it "responds with status 200" do
-                get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png"
+                get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png"
 
                 expect(response).to have_gitlab_http_status(200)
               end
 
               it_behaves_like 'content not cached without revalidation' do
                 subject do
-                  get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png'
+                  get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'dk.png'
 
                   response
                 end
@@ -304,7 +304,7 @@ describe UploadsController do
 
           context "when the user doesn't have access to the project" do
             it "responds with status 404" do
-              get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png"
+              get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "dk.png"
 
               expect(response).to have_gitlab_http_status(404)
             end
@@ -319,14 +319,14 @@ describe UploadsController do
       context "when the group is public" do
         context "when not signed in" do
           it "responds with status 200" do
-            get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
+            get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png'
+              get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'dk.png'
 
               response
             end
@@ -339,14 +339,14 @@ describe UploadsController do
           end
 
           it "responds with status 200" do
-            get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
+            get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png'
+              get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'dk.png'
 
               response
             end
@@ -375,7 +375,7 @@ describe UploadsController do
               end
 
               it "redirects to the sign in page" do
-                get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
+                get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "dk.png"
 
                 expect(response).to redirect_to(new_user_session_path)
               end
@@ -383,14 +383,14 @@ describe UploadsController do
 
             context "when the user isn't blocked" do
               it "responds with status 200" do
-                get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
+                get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "dk.png"
 
                 expect(response).to have_gitlab_http_status(200)
               end
 
               it_behaves_like 'content not cached without revalidation' do
                 subject do
-                  get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png'
+                  get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'dk.png'
 
                   response
                 end
@@ -400,7 +400,7 @@ describe UploadsController do
 
           context "when the user doesn't have access to the project" do
             it "responds with status 404" do
-              get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
+              get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "dk.png"
 
               expect(response).to have_gitlab_http_status(404)
             end
@@ -420,14 +420,14 @@ describe UploadsController do
 
         context "when not signed in" do
           it "responds with status 200" do
-            get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png"
+            get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
+              get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png'
 
               response
             end
@@ -440,14 +440,14 @@ describe UploadsController do
           end
 
           it "responds with status 200" do
-            get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png"
+            get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png"
 
             expect(response).to have_gitlab_http_status(200)
           end
 
           it_behaves_like 'content not cached without revalidation' do
             subject do
-              get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
+              get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png'
 
               response
             end
@@ -462,7 +462,7 @@ describe UploadsController do
 
         context "when not signed in" do
           it "redirects to the sign in page" do
-            get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png"
+            get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png"
 
             expect(response).to redirect_to(new_user_session_path)
           end
@@ -485,7 +485,7 @@ describe UploadsController do
               end
 
               it "redirects to the sign in page" do
-                get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png"
+                get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png"
 
                 expect(response).to redirect_to(new_user_session_path)
               end
@@ -493,14 +493,14 @@ describe UploadsController do
 
             context "when the user isn't blocked" do
               it "responds with status 200" do
-                get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png"
+                get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png"
 
                 expect(response).to have_gitlab_http_status(200)
               end
 
               it_behaves_like 'content not cached without revalidation' do
                 subject do
-                  get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png'
+                  get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png'
 
                   response
                 end
@@ -510,7 +510,7 @@ describe UploadsController do
 
           context "when the user doesn't have access to the project" do
             it "responds with status 404" do
-              get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png"
+              get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "dk.png"
 
               expect(response).to have_gitlab_http_status(404)
             end
@@ -560,5 +560,43 @@ describe UploadsController do
         end
       end
     end
+
+    context 'original filename or a version filename must match' do
+      let!(:appearance) { create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'), 'image/png') }
+
+      context 'has a valid filename on the original file' do
+        it 'successfully returns the file' do
+          get :show, model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'dk.png'
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.header['Content-Disposition']).to end_with 'filename="dk.png"'
+        end
+      end
+
+      context 'has an invalid filename on the original file' do
+        it 'returns a 404' do
+          get :show, model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'bogus.png'
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'has a valid filename on the version file' do
+        it 'successfully returns the file' do
+          get :show, model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'favicon_main_dk.png'
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.header['Content-Disposition']).to end_with 'filename="favicon_main_dk.png"'
+        end
+      end
+
+      context 'has an invalid filename on the version file' do
+        it 'returns a 404' do
+          get :show, model: 'appearance', mounted_as: 'favicon', id: appearance.id, filename: 'favicon_bogusversion_dk.png'
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
   end
 end

spec/features/admin/admin_appearance_spec.rb

@@ -76,6 +76,26 @@ feature 'Admin Appearance' do
     expect(page).not_to have_css(header_logo_selector)
   end
 
+  scenario 'Favicon' do
+    sign_in(create(:admin))
+    visit admin_appearances_path
+
+    attach_file(:appearance_favicon, logo_fixture)
+    click_button 'Save'
+
+    expect(page).to have_css('.appearance-light-logo-preview')
+
+    click_link 'Remove favicon'
+
+    expect(page).not_to have_css('.appearance-light-logo-preview')
+
+    # allowed file types
+    attach_file(:appearance_favicon, Rails.root.join('spec', 'fixtures', 'sanitized.svg'))
+    click_button 'Save'
+
+    expect(page).to have_content 'Favicon You are not allowed to upload "svg" files, allowed types: png, ico'
+  end
+
   def expect_custom_sign_in_appearance(appearance)
     expect(page).to have_content appearance.title
     expect(page).to have_content appearance.description

spec/features/merge_request/user_creates_image_diff_notes_spec.rb

@@ -12,7 +12,7 @@ feature 'Merge request > User creates image diff notes', :js do
     # Stub helper to return any blob file as image from public app folder.
     # This is necessary to run this specs since we don't display repo images in capybara.
     allow_any_instance_of(DiffHelper).to receive(:diff_file_blob_raw_url).and_return('/apple-touch-icon.png')
-    allow_any_instance_of(DiffHelper).to receive(:diff_file_old_blob_raw_url).and_return('/favicon.ico')
+    allow_any_instance_of(DiffHelper).to receive(:diff_file_old_blob_raw_url).and_return('/favicon.png')
   end
 
   context 'create commit diff notes' do

spec/helpers/page_layout_helper_spec.rb

@@ -40,23 +40,6 @@ describe PageLayoutHelper do
     end
   end
 
-  describe 'favicon' do
-    it 'defaults to favicon.ico' do
-      allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
-      expect(helper.favicon).to eq 'favicon.ico'
-    end
-
-    it 'has blue favicon for development' do
-      allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('development'))
-      expect(helper.favicon).to eq 'favicon-blue.ico'
-    end
-
-    it 'has yellow favicon for canary' do
-      stub_env('CANARY', 'true')
-      expect(helper.favicon).to eq 'favicon-yellow.ico'
-    end
-  end
-
   describe 'page_image' do
     it 'defaults to the GitLab logo' do
       expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png'

spec/javascripts/jobs/mock_data.js

@@ -20,7 +20,7 @@ export default {
     group: 'success',
     has_details: true,
     details_path: '/root/ci-mock/-/jobs/4757',
-    favicon: '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+    favicon: '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
     action: {
       icon: 'retry',
       title: 'Retry',
@@ -78,7 +78,7 @@ export default {
         group: 'success',
         has_details: true,
         details_path: '/root/ci-mock/pipelines/140',
-        favicon: '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+        favicon: '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
       },
       duration: 6,
       finished_at: '2017-06-01T17:32:00.042Z',

spec/javascripts/lib/utils/common_utils_spec.js

@@ -2,6 +2,7 @@
 import axios from '~/lib/utils/axios_utils';
 import * as commonUtils from '~/lib/utils/common_utils';
 import MockAdapter from 'axios-mock-adapter';
+import { faviconDataUrl, overlayDataUrl, faviconWithOverlayDataUrl } from './mock_data';
 
 describe('common_utils', () => {
   describe('parseUrl', () => {
@@ -395,6 +396,7 @@ describe('common_utils', () => {
       const favicon = document.createElement('link');
       favicon.setAttribute('id', 'favicon');
       favicon.setAttribute('href', 'default/favicon');
+      favicon.setAttribute('data-default-href', 'default/favicon');
       document.body.appendChild(favicon);
     });
 
@@ -413,7 +415,7 @@ describe('common_utils', () => {
     beforeEach(() => {
       const favicon = document.createElement('link');
       favicon.setAttribute('id', 'favicon');
-      favicon.setAttribute('href', 'default/favicon');
+      favicon.setAttribute('data-original-href', 'default/favicon');
       document.body.appendChild(favicon);
     });
 
@@ -421,12 +423,43 @@ describe('common_utils', () => {
       document.body.removeChild(document.getElementById('favicon'));
     });
 
-    it('should reset page favicon to tanuki', () => {
+    it('should reset page favicon to the default icon', () => {
+      const favicon = document.getElementById('favicon');
+      favicon.setAttribute('href', 'new/favicon');
       commonUtils.resetFavicon();
       expect(document.getElementById('favicon').getAttribute('href')).toEqual('default/favicon');
     });
   });
 
+  describe('createOverlayIcon', () => {
+    it('should return the favicon with the overlay', (done) => {
+      commonUtils.createOverlayIcon(faviconDataUrl, overlayDataUrl).then((url) => {
+        expect(url).toEqual(faviconWithOverlayDataUrl);
+        done();
+      });
+    });
+  });
+
+  describe('setFaviconOverlay', () => {
+    beforeEach(() => {
+      const favicon = document.createElement('link');
+      favicon.setAttribute('id', 'favicon');
+      favicon.setAttribute('data-original-href', faviconDataUrl);
+      document.body.appendChild(favicon);
+    });
+
+    afterEach(() => {
+      document.body.removeChild(document.getElementById('favicon'));
+    });
+
+    it('should set page favicon to provided favicon overlay', (done) => {
+      commonUtils.setFaviconOverlay(overlayDataUrl).then(() => {
+        expect(document.getElementById('favicon').getAttribute('href')).toEqual(faviconWithOverlayDataUrl);
+        done();
+      });
+    });
+  });
+
   describe('setCiStatusFavicon', () => {
     const BUILD_URL = `${gl.TEST_HOST}/frontend-fixtures/builds-project/-/jobs/1/status.json`;
     let mock;
@@ -434,6 +467,8 @@ describe('common_utils', () => {
     beforeEach(() => {
       const favicon = document.createElement('link');
       favicon.setAttribute('id', 'favicon');
+      favicon.setAttribute('href', 'null');
+      favicon.setAttribute('data-original-href', faviconDataUrl);
       document.body.appendChild(favicon);
       mock = new MockAdapter(axios);
     });
@@ -449,7 +484,7 @@ describe('common_utils', () => {
       commonUtils.setCiStatusFavicon(BUILD_URL)
         .then(() => {
           const favicon = document.getElementById('favicon');
-          expect(favicon.getAttribute('href')).toEqual('null');
+          expect(favicon.getAttribute('href')).toEqual(faviconDataUrl);
           done();
         })
         // Error is already caught in catch() block of setCiStatusFavicon,
@@ -458,16 +493,14 @@ describe('common_utils', () => {
     });
 
     it('should set page favicon to CI status favicon based on provided status', (done) => {
-      const FAVICON_PATH = '//icon_status_success';
-
       mock.onGet(BUILD_URL).reply(200, {
-        favicon: FAVICON_PATH,
+        favicon: overlayDataUrl,
       });
 
       commonUtils.setCiStatusFavicon(BUILD_URL)
         .then(() => {
           const favicon = document.getElementById('favicon');
-          expect(favicon.getAttribute('href')).toEqual(FAVICON_PATH);
+          expect(favicon.getAttribute('href')).toEqual(faviconWithOverlayDataUrl);
           done();
         })
         .catch(done.fail);

spec/javascripts/lib/utils/mock_data.js

@@ -0,0 +1,5 @@
+export const faviconDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAMAAABEpIrGAAAABGdBTUEAALGPC/xhBQAAACBjSFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAACcFBMVEX////iQyniQyniQyniQyniQyniQyniQyniQynhRiriQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniRCniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQyniQynhQiniQiniQiniQinhQinpUSjqUSjqTyjqTyjqTyjlSCniRCniQynjRCjqTyjsZSjrWyj8oib9kSb8pyb9pib8oyb8fyb3ZSb4Zib8fCb8oyb8oyb8oyb8pCb8cSbiQyn7bCb8cib8oyb8oSb8bSbtVSjpTij8nyb8oyb8oyb8lCb2Yyf3ZCf8mCb8oyb8oyb8oyb8iib8bSbiRCn8gyb8oyb8eCbpTinrUSj8oyb8oyb8oyb8pSb8bib4Zif0YCf8byb8oyb8oyb8oyb7oib8oyb8nCbjRSn9bib8ayb8nib8oyb8oyb8oyb8kSbpTyjpTyj8jib8oyb8oyb8oyb8fib0Xyf2ZSb8gCb8oyb6pSb8oyb8dib+cCbgQCnjRSn8cCb8oib8oyb8oyb8oybqUCjnSyn8bCb8oyb8oyb8oyb8myb2YyfyXyf8oyb8oyb8hibhQSn+bib8iSb8oyb8qCb+fSbmSSnqTyj8oib9pCb1YifxXyf7pSb8oCb8pCb+mCb0fCf8pSb7hSXvcSjiQyniQinqTyj9kCb9bib9byb+cCbqUSjiRCnsVCj+cSb8pib8bCb8bSbgQCn7bCb8bibjRSn8oyb8ayb8oib8aib8pCbjRCn8pybhQinhQSn8pSb7ayb7aSb6aib8eib///8IbM+7AAAAr3RSTlMBA3NtX2vT698HGQcRLwWLiXnv++3V+eEd/R8HE2V/Y5HjyefdFw99YWfJ+/3nwQP78/HvX1VTQ/kdA2HzbQXj9fX79/3DGf379/33T/v99/f7ba33+/f1+9/18/v59V339flzF/H9+fX3/fMhBwOh9/v5/fmvBV/z+fP3Awnp9/f38+UFgff7+/37+4c77/f7/flFz/f59dFr7/v98Wnr+/f3I5/197EDBU1ZAwUD8/kLUwAAAAFiS0dEAIgFHUgAAAAHdElNRQfhBQoLHiBV6/1lAAACHUlEQVQ4y41TZXsTQRCe4FAIUigN7m7FXY+iLRQKBG2x4g7BjhZ3Le7uMoEkFJprwyQk0CC/iZnNhUZaHt4vt6/szO7cHcD/wFKjZrJWq3YMq1M3eVc9rFzXR2yQkuA3RGxkjZLGiEk9miA2tURJs1RsnhhokYYtzaU13WZDbBVnW1sjo43J2vI6tZ0lLtFeAh1M0lECneI7dGYtrUtk3RUVIKaEJR25qw27yT0s3W0qEHuPlB4RradivXo7GX36xnbo51SQ+fWHARmCgYMGDxkaxbD3SssYPmIkwKgPLrfA87EETTg/fVaSa/SYsQDjSsd7DcGEsr+BieVKmaRNBsjUtClTfUI900y/5Mt05c8oJQKYSURZ2UqYFa0w283M588JEM2BuRwI5EqT8nmmXzZf4l8XsGNfCIv4QcHFklhiBpaqAsuC4tghj+ySyOdjeJYrP7RCCuR/E5tWAqxaLcmCNSyujdxjHZdbn8UHoA0bN/GoNm8hjQJb/ZzYpo6w3TB27JRduxxqrA7YzbWCezixN8RD2Oc2/Ptlfx7o5uT1A4XMiwzj4HfEikNe7+Ew0ZGjeuW70eEYaeHjxomTiKd++E4XnKGz8d+HDufOB3Ky3RcwdNF1qZiKLyf/B44r2tWf15wV143cwI2qfi8dbtKtX6Hbd+6G74EDqkTm/QcPH/0ufFyNLXjy9NnzF9Xb8BJevYY38C+8fZcg/AF3QTYemVkCwwAAACV0RVh0ZGF0ZTpjcmVhdGUAMjAxNy0wNS0xMFQxMTozMDozMiswMjowMMzup8UAAAAldEVYdGRhdGU6bW9kaWZ5ADIwMTctMDUtMTBUMTE6MzA6MzIrMDI6MDC9sx95AAAAAElFTkSuQmCC';
+
+export const overlayDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAAAXNSR0IArs4c6QAAA85JREFUWAntVllIVGEUPv/9b46O41KplYN7PeRkti8TjQlhCUGh3MmeQugpIsGKAi2soIcIooiohxYKK2daqDAlIpIiWwxtQaJcaHE0d5tMrbn37z9XRqfR0TvVW56Hudf//uec72zfEWBCJjIwkYGJDPzvGSD/KgExN3Oi2Q+2DJgSDYQEMwItVGH1iZGmJw/Si1y+/PwVAMYYib22MYc/8hVQFgKDEfYoId0KYzagAQebsos/ewMZoeB9wdffcTYpQSaCTWHKoqSQaDk7zkIt0+aCUR8BelEHrf3dUNv9AcqbnsHtT5UKB/hTASh0SLYjnjb/CIDRJi0XiFAaJOpCD8zLpdb4NB66b1OfelthX815dtdRRfiti2aAXLvVLiMQ6olGyztGDkSo4JGGXk8/QFdGpYzpHG2GBQTDhtgVhPEaVbbVpvI6GJz22rv4TcAfrYI1x7Rj5MWWAppomKFVVb2302SFzUkZHAbkG+0b1+Gh77yNYjrmqnWTrLBLRxdvBWv8qlFujH/kYjJYyvLkj71t78zAUvzMAMnHhpN4zf9UREJhd8omyssxu1IgazQDwDnHUcNuH6vhPIE1fmuBzHt74Hn7W89jWGtcAjoaIDOFrdcMYJBkgOCoaRF0Lj0oglddDbCj6tRvKjphEpgjkzEQs2YAKsNxMzjn3nKurhzK+Ly7xe28ua8TwgMMcHJZnvvT0BPtEEKM4tDJ+C8GvIIk4ylINIXVZ0EUKJxYuh3mhCeokbudl6TtVc88dfBdLwbyaWB6zQCYQJpBYSrDGQxBQ/ZWRM2B+VNmQnVnHWx7elyNuL2/R336co7KyJR8CL9oLgEuFlREevWUkEl6uGwpVEG4FBm0OEf9N10NMgPlvWYAuNVwsWDKvcUNYsHUWTCZ13ysyFEXe6TO6aC8CUr9IiK+A05TQrc8yjwmxARHeeMAPlfQJw+AQRwu0YhL/GDXi9NwufG+S8dYkuYMqIb4SsWthotlNMOUCOM6r+G9cqXxPmd1dqrBav/o1zJy2l5/NUjJA/VORwYuFnOUaTQcPs9wMqwV++Xv8oADxKAcZ8nLPr8AoGW+xR6HSqYk3GodAz2QNj0V+Gr26dT9ASNH5239Pf0gktVNWZca8ZvfAFBprWS6hSu1pqt++Y0PD+WIwDAhIWQGtzvSHDbcodfFUFB9hg1Gjs5LXqIdFL+acFBl+FddqYwdxsWC3I70OvgfUaA65zhq2O2c8VxYcyIGFTVlXegYtvCXANCQZJMobjVcLMjtSK/IcEgyOOe8Ve5w7ryKDefp2P3+C/5ohv8HZmVLAAAAAElFTkSuQmCC';
+
+export const faviconWithOverlayDataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAGt0lEQVRYR8WWf3DT5R3H35/vN0lDiCztSuiPAEnTFhSOUamIzFGokwMH55g0E845yjbP6+4qIoiHY6JjnHLeRI6h9jgQpQcD3ImH2u00eHBwjGthKLI1TSm26S8oKYVS0vT7/X52z7ckTUhqC/yx55/kks/zeb+ez6/nIWYm/B8XJQB4SGq6lL+CJA47vvRtvWs2D0nNl/Kf0qBZxx6u23arv0QAAIHivK8BynB4ffa7BgDQXJx/ngGnw+uThgTwP5ZnMocoJAxVxZDVZ+0L5n5WF75TkMafjLdJxpSg2E+gqW1X7zk3rbpaifhLiEBgTv4mEFbpBoTyu01DoDj/dQAv9rvjtdnp/k3Yx9rgAMV5QYCsAAwAgg6vL/1OTy/2BYrzzwLIBWACuNHhrXPG+otGoKaw0JA58kqGJtOFfgPS8yWT4sz88nzj7UIIfz+wd0mRdEZPLMnp2V/8R0+JrhLbBYFHJvwWzBUxYgqYNzhG+zfEhm24MIE5ectBtP0W+y0Or29FcoDifHFSRxwAcMrh9c0YrmisXaA4r0V0U8xvopgDDq9PpCQ+Ag0/zbEbNUNbMiG9fTwkDTsKHpJa2t1Zmiw1AqLg+tMZ+R6WVVtnZ2qP6Ib+FIjh05G3lsDrB4xjUIbRDeM+WZLJYZ4B1rKMzKPm/fdyzs9qg6WT225IMnPcuYjxbPZhn57qaA00zc4/QYT7b1b/wAZmDYSLjsN1WcmiM+6jXz7JTCs1aNPASBjrtrCGOXVBLK9ph72772bc0REZcsQlkEVoOxblhaFBH0Bxi6GBWFNC8gpV0XqYSe/hI85R9o1zxr/QaZbdbmuW9oRzljRrzBRkW9JhMaTgYugKzl35DlXNJ/Fp43FImoZnz7T0ln7bLihM0g85N627vkWPgLrbvYyCvAP1+rRIWETA5QsyQlcJYOCbMRasWpALtljwSsFyeJxFYsoNWqdN1y/ildM78Y/WGjxx8TL+ol3oluy8VupKe7cfoNLdCJkdqEUPOmBJ5ksJoae91mBps5lQ6pkIm20MPiz6A3KsmcNukDe/3Ye3zh3A77Q2XqcGjslLz88i/nB8pkpSoL8nAFSTBpUN4qSxS5KB5jOGUOniCebmzFQcevSN2xKP+Fp7ajt21f8TOxU/5i45JZFS6XwcTB9HxZgUnGTRNgk31x5jet+aGU7jWw+UweOcPeyTxxoqrGL25+UwdjehSvnmOVIqcz4C8y8GAABcQwjnYI5NheikhQWT+EZmDh2ev/l7cz4U2cGmYyg78TYqVH87Kbtd1wFY4hsVQAt14zu2RiDaTUZMf/BHWD35STx37wDv94k1dLeh7MRmvDZ1GR5Inxg17dX6MPnjZfh5X6tGSqXrV2B8ACIx98UNGOlV4CxCuA6zqIeq9FQ8c68bhx7ZiIK06CQdVF+Il3y1Hq03gnDfk4Uj8zbH2T51dCPOtlW39Q+iPTl2VSMfwKPiKw8aTuhgpl1Zdqxzj8PphRWwm21xZjv9VcgYkYb52dP132PFbSYr/la0DpNtrrg9a2oqsKfB2zlwG+4nSe1z7QDjaQBi2Eh6J4QRwimYt43LwOsuB2oX7YLVMCLqTAya3xx/EwZJxtYHy3WhyMkHExebXz3zAbbXfdo7AFBRaMAz1Ypa6XoaoPejKRGteZm6D3SlWVdOcOHo/Lfj2u9aXw+WHNmA00G/DiFEO0Jd+meyk0fIf/+vLfik6Xhj4qN0v7i5HCY1bBQPk+ij9GSzNbzYNdH03kMrscARfzvHQgiBocSFTVHVCrW+u+WrpK9iCIgS1rRK93oG/1GkRJVIup8KMNs1Sw/1rUtALD36ZzRca8XeJDmPtRc18vDn5SCJViYHENY3IZTK3JkE7RAYtpdkp3bAaJeOzN+CsSMTX+wqa7ih9sbVSLI2WV3znihAJYXZPThA7M6KQoM2MniyhUxTioxTpKLMadjx8Jqh5k3S//8d9GOh92XWmP/aXLKvfHgA0ZTklL0jj9m6UR6L5+9bjFWTPLcFIWbCY1+8pHb0drWybJ4aWLQrODyAWJndzoyylNyGg0hL+bV7Ll4rKIWB5CFBxMlLj21SL4W6QjDQjwOL9n4tNt0+AADPfo+UqgXPHJLSJrkso7F6ylLMy56OFMmYACIKblvtQext8Iqp0swyLYiI3zEAbs6Ml3cXv/p3Y+ryq5KcnSKb1Jmj75P7X0Rm/UV0tvO86r/WIhORwszvkmHEehH2WMo7ikDUQUWhoaIG+NNc96Os8eMEmklE2Qy2ANTO0OrA+CwFOFBfsq8pWZ7+B25aDBxvPp+QAAAAAElFTkSuQmCC';

spec/javascripts/pipelines/graph/mock_data.js

@@ -20,7 +20,7 @@ export default {
       has_details: true,
       details_path: '/root/ci-mock/pipelines/123',
       favicon:
-        '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+        '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
     },
     duration: 9,
     finished_at: '2017-04-19T14:30:27.542Z',
@@ -40,7 +40,7 @@ export default {
               has_details: true,
               details_path: '/root/ci-mock/builds/4153',
               favicon:
-                '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+                '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
               action: {
                 icon: 'retry',
                 title: 'Retry',
@@ -65,7 +65,7 @@ export default {
                   has_details: true,
                   details_path: '/root/ci-mock/builds/4153',
                   favicon:
-                    '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+                    '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
                   action: {
                     icon: 'retry',
                     title: 'Retry',
@@ -85,7 +85,7 @@ export default {
           has_details: true,
           details_path: '/root/ci-mock/pipelines/123#test',
           favicon:
-            '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+            '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
         },
         path: '/root/ci-mock/pipelines/123#test',
         dropdown_path: '/root/ci-mock/pipelines/123/stage.json?stage=test',
@@ -105,7 +105,7 @@ export default {
               has_details: true,
               details_path: '/root/ci-mock/builds/4166',
               favicon:
-                '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+                '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
               action: {
                 icon: 'retry',
                 title: 'Retry',
@@ -130,7 +130,7 @@ export default {
                   has_details: true,
                   details_path: '/root/ci-mock/builds/4166',
                   favicon:
-                    '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+                    '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
                   action: {
                     icon: 'retry',
                     title: 'Retry',
@@ -152,7 +152,7 @@ export default {
               has_details: true,
               details_path: '/root/ci-mock/builds/4159',
               favicon:
-                '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+                '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
               action: {
                 icon: 'retry',
                 title: 'Retry',
@@ -177,7 +177,7 @@ export default {
                   has_details: true,
                   details_path: '/root/ci-mock/builds/4159',
                   favicon:
-                    '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+                    '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
                   action: {
                     icon: 'retry',
                     title: 'Retry',
@@ -197,7 +197,7 @@ export default {
           has_details: true,
           details_path: '/root/ci-mock/pipelines/123#deploy',
           favicon:
-            '/assets/ci_favicons/dev/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.ico',
+            '/assets/ci_favicons/favicon_status_success-308b4fc054cdd1b68d0865e6cfb7b02e92e3472f201507418f8eddb74ac11a59.png',
         },
         path: '/root/ci-mock/pipelines/123#deploy',
         dropdown_path: '/root/ci-mock/pipelines/123/stage.json?stage=deploy',

spec/javascripts/vue_mr_widget/mr_widget_options_spec.js

@@ -5,6 +5,7 @@ import notify from '~/lib/utils/notify';
 import { stateKey } from '~/vue_merge_request_widget/stores/state_maps';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import mockData from './mock_data';
+import { faviconDataUrl, overlayDataUrl, faviconWithOverlayDataUrl } from '../lib/utils/mock_data';
 
 const returnPromise = data => new Promise((resolve) => {
   resolve({
@@ -273,6 +274,7 @@ describe('mrWidgetOptions', () => {
       beforeEach(() => {
         const favicon = document.createElement('link');
         favicon.setAttribute('id', 'favicon');
+        favicon.setAttribute('data-original-href', faviconDataUrl);
         document.body.appendChild(favicon);
 
         faviconElement = document.getElementById('favicon');
@@ -282,10 +284,13 @@ describe('mrWidgetOptions', () => {
         document.body.removeChild(document.getElementById('favicon'));
       });
 
-      it('should call setFavicon method', () => {
-        vm.setFaviconHelper();
-
-        expect(faviconElement.getAttribute('href')).toEqual(vm.mr.ciStatusFaviconPath);
+      it('should call setFavicon method', (done) => {
+        vm.mr.ciStatusFaviconPath = overlayDataUrl;
+        vm.setFaviconHelper().then(() => {
+          expect(faviconElement.getAttribute('href')).toEqual(faviconWithOverlayDataUrl);
+          done();
+        })
+        .catch(done.fail);
       });
 
       it('should not call setFavicon when there is no ciStatusFaviconPath', () => {

spec/lib/gitlab/favicon_spec.rb

@@ -0,0 +1,52 @@
+require 'rails_helper'
+
+RSpec.describe Gitlab::Favicon, :request_store do
+  describe '.main' do
+    it 'defaults to favicon.png' do
+      allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
+      expect(described_class.main).to match_asset_path '/assets/favicon.png'
+    end
+
+    it 'has blue favicon for development' do
+      allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('development'))
+      expect(described_class.main).to match_asset_path '/assets/favicon-blue.png'
+    end
+
+    it 'has yellow favicon for canary' do
+      stub_env('CANARY', 'true')
+      expect(described_class.main).to match_asset_path 'favicon-yellow.png'
+    end
+
+    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'))
+      expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/favicon_main_dk.png}
+    end
+  end
+
+  describe '.status_overlay' do
+    subject { described_class.status_overlay('favicon_status_created') }
+
+    it 'returns the overlay for the status' do
+      expect(subject).to match_asset_path '/assets/ci_favicons/favicon_status_created.png'
+    end
+  end
+
+  describe '.available_status_names' do
+    subject { described_class.available_status_names }
+
+    it 'returns the available status names' do
+      expect(subject).to eq %w(
+        favicon_status_canceled
+        favicon_status_created
+        favicon_status_failed
+        favicon_status_manual
+        favicon_status_not_found
+        favicon_status_pending
+        favicon_status_running
+        favicon_status_skipped
+        favicon_status_success
+        favicon_status_warning
+      )
+    end
+  end
+end

spec/models/group_spec.rb

@@ -240,7 +240,7 @@ describe Group do
 
     it "is false if avatar is html page" do
       group.update_attribute(:avatar, 'uploads/avatar.html')
-      expect(group.avatar_type).to eq(["file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff"])
+      expect(group.avatar_type).to eq(["file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico"])
     end
   end
 

spec/models/project_services/jira_service_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe JiraService do
   include Gitlab::Routing
+  include AssetsHelpers
 
   describe '#options' do
     let(:service) do
@@ -164,6 +165,8 @@ describe JiraService do
     it "creates Remote Link reference in JIRA for comment" do
       @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
 
+      favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}"
+
       # Creates comment
       expect(WebMock).to have_requested(:post, @comment_url)
       # Creates Remote Link in JIRA issue fields
@@ -173,7 +176,7 @@ describe JiraService do
           object: {
             url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{merge_request.diff_head_sha}",
             title: "GitLab: Solved by commit #{merge_request.diff_head_sha}.",
-            icon: { title: "GitLab", url16x16: "http://localhost/favicon.ico" },
+            icon: { title: "GitLab", url16x16: favicon_path },
             status: { resolved: true }
           }
         )
@@ -464,4 +467,18 @@ describe JiraService do
       end
     end
   end
+
+  describe 'favicon urls', :request_store do
+    it 'includes the standard favicon' do
+      props = described_class.new.send(:build_remote_link_props, url: 'http://example.com', title: 'title')
+      expect(props[:object][:icon][:url16x16]).to match %r{^http://localhost/assets/favicon(?:-\h+).png$}
+    end
+
+    it 'includes returns the custom favicon' do
+      create :appearance, favicon: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
+
+      props = described_class.new.send(:build_remote_link_props, url: 'http://example.com', title: 'title')
+      expect(props[:object][:icon][:url16x16]).to match %r{^http://localhost/uploads/-/system/appearance/favicon/\d+/favicon_main_dk.png$}
+    end
+  end
 end

spec/models/project_spec.rb

@@ -960,7 +960,7 @@ describe Project do
 
     it 'is false if avatar is html page' do
       project.update_attribute(:avatar, 'uploads/avatar.html')
-      expect(project.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff'])
+      expect(project.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico'])
     end
   end
 

spec/models/user_spec.rb

@@ -1260,7 +1260,7 @@ describe User do
     it 'is false if avatar is html page' do
       user.update_attribute(:avatar, 'uploads/avatar.html')
 
-      expect(user.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff'])
+      expect(user.avatar_type).to eq(['file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico'])
     end
   end
 

spec/serializers/build_serializer_spec.rb

@@ -39,7 +39,7 @@ describe BuildSerializer do
         expect(subject[:label]).to eq('failed')
         expect(subject[:tooltip]).to eq('failed <br> (unknown failure)')
         expect(subject[:icon]).to eq(status.icon)
-        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.png")
       end
     end
 
@@ -54,7 +54,7 @@ describe BuildSerializer do
         expect(subject[:label]).to eq('passed')
         expect(subject[:tooltip]).to eq('passed')
         expect(subject[:icon]).to eq(status.icon)
-        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.png")
       end
     end
   end

spec/serializers/pipeline_serializer_spec.rb

@@ -179,7 +179,7 @@ describe PipelineSerializer do
         expect(subject[:text]).to eq(status.text)
         expect(subject[:label]).to eq(status.label)
         expect(subject[:icon]).to eq(status.icon)
-        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico")
+        expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.png")
       end
     end
   end

spec/serializers/status_entity_spec.rb

@@ -18,17 +18,7 @@ describe StatusEntity do
     it 'contains status details' do
       expect(subject).to include :text, :icon, :favicon, :label, :group, :tooltip
       expect(subject).to include :has_details, :details_path
-      expect(subject[:favicon]).to match_asset_path('/assets/ci_favicons/favicon_status_success.ico')
-    end
-
-    it 'contains a dev namespaced favicon if dev env' do
-      allow(Rails.env).to receive(:development?) { true }
-      expect(entity.as_json[:favicon]).to match_asset_path('/assets/ci_favicons/dev/favicon_status_success.ico')
-    end
-
-    it 'contains a canary namespaced favicon if canary env' do
-      stub_env('CANARY', 'true')
-      expect(entity.as_json[:favicon]).to match_asset_path('/assets/ci_favicons/canary/favicon_status_success.ico')
+      expect(subject[:favicon]).to match_asset_path('/assets/ci_favicons/favicon_status_success.png')
     end
   end
 end

spec/services/system_note_service_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 describe SystemNoteService do
   include Gitlab::Routing
   include RepoHelpers
+  include AssetsHelpers
 
   set(:group)    { create(:group) }
   set(:project)  { create(:project, :repository, group: group) }
@@ -769,6 +770,8 @@ describe SystemNoteService do
     end
 
     describe "new reference" do
+      let(:favicon_path) { "http://localhost/assets/#{find_asset('favicon.png').digest_path}" }
+
       before do
         allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
       end
@@ -789,7 +792,7 @@ describe SystemNoteService do
               object: {
                 url: project_commit_url(project, commit),
                 title: "GitLab: Mentioned on commit - #{commit.title}",
-                icon: { title: "GitLab", url16x16: "http://localhost/favicon.ico" },
+                icon: { title: "GitLab", url16x16: favicon_path },
                 status: { resolved: false }
               }
             )
@@ -815,7 +818,7 @@ describe SystemNoteService do
               object: {
                 url: project_issue_url(project, issue),
                 title: "GitLab: Mentioned on issue - #{issue.title}",
-                icon: { title: "GitLab", url16x16: "http://localhost/favicon.ico" },
+                icon: { title: "GitLab", url16x16: favicon_path },
                 status: { resolved: false }
               }
             )
@@ -841,7 +844,7 @@ describe SystemNoteService do
               object: {
                 url: project_snippet_url(project, snippet),
                 title: "GitLab: Mentioned on snippet - #{snippet.title}",
-                icon: { title: "GitLab", url16x16: "http://localhost/favicon.ico" },
+                icon: { title: "GitLab", url16x16: favicon_path },
                 status: { resolved: false }
               }
             )

spec/support/helpers/assets_helpers.rb

@@ -0,0 +1,15 @@
+module AssetsHelpers
+  # In a CI environment the assets are not compiled, as there is a CI job
+  # `compile-assets` that compiles them in the prepare stage for all following
+  # specs.
+  # Locally the assets are precompiled dynamically.
+  #
+  # Sprockets doesn't provide one method to access an asset for both cases.
+  def find_asset(asset_name)
+    if ENV['CI']
+      Sprockets::Railtie.build_environment(Rails.application, true)[asset_name]
+    else
+      Rails.application.assets.find_asset(asset_name)
+    end
+  end
+end

spec/uploaders/favicon_uploader_spec.rb

@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+RSpec.describe FaviconUploader do
+  include CarrierWave::Test::Matchers
+
+  let(:uploader) { described_class.new(build_stubbed(:user)) }
+
+  after do
+    uploader.remove!
+  end
+
+  def upload_fixture(filename)
+    fixture_file_upload(Rails.root.join('spec', 'fixtures', filename))
+  end
+
+  context 'versions' do
+    before do
+      uploader.store!(upload_fixture('dk.png'))
+    end
+
+    it 'has the correct format' do
+      expect(uploader.favicon_main).to be_format('png')
+    end
+
+    it 'has the correct dimensions' do
+      expect(uploader.favicon_main).to have_dimensions(32, 32)
+    end
+  end
+end