Merge pull request #3654 from why-el/21785-various-app-errors

Centralize favicons in one template and add Apple ones

app/views/common/_favicons.html.erb

@@ -0,0 +1,6 @@
+<%= favicon_link_tag 'favicon.ico' %>
+
+<%# In the future we can add more sizes should we see 404 errors in the logs. %>
+<%= favicon_link_tag "apple-touch-icon-120x120-precomposed.png", rel: 'apple-touch-icon',
+															 type: 'image/png',
+															 sizes: "120x120" %>

app/views/layouts/base.html.erb

@@ -38,7 +38,7 @@ See doc/COPYRIGHT.rdoc for more details.
     <meta name="current_menu_item" content="<%= current_menu_item %>" />
     <meta name="accessibility-mode" content="<%= current_user.impaired? %>" />
     <%= csrf_meta_tags %>
-    <%= favicon_link_tag 'favicon.ico' %>
+    <%= render 'common/favicons' %>
     <%= stylesheet_link_tag current_theme.stylesheet_manifest, :media => "all" %>
     <% if User.current.impaired? && accessibility_css_enabled? %>
       <%= stylesheet_link_tag 'accessibility' %>

spec/views/layouts/base.html.erb_spec.rb

@@ -29,6 +29,11 @@
 require 'spec_helper'
 
 describe 'layouts/base', type: :view do
+  # This is to make `visit` available. It might be already included by the time
+  # we reach this spec, but for running this spec alone we need it here. Best
+  # of both worlds.
+  include Capybara::DSL
+
   include Redmine::MenuManager::MenuHelper
   helper Redmine::MenuManager::MenuHelper
   let!(:user) { FactoryGirl.create :user }
@@ -124,4 +129,36 @@ describe 'layouts/base', type: :view do
       end
     end
   end
+
+
+  describe 'icons' do
+    before do
+      allow(User).to receive(:current).and_return anonymous
+      allow(view).to receive(:current_user).and_return anonymous
+      render
+    end
+
+    it 'renders main favicon' do
+      expect(rendered).to have_selector("link[rel='shortcut icon'][type='image/x-icon'][href='/assets/favicon.ico']", visible: false)
+    end
+
+    it 'renders apple icons' do
+      expect(rendered).to have_selector("link[rel='apple-touch-icon'][type='image/png'][href='/assets/apple-touch-icon-120x120-precomposed.png']", visible: false)
+    end
+
+    # We perform a get request against the icons to ensure they are there (and
+    # avoid 404 errors in production). Should you continue to see 404s in production,
+    # ensure your asset cache is not stale.
+
+    # We do this here as opposed to a request spec to 1. keep icon specs contained
+    # in one place, and 2. the view itself makes this request, so this is an appropriate
+    # location for it.
+    it 'icons actually exist' do
+      visit 'assets/favicon.ico'
+      expect(page.status_code).to eq(200)
+
+      visit 'apple-touch-icon-120x120-precomposed.png'
+      expect(page.status_code).to eq(200)
+    end
+  end
 end