Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom favicons not working in 1.0 #8771

Closed
david-cako opened this issue Jul 28, 2017 · 19 comments
Closed

Custom favicons not working in 1.0 #8771

david-cako opened this issue Jul 28, 2017 · 19 comments

Comments

@david-cako
Copy link

david-cako commented Jul 28, 2017

Issue Summary

Custom favicons are no longer working.

I've looked in the admin panel to make sure the favicon isn't configured there, but I don't see anything of the sort, only a way to upload a much larger "Publication Icon".

Two separate issues:

  • {{ asset "/favicon.ico" }} does not correctly source the theme favicon anymore. I would expect this to return /assets/favicon.ico, but it instead returns /favicon.ico.

  • Further, {{ ghost_head }} is adding another line of the default favicon to the page.

I can't seem to override it, regardless of whether I put a hard-coded <link rel="shortcut icon" href="/assets/favicon.ico" type="image/x-icon"> before or after {{ ghost_head }}.

Steps to Reproduce

Try to use {{ asset "/favicon.ico" }} to serve a custom favicon from the theme assets.

Technical details:

  • Ghost Version: 1.0
  • Node Version: 6.9.5
  • Browser/OS: Chrome/OSX
  • Database: mysql
@acburdine
Copy link
Member

@david-cako the publication icon in the admin settings panel is what you're looking for - while the upload screen may look a bit large (all of the uploaders are the same), that's what you want to use to configure your publication icon.

Going to close this as this isn't a bug in Ghost and was completely intentional.

@david-cako
Copy link
Author

Just to make sure, I have to upload a 60x60px image for my favicon?

@jloh
Copy link
Member

jloh commented Jul 29, 2017

I'm currently running into this and I'm 99% sure uploading a publication icon still doesn't override it :/

@ErisDS
Copy link
Member

ErisDS commented Jul 29, 2017

As of Ghost 1.0, {{ghost_head}} automatically outputs the favicon code for you. Any code like {{ asset "/favicon.ico" }}should be removed from your theme.

I've added a section in the migration guide of the docs to cover this, and also opened an issue to add a check for this to GScan

P.S. Also note favicons are EXTREMELY heavily cached by browsers. If you don't see it update please check it on a different device first.

@david-cako
Copy link
Author

david-cako commented Jul 29, 2017

Okay, thank you both for clarifying.

I will say, it's entirely non-obvious based on the admin panel. There doesn't seem to be any indication there or in the documentation that the Publication Icon is what you need to upload to display a custom favicon.

@aphe
Copy link

aphe commented Sep 20, 2017

I use custom storage, upload the favicon from admin settings and success.

from the admin setting the url given for the favicon image is https://res.cloudinary.com/tendabiru/image/upload/q_auto:good/ywf2l3wagedtzlkvfdrr.png

but when i open my-site/favicon.png it returns 500 with message

Cannot read property 'then' of undefined

@kirrg001
Copy link
Contributor

@aphe Could you please share the whole error message+stack?

@aphe
Copy link

aphe commented Sep 20, 2017

here's what i got from pm2 logs (ghost install using ghost-cli)

ERROR [2017-09-20 09:45:19] "GET /favicon.png" 500 6ms

NAME: InternalServerError
MESSAGE: Cannot read property 'then' of undefined
level:normal

InternalServerError: Cannot read property 'then' of undefined
8|cizzu-be |     at new GhostError (/home/phe/cizzu-beta/ghost/versions/1.8.6/core/server/errors.js:9:26)
8|cizzu-be |     at prepareError (/home/phe/cizzu-beta/ghost/versions/1.8.6/core/server/middleware/error-handler.js:40:19)
8|cizzu-be |     at Layer.handle_error (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:71:5)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:315:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at Layer.handle_error (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:67:12)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:315:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at Layer.handle_error (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:67:12)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:315:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at Layer.handle_error (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:67:12)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:315:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at Layer.handle_error (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:67:12)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:315:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at Layer.handle_error (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:67:12)
8|cizzu-be | TypeError: Cannot read property 'then' of undefined
8|cizzu-be |     at serveFavicon (/home/phe/cizzu-beta/ghost/versions/1.8.6/core/server/middleware/serve-favicon.js:54:21)
8|cizzu-be |     at Layer.handle [as handle_request] (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:95:5)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:317:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at expressInit (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/middleware/init.js:40:5)
8|cizzu-be |     at Layer.handle [as handle_request] (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:95:5)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:317:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at query (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/middleware/query.js:44:5)
8|cizzu-be |     at Layer.handle [as handle_request] (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:95:5)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:317:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at Function.handle (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:174:3)
8|cizzu-be |     at EventEmitter.handle (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/application.js:174:10)
8|cizzu-be |     at mounted_app (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/application.js:230:10)
8|cizzu-be |     at Layer.handle [as handle_request] (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:95:5)
8|cizzu-be |     at trim_prefix (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:317:13)
8|cizzu-be |     at /home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:284:7
8|cizzu-be |     at Function.process_params (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:335:12)
8|cizzu-be |     at next (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/index.js:275:10)
8|cizzu-be |     at ghostLocals (/home/phe/cizzu-beta/ghost/versions/1.8.6/core/server/middleware/ghost-locals.js:15:5)
8|cizzu-be |     at Layer.handle [as handle_request] (/home/phe/cizzu-beta/ghost/versions/1.8.6/node_modules/express/lib/router/layer.js:95:5)

@kirrg001
Copy link
Contributor

@aphe This is because the custom storage adapter you are using has not implemented the read method, see. It's documented here.

@evenfrost
Copy link

evenfrost commented Sep 20, 2017

@kirrg001 I'm using custom storage (Cloudinary), favicon shows correctly in admin under 'Publication Icon' (src is https://res.cloudinary.com/...), but on site it renders as

<link rel="shortcut icon" href="/favicon.png" type="image/png">

So no favicon is shown. What could the problem be?

{{ghost_head}} is present.

@evenfrost
Copy link

Possibly related: #8885.

@aphe
Copy link

aphe commented Sep 22, 2017

@kirrg001 thank you for the info.

@kirrg001
Copy link
Contributor

@evenfrost You can always swing by our slack channel and ask in the #help channel, i had no time so far to investigate the underlying problem.

@kevinansfield
Copy link
Member

kevinansfield commented Sep 22, 2017

I'm using custom storage (Cloudinary), favicon shows correctly in admin under 'Publication Icon' (src is https://res.cloudinary.com/...), but on site it renders as

<link rel="shortcut icon" href="/favicon.png" type="image/png">

This is expected - /favicon.png is a special route that will load the saved image from storage engine when there's a custom icon otherwise render or redirect to the default icon.

As @kirrg001 mentioned above, the ghost-cloudinary-store storage engine has not implemented all of the required storage engine methods.

This is because the custom storage adapter you are using has not implemented the read method, see. It's documented here.

@mmornati
Copy link

I pushed the method implementation suggested by @aphe which is fixing the problem. Actually pushed on the mmornati repo. I will push on the seth one too (I think we will keep only one repo).

@evenfrost
Copy link

@mmornati can you confirm implementing read method fixed the issue for you (i.e. when added Publication icon via admin, it showed on blog side correctly)? As for me, it still returns default Ghost favicon . Busted cache, made hard refresh etc., /favicon.png route returns 200 status and empty response.

@aphe
Copy link

aphe commented Sep 25, 2017

@evenfrost have you save the changes (Save settings on the top right corner)?
you can verify this changes by going into the database and look at the settings table for column icon

@evenfrost
Copy link

Yes, I've saved them. Favicon shows correctly in admin, but in blog it's still the same. Managed to overwrite it for now with setting explicit link in theme's head.

@TwDrake
Copy link

TwDrake commented Mar 18, 2018

I'm currently not able to get my png that was uploaded to general settings' publication icon to show on my browser tab. I'm testing on a local build and have made sure that the default icon isn't cached. I'm on Ghost-CLI version: 1.5.2 and Ghost Version: 1.21.5. Any advice?

Edit: just saw the 'save settings' button and after using that everything is working as expected, I thought settings were saved as you edited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants