From cca27d8651be5332dc71f31c834febf131e05452 Mon Sep 17 00:00:00 2001
From: Mohamed Wael Khobalatte <w.khobalatte@openproject.com>
Date: Mon, 19 Oct 2015 16:33:27 +0200
Subject: [PATCH] Centralize favicons in one template and add Apple ones

---
 .../apple-touch-icon-120x120-precomposed.png  | Bin 0 -> 2612 bytes
 app/views/common/_favicons.html.erb           |   6 +++
 app/views/layouts/base.html.erb               |   2 +-
 spec/views/layouts/base.html.erb_spec.rb      |  37 ++++++++++++++++++
 4 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 app/assets/images/apple-touch-icon-120x120-precomposed.png
 create mode 100644 app/views/common/_favicons.html.erb

diff --git a/app/assets/images/apple-touch-icon-120x120-precomposed.png b/app/assets/images/apple-touch-icon-120x120-precomposed.png
new file mode 100644
index 0000000000000000000000000000000000000000..d329190cca34465dd77328ce6922f17d4ba0ac6d
GIT binary patch
literal 2612
zcmbuBXFMB<0>!lrVwH<)kH(5xwYOBvs-jh@MsR5_f>g~|Ay&DlqN<fk%%V!{RrA?Z
zGg0e`*oi$$%-i>SzxVn5-g_U;IbY8CeEta!&5c<Y`50+vXjn|{8Cd>I<Ua+Z`{^3b
zRL6hDH9t#ZJ(`MP0SXNb!?uY5)CzfR3kHi9v-uS=VSRB|r?n;`-RrLaHO!n?dgFWd
zj!21;5*{qOcmlAK`#F^Mmn6wOC@ehQ=vgOsbVLcMixF}n0!Ve=eHEEy7B_ePAP{W&
z7q(KVMhSlfTM2@X4?0^+wp!0w;{(ndcs^1f0cU^il=*ECXV?Dp40abAdJ0hNwUdaW
z0)d@z^FX~yHZS$-aTu_W41oW2<ictKJyU0#sB>fRVy9l^0+0uhgZ$4G*T!)D4?(44
zMclXmkKfAm2Z!Vg6TE$8@>>mGAuxgN6)uklOae!1#U#qEgw$)sKVq>jMzMDq+)F)E
zpR=yf3mmQ1@%xu-j_U77jux|WmS)i&FernN9Q^_jC-_*t3uGQtMcNcm23>B(h{c6^
zOxsphYcy-j=e-!opya@s7BmTsc0d%rm=LP{cwFUl1tZi_PV4f8Z`Y8csybN8&Ea&R
z>h>wg3t|V(vJpSi9kqgMb)%{(+n^OYb$h9U^}$;b<xA2+cUz6zY_zJfqyaaGlky3g
zRCsuo9|t4Hv8K1(p1|pFqBJ2{F%;h-8{<rKq@*3>$;+IfaC6XpZ$V|_aOBtgc1I7y
z(cV2G1{sA@g@vrTguCjgu;0@UjN(1rxkJ!*EW88x>)h!owD*bcr5)y-LkFq;D4>?p
zR3M^%LLf-N>dJGVK(m*UUrx4h>e@)-*y&tD))zDluN+loNWV#zWd)O1@sid1v?3G9
zG<@1JT^02XAxxqs0^W-{$nqPCx-nnpWx&R2=$aAXuVXN!$DFkmRpT!yDctsl8wcL-
zCbEW)>=raGjFl^DNmsYnV)o;-fGU(|UxX<>3|rj(ARw4iO~Krm=y;(`EG;<gW*yVK
zRdo#cG2Eis)Cv(MIu!=l$XmJ|!@p#)eU8?a&<lD}qnQ@2=+}stYTaxz*KYv<JkhZx
z))F(!YU=@Za1S6uC*Q5Adjyt>l1YzGMVuT_8Ei%Nx<GTai2GZy&f*3XF19HA5Y{pj
z^pxRIGi?mX|Dc%Q2z;XV7I6)vK6ie*j{AR`uz(}2>K8P5_TiwX^txJWVDv`(Wjk{p
zER><R+w1#piwxiYsP)x4#Pn#E*LKCWLyn&$UREt5#Ol0_bUG(PpluA2-RO&LRe(Gd
zO<_D!=h*d<wdlh-<>3Tm@#T2iuXK%c^!U4>MTQnRwg=HOqV)c^567uUdyWpQ6C96~
zt0UhX^lo<8d5Ec@VMmnp>oxY?Zst?V6rN97-qnd^R^+EUL$L%K4Wio8dv}vi`;E4j
zKJy68r}f5&i+bd)2chF+Oj@PTOe9@sdRqwGe_Al_JZglX&I*Rewc+vnAs62w*nFE}
zn+2}{&Ws;5Rhkt)S1g%a7#nubt_Pd%ma45E&MB$btsTmmQ``yu&OE}rKm705^<<YP
zk(vel@Iq$v$y9q-3Dq$lfVKNP#&OW!6WfQJJMnc<@fK!PNCIG|xSg_JN0$D>eGA)H
z8FoGA3l!0!B5za{9M<|$;hkucYWSv6p=>XK9rS+9+#=id$dn@(2AW(`bX@|ij8Cpf
zRf(aFw=)gm+;gmSf->CRf~v`(e0(SKdYV$UydmfAXkt6KZQ(<nX$;9tPPKo~g$Mqb
z|DFvnU02EmSn^cxl&?ShME7=g&zJ!R=!EvO-^EJ&yVp;n)E_eGPm$sjg!Nk|SsuAE
z4*oKHu;oeCFBZPCJN%`TYATiZ2O=F(iy@yZ{Lya9UW$9e-DB><J;>O83BajY4}m8N
zL=96A9rL+E^p16o9qa31N=&}H4{XX^q2J64kBJ&{?ZogK7k`!&tQ(D@6o8s>2HT(T
z5#g04T+@x<A6#BCu>v9U6_#YNebVIww%5FJ!<Im26W!ljA<{pWIyl;}Er~b&oe6D^
z4l`q!MHkB{lLu-x2;CdGm(ypjw|2>t(BCT}U^_i{TsfZAvCo~YQmDZvI_ySz-WRug
zarAMUr`DyhfS(Z#PIn9PGBiP`BMs^{<fNBP?7PWD@uNSGOwG+HVV<F#C|8{1C3KSW
zcsx>Eq4JsPQ<(vj0<{<?R+Mi9!-%RElgo9{a$)qIF75I+=RTeLAbH}VwBNO=$kqi^
z4R|ek?-ml+Lfw<|mcL#KMpk<4Gl<>-@S}u*|HTvh>n!+Ii>TaX3CARz6Tz7gaXtfO
zlv^O%L0!61x8>A$=@Ag!78LYaDn9VgHKT!?*@Y%r2fU$jhn~!E%|p)ZUG;qkkx3<!
zN4c{$wG`8Ss(A60Kc{qNaoR)?j`#!w`J|KV{QzM)l9IHXOQ;NT;D6*NdqpZGGl%_n
z$V+WgIUUeQxtxumi{D)?+?gHZx=tu&*C^Ghz_`f(S33@R;>>W1mCFSgK2jH<rG{nd
zuha+&Y4*9)57FYM>LQvJqWodJZbMtr@8p$dtw<fKoDDwiz0W11miTw)I1bAE_bWaL
z3zr{QC)UY@U#-mauTSk7tY7NbKf6noRQjmMz?Re{9KIC&>fvzC)tPUO(8VmfPW<Cr
z$C;aGbeh26`Byjw6_jU^sr8=LRo%Ld^uh*~O&oCrzqh#jw96c8ar5!B*Zb2ym>l2b
zga~RF=R_&zz(56^8}PUHBfV6}jiZPA#RToZsA)e$XF~QJ0yQIxd7fWI+tOoN;#x)V
z%i6SUH;ROxUh9MIb=FNoc!CsE=Isn923j$J7;XJBL6HT8m?gW50OskVUii-3n|}Za
zPE?a3)^z1*iR1`n@k!82W&u0TnIR}9RSxA8OW|D4lZJ&>UaQix#nju2o9|4#64<_V
zpcqYDSU2}29|OirlqJ-h_G?&!wR|n@T>v%o+3HV<&dOk4Idy%LrXyQ$--_6UJ{*w;
z8$|p<qwE)pn42V41~MnfHi>5N-{0h=9O1Aq+@t&QZ@@VZeoIWnHd%lB-tN;Ff4(r@
zQRnaML>K;Ey;Em%wMzVCwukOw<$gNxbDlY!MMr*!%8z_+oId^io8d?<j5s|bQqXFt
zpg5L#)sRdQs&)8~;VI<=_ungk)?&YF;7*LrrMe4^1{Ag_E{j**;3<QLeFGo6C2W_T
p9g0yz0Q|%CZ2u1o{hODL?}}lnD_z&WK!0K&jftVTL4}@k)ZaX&3yuH)

literal 0
HcmV?d00001

diff --git a/app/views/common/_favicons.html.erb b/app/views/common/_favicons.html.erb
new file mode 100644
index 0000000000..8baf1eddc6
--- /dev/null
+++ b/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" %>
diff --git a/app/views/layouts/base.html.erb b/app/views/layouts/base.html.erb
index c6d306b8ba..6cf4de2308 100644
--- a/app/views/layouts/base.html.erb
+++ b/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' %>
diff --git a/spec/views/layouts/base.html.erb_spec.rb b/spec/views/layouts/base.html.erb_spec.rb
index f1a5e5bdf8..1b6d4f2ba5 100644
--- a/spec/views/layouts/base.html.erb_spec.rb
+++ b/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