From 4ba5cc862a5786d43afce8f18852f4c5c60eae42 Mon Sep 17 00:00:00 2001 From: Aileen Nowak Date: Mon, 10 Apr 2017 18:04:46 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20=20Blog=20icon=20improvements=20?= =?UTF-8?q?(#8298)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #7688 - renders the correct `/favicon.ico` or `/favcicon.png` in `{{ghost_head}}` - removes an regex issue in `serve-favicon` --- core/server/data/meta/asset_url.js | 2 +- core/server/helpers/ghost_head.js | 7 +-- core/server/middleware/serve-favicon.js | 2 +- core/test/unit/metadata/asset_url_spec.js | 4 +- .../unit/middleware/serve-favicon_spec.js | 20 +++++++ core/test/unit/server_helpers/asset_spec.js | 8 +-- .../unit/server_helpers/ghost_head_spec.js | 50 +++++++++--------- core/test/utils/fixtures/images/myicon.ico | Bin 0 -> 15086 bytes 8 files changed, 57 insertions(+), 36 deletions(-) create mode 100644 core/test/utils/fixtures/images/myicon.ico diff --git a/core/server/data/meta/asset_url.js b/core/server/data/meta/asset_url.js index ddfb21e1df..1b312ed671 100644 --- a/core/server/data/meta/asset_url.js +++ b/core/server/data/meta/asset_url.js @@ -8,7 +8,7 @@ var config = require('../../config'), */ function getFaviconUrl() { if (settingsCache.get('icon')) { - return utils.url.urlJoin(utils.url.getSubdir(), utils.url.urlFor('image', {image: settingsCache.get('icon')})); + return settingsCache.get('icon').match(/\.ico$/i) ? utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico') : utils.url.urlJoin(utils.url.getSubdir(), '/favicon.png'); } return utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico'); diff --git a/core/server/helpers/ghost_head.js b/core/server/helpers/ghost_head.js index c19be9b9dc..4d2674bcd4 100644 --- a/core/server/helpers/ghost_head.js +++ b/core/server/helpers/ghost_head.js @@ -98,8 +98,9 @@ module.exports = function ghost_head(options) { }, blogIcon = settingsCache.get('icon'), // CASE: blog icon is not set in config, we serve the default - iconType = !blogIcon ? 'x-icon' : blogIcon.match(/\/favicon\.ico$/i) ? 'x-icon' : 'png', - favicon = !blogIcon ? '/favicon.ico' : url.urlFor('image', {image: blogIcon}); + iconType = !blogIcon ? 'x-icon' : blogIcon.match(/\.ico$/i) ? 'x-icon' : 'png', + favicon = !blogIcon ? url.urlFor({relativeUrl: '/favicon.ico'}) : + blogIcon.match(/\.ico$/i) ? url.urlFor({relativeUrl: '/favicon.ico'}) : url.urlFor({relativeUrl: '/favicon.png'}); return Promise.props(fetch).then(function (response) { client = response.client; @@ -111,7 +112,7 @@ module.exports = function ghost_head(options) { head.push(''); } - head.push(''); + head.push(''); head.push(''); head.push(''); diff --git a/core/server/middleware/serve-favicon.js b/core/server/middleware/serve-favicon.js index a496a44c3a..9deea9fd72 100644 --- a/core/server/middleware/serve-favicon.js +++ b/core/server/middleware/serve-favicon.js @@ -51,7 +51,7 @@ function serveFavicon() { storage.getStorage() .read({path: filePath}) .then(function readFile(buf) { - iconType = settingsCache.get('icon').match(/\/favicon\.ico$/i) ? 'x-icon' : 'png'; + iconType = settingsCache.get('icon').match(/\.ico$/i) ? 'x-icon' : 'png'; content = buildContentResponse(iconType, buf); res.writeHead(200, content.headers); diff --git a/core/test/unit/metadata/asset_url_spec.js b/core/test/unit/metadata/asset_url_spec.js index 315792677a..9e2c921d0a 100644 --- a/core/test/unit/metadata/asset_url_spec.js +++ b/core/test/unit/metadata/asset_url_spec.js @@ -44,7 +44,7 @@ describe('getAssetUrl', function () { testUrl.should.equal('/favicon.ico'); }); - it.skip('should correct favicon path for custom png', function () { + it('should correct favicon path for custom png', function () { sandbox.stub(settingsCache, 'get').withArgs('icon').returns('/content/images/2017/04/my-icon.png'); var testUrl = getAssetUrl('favicon.ico'); testUrl.should.equal('/favicon.png'); @@ -113,7 +113,7 @@ describe('getAssetUrl', function () { testUrl.should.equal('/blog/favicon.ico'); }); - it.skip('should return correct favicon path for custom png', function () { + it('should return correct favicon path for custom png', function () { sandbox.stub(settingsCache, 'get').withArgs('icon').returns('/content/images/2017/04/my-icon.png'); var testUrl = getAssetUrl('favicon.ico'); testUrl.should.equal('/blog/favicon.png'); diff --git a/core/test/unit/middleware/serve-favicon_spec.js b/core/test/unit/middleware/serve-favicon_spec.js index d2ec0f77ec..5db57193a0 100644 --- a/core/test/unit/middleware/serve-favicon_spec.js +++ b/core/test/unit/middleware/serve-favicon_spec.js @@ -88,6 +88,26 @@ describe('Serve Favicon', function () { middleware(req, res, next); }); + it('custom uploaded myicon.ico', function (done) { + var middleware = serveFavicon(); + req.path = '/favicon.ico'; + + storage.getStorage().storagePath = path.join(__dirname, '../../../test/utils/fixtures/images/'); + localSettingsCache.icon = 'myicon.ico'; + + res = { + writeHead: function (statusCode) { + statusCode.should.eql(200); + }, + end: function (body) { + body.length.should.eql(15086); + done(); + } + }; + + middleware(req, res, next); + }); + it('default favicon.ico', function (done) { var middleware = serveFavicon(); req.path = '/favicon.ico'; diff --git a/core/test/unit/server_helpers/asset_spec.js b/core/test/unit/server_helpers/asset_spec.js index 1d12ca0a7a..ca49fc428f 100644 --- a/core/test/unit/server_helpers/asset_spec.js +++ b/core/test/unit/server_helpers/asset_spec.js @@ -42,14 +42,14 @@ describe('{{asset}} helper', function () { // with png rendered = helpers.asset('favicon.png'); should.exist(rendered); - String(rendered).should.equal('/content/images/favicon.png'); + String(rendered).should.equal('/favicon.png'); localSettingsCache.icon = '/content/images/favicon.ico'; // with ico rendered = helpers.asset('favicon.ico'); should.exist(rendered); - String(rendered).should.equal('/content/images/favicon.ico'); + String(rendered).should.equal('/favicon.ico'); }); it('handles public assets correctly', function () { @@ -96,14 +96,14 @@ describe('{{asset}} helper', function () { // with png rendered = helpers.asset('favicon.png'); should.exist(rendered); - String(rendered).should.equal('/blog/content/images/favicon.png'); + String(rendered).should.equal('/blog/favicon.png'); localSettingsCache.icon = '/content/images/favicon.ico'; // with ico rendered = helpers.asset('favicon.ico'); should.exist(rendered); - String(rendered).should.equal('/blog/content/images/favicon.ico'); + String(rendered).should.equal('/blog/favicon.ico'); }); it('handles public assets correctly', function () { diff --git a/core/test/unit/server_helpers/ghost_head_spec.js b/core/test/unit/server_helpers/ghost_head_spec.js index f34827f07d..4daa5ad3ac 100644 --- a/core/test/unit/server_helpers/ghost_head_spec.js +++ b/core/test/unit/server_helpers/ghost_head_spec.js @@ -51,7 +51,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['paged', 'index']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -69,7 +69,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['home', 'index']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -123,7 +123,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['page']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -171,7 +171,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['tag']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -214,7 +214,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['tag']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -256,7 +256,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['tag']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.not.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -308,7 +308,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['author']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -360,7 +360,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['paged', 'author']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -378,7 +378,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: []}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -416,7 +416,7 @@ describe('{{ghost_head}} helper', function () { re4 = new RegExp('"dateModified": "' + post.updated_at); should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -496,7 +496,7 @@ describe('{{ghost_head}} helper', function () { re4 = new RegExp('"dateModified": "' + post.updated_at); should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.not.match(//); rendered.string.should.match(//); @@ -576,7 +576,7 @@ describe('{{ghost_head}} helper', function () { re4 = new RegExp('"dateModified": "' + post.updated_at); should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -654,7 +654,7 @@ describe('{{ghost_head}} helper', function () { re4 = new RegExp('"dateModified": "' + post.updated_at); should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -730,7 +730,7 @@ describe('{{ghost_head}} helper', function () { re4 = new RegExp('"dateModified": "' + post.updated_at); should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -783,7 +783,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['featured']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -814,7 +814,7 @@ describe('{{ghost_head}} helper', function () { ).then(function (rendered) { should.exist(rendered); rendered.string.should.not.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -837,7 +837,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['page']}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -857,7 +857,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['index', 'paged'], pagination: {total: 4, page: 3, next: 4, prev: 2}}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -881,7 +881,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: ['index', 'paged'], pagination: {total: 3, page: 2, next: 3, prev: 1}}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -910,7 +910,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: []}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -948,7 +948,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: []}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.not.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -1042,7 +1042,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: []}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); @@ -1078,7 +1078,7 @@ describe('{{ghost_head}} helper', function () { {data: {root: {context: []}}} ).then(function (rendered) { should.exist(rendered); - rendered.string.should.match(//); + rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); rendered.string.should.match(//); diff --git a/core/test/utils/fixtures/images/myicon.ico b/core/test/utils/fixtures/images/myicon.ico new file mode 100644 index 0000000000000000000000000000000000000000..c24e3c410330d199710adb42c6a1f624ef87c013 GIT binary patch literal 15086 zcmeHOX>b%p6dnqz{G$cGEPv*QTK*FSLP$2*T;vWR2PqFw!4hH#B7$5cuyPfLMM)v2 zf+)om!WoXp8L^Nn;Yb2Xgak+|NjL=aPf~fluXlSg+00~SCp+0Jq}3P9v1?8 z-Vs*UACo4_CrGK(C2((M%IN%8o5Q- z?ux{_+8f8anI9j*XBTTkSKm3v=t^F!n!e55?MK=2xrrwrr1$9z}CB#LE_DQ|44P~e^k^?5NrHm6PY*@Br z5!F;zQElz-RC~QMUAtC8>({QLeJDso{J{*yqO7-=1 zWcGV-|2~a=r;uDj2MR(SWoAEMLx&S%DK>f_<>zKnb5oO<8Pw9!LLZKOSG6B`l$rg2 z4IR)G>^^``@<(J+)7`sfW(Zq97&A(>A9*YG0~I-IZ{{R!N{g5fm(%h~)MFQlb@gtA2oJYwwo_dnhN)EUVEv#=n?G25j* zXhTfY8|v7Q?`V7Kt&{vR_11SG$Cys}UE1(8q|*P{{z1uH;d-Zo+-xi z!ucKw2I0K`Ca|4q18`G$Za8uLD7Ch>Qd?VFPzjj8h7Kp%08Fd%zu7!#RPXHk&+9L* zEzM2l>z9#zX63;4u3yfqzv(FnbmrI7W@gx~KYQjhL5CCLN6ww2-X2NIzgj}8zga=6 zR(>560v0fVE%i8+Vjb#w{qlZuwy$T_o+YMJc0Vn+7@P1_46k1?QP#-C{Ce&8_9bHo zte#oFa=r`%+jXzWzU;-ODoM;0u^G9UR-MfF;njiP= z-L39zG5+wqfHhqE12%LxW54zNqt+&wKTx`_gZ=Y7Uv#qH>HQ=2QFrd#mL>(1mh9Y4 z@v%e1I{^RBnlV*;M`=NTE%$AD?|XO465T^-?dp}(*w8@D%}ru251$zlc$lu%RMRID z#;fB3<0Fs9$?;=HsENNBzynNRLq~V*G?Is2;KN5aC$LI5xAMmXR%)_aeD44Un80>o z?Ca^q8HaV!KO2OtUILarkH2o6XIp2WIyjwt+ygu^Mi~m-!O$t=tHHnu1%`+X_dq{8 z)wWG>-dKB>#_tB_hI{-JaqMu8U&}Z=V)FbRgysDKwGCl>e?zvxdw)j6nOc~Fy*2?8*wdg(YX{FUuW)%O)4xo? z*6WW49i`9-8+`4CZht*vayb4CLL3}-6%yjw_oK+840VzZGLW^S6EbK|YH}Ry-Ls2s)z?#f zU7eRW7GHPs2CZDNT*XEnW%XPq`5?1*Pl>>=vxkEW;yy;_LTC6)9Q#@j@_IkW+KnAS ze_l>HH8kAOgBI~>+}i*d#jYU>3_I;d9{_v7yg9VHq?mS1 z(u5*WwoAhY8OQ>|?ihd^&IJAK$1}<$t`#218)ypb?ogomwEXa$XU;4t-m%q7+c~vD zK7PpB^*?-^i}7dkTn>X4vcT}~KW!V0@{q~P$)Lu@2Ho**r4X{fu%jP#;oV-kcmW+c zu%8YFNRU~&c%i^BYNM5M?Kro>ujT2Eti%C^h^rvBVnA9w@U`UAmP@=q()idYai06* z4;#sdzTfyQ6^$FC&P8t(=F<9gt7$XK;4E044F{qZyl4l`E^pteHyvsxPaNZZju!3S zxM97@4_UkC&N#y?$jh>HuL`kW+{J^xw{6|5>POrh78FPj_F^ye2FS6FVMt~#|c6nWo3sC2p;fCz5Xfg1K@6aMrxwi zXJW63Jjzh#&o7^WnaTPOA1bBGmoCz!ixo_jKB^F9hYywt9zFeD`;+PDkuth=wMN`; z$Gv$$HC3Vvb%>4X$xAz={h4X7Ig!#*;@R$4UbB#z*w1Y&-M@z_E}WzD=gx9^Uab4r zFK_s69e=Ct6MoRsk237h;b+`^zjv>h9z3{Dznm%;cHpi>b@dg&qbCphF*eY@BlELq zWF8Zz*%`xx&*6XI>x`ccV-VQ*UXdhqA(eUQD^4Mccfj`hqj?8yy}zDr%uJB|4{t4J A_5c6? literal 0 HcmV?d00001