Skip to content
This repository has been archived by the owner on Nov 27, 2020. It is now read-only.

Add a HiDPI favicon. #696

Merged
merged 1 commit into from Sep 22, 2014
Merged

Add a HiDPI favicon. #696

merged 1 commit into from Sep 22, 2014

Conversation

whatthejeff
Copy link
Contributor

Difference on my retina display:

buvjv7biuaa5wwo

@stof
Copy link
Member

stof commented Aug 18, 2014

Given the symfony-standard repo is meant to be a bootstrap for your own projects, you are expected to change the favicon instead of keeping the Symfony one. So IMO, including the HiDPI version in the SE does not make much sense

@gnugat
Copy link

gnugat commented Aug 18, 2014

@stof it doesn't, but now that the PR is here... ;)

@docteurklein
Copy link

there shouldn't be a favicon.ico (neither apple-touch-icon) in the first place.

either remove them, or keep them.
If second option, then better have a hi dpi version :)

@weaverryan
Copy link
Member

+1 for @docteurklein's comments. I don't feel too strongly in either direction. Icon looks great though :)

@gnugat
Copy link

gnugat commented Aug 18, 2014

Web application should contain favicons. The only problem I see with the current one is that if developers forget to change it (and they will), then in production it will be easy to know that's it's based on Symfony2.

@whatthejeff
Copy link
Contributor Author

either remove them, or keep them.
If second option, then better have a hi dpi version :)

👍

I don't really have a strong opinion regarding the inclusion of a favicon. I only created this ticket after someone requested it on Twitter. I do think it's nice to have the HiDPI version if you are going to include a favicon, though.

@peterrehm
Copy link
Contributor

If we assume the developers are unable to change the favicon and we see this as issue then the favicon might need to be removed. But on the other hand if we do not provide an favicon and the developer forgets to add one then he gets a second request (for the failed url) against the application which is more worse than exposing that the application is using symfony.

What do you think?

@gnugat
Copy link

gnugat commented Aug 18, 2014

The best solution would be to provide a blank favicon (I don't know if it should be HiDPI though :) )

@peterrehm
Copy link
Contributor

@gnugat I thought about this as well but it might be then possible as well to determine if you used symfony based on the file information. If we fear just that.

@fabpot
Copy link
Member

fabpot commented Sep 22, 2014

Thank you @whatthejeff.

@fabpot fabpot merged commit c81a658 into symfony:2.3 Sep 22, 2014
fabpot added a commit that referenced this pull request Sep 22, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

Add a HiDPI favicon.

Difference on my retina display:

![buvjv7biuaa5wwo](https://cloud.githubusercontent.com/assets/306525/3951439/8058ac42-26d6-11e4-89f4-3d69904f7b4c.png)

Commits
-------

c81a658 Add a HiDPI favicon.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
7 participants