Closed (fixed)
Project:
Drupal core
Version:
9.2.x-dev
Component:
theme system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
21 Oct 2020 at 11:16 UTC
Updated:
29 Dec 2020 at 10:39 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
saschaeggiComment #3
gábor hojtsyWould be great to see screenshots of how it looked before / after :)
Comment #4
gábor hojtsyAlso parenting to #3134326: Adopt the 2020 (evergreen) Drupal logo in Drupal core.
Comment #5
gábor hojtsyPer our current policy the software logo should be the favicon, see table https://www.drupal.org/about/media-kit/logo-wordmark-guidelines, so this issue makes total sense to me.
Comment #6
saschaeggiComment #7
saschaeggiAdded screenshots
Comment #8
shaalThe existing favicon.ico file size is 5k.
After applying this patch the file size is 300k.
Comment #9
saschaeggi@shaal You were right. I was using the one from drupal.org but it seems way to heavy.
So I now created one with the sizes 16px, 32px, 48px & 256px which is ~ 25kb.
Additional I also created another one with only the 16px & 32px variants (8kb).
Comment #10
shaalI tested and reviewed both patches. The file size is good, I'm RTBCing.
I think we should open a new issue to create additional "favicon" icons to cover all options and higher resolutions.
Keeping the 32x32 favicon.ico from #9 patch as-is to support older browsers + creating additional files to support new browsers and higher resolutions.
Additional context:
https://css-tricks.com/favicon-quiz/
https://css-tricks.com/svg-favicons-and-all-the-fun-things-we-can-do-wit...
https://realfavicongenerator.net/faq
Comment #11
gábor hojtsyWhich resolutions are present in the current favicon?
Comment #12
lauriiiMoving to needs review until #11 has been documented.
Comment #13
saschaeggiThe current favicon contains the 16x16 & 32x32 sizes.

So the
3178135-9_32px-max.patchfrom #9 is the right one.Comment #14
lauriiiWe still need follow-up for #10.
Comment #15
saschaeggiFollow-up is created: #3179839: Consider using more favicon sizes
Comment #16
lauriiiBartik is using Druplicon as the default logo. Should Bartik continue to use Druplicon as the favicon or should we change the logo Bartik is using?
Comment #17
saschaeggi@lauriii for the same out of the box experience I'd vote to change it in Bartik, too.
Comment #18
cilefen commentedComment #19
saschaeggiI discussed this with @lauriii & @gábor-hojtsy
We better leave Bartik out, as the Druplicon is also the Logo of the Bartik theme.
So RTBCing this again.
Comment #20
lauriiiJust discussed this with @Gábor Hojtsy. We agreed that we should not change the favicon in Bartik from Druplicon because Bartik is using Druplicon also as the site logo. The current patch changes Druplicon to the new logo so we should make some changes to the patch.
Comment #21
saschaeggiAlright updated the patch to include the Druplicon favicon for Bartik.
Comment #22
saschaeggiUpdated patch to include the favicon with only 16x16 & 32x32
Comment #23
lauriiiThis addresses #20. Moving back to RTBC
Comment #24
gábor hojtsyLooks good to me too :)
Comment #25
saschaeggiCan we get this committed?
Comment #27
gábor hojtsyCommitted to 9.2.x. I consulted the committers about whether this should be backported. Leaving RTBC for 9.1.x in the meantime.
Comment #28
gábor hojtsyDiscussed with @lauri, will keep in 9.2.x :)