potential contributions

[gurgeous]

Hey Mat. I love besticon, thank you for creating such a great project and maintaining it for so long! I was thinking of making some enhancements and I wanted to get your thoughts. I'm an experienced engineer, though relatively new to Go so please be gentle 😄

I've gone deep on favicons this week. Real deep. Too deep, maybe. I dug into Chromium's favicon code and examined the sqlite db under the hood. I studied the favicon links for the Tranco top 1000 websites. Here are some ideas to consider:

1. Add support for more formats. See below for the formats from the top 1000.

2. Make the parser a bit more flexible. See below for the rel attributes from the top 1000.

3. Add support for the Chromium favicon finder, or an approximation (see below). This could be done as an ENV variable or a query param. The nice thing about this approach is that it would result in icons that are almost always in agreement with Chrome. We could even make it the default if you ask for a small icon. Maybe it should be a bug (or known issue) if besticon and Chrome disagree. Probably the biggest difference today is that besticon searches the apple touch icons by default.

4. Richer test suite. Maybe we can include some of the top 1000 in the suite? I like your VCR infrastructure. I also have an unreleased httpdisk package that I'm working on, if we wanted to move this out of besticon. Suggestions welcome, of course.

5. Image normalization. Wouldn't it be nice if besticon could always return a normalized png of the requested size? At the very least we could add a script to make this easier. My team uses some Ruby code to drive imagemagick, librsvg, zopfli, etc.

What do you think? Sorry for the wall of text 😄

Appendix: Chromium Favicon Finder

Chromium uses the following algorithm for finding a favicon:

1. Parse the html and build a list of icons. If there are no icons, use /favicon.ico.
2. Each icon is scored based on the sizes attribute to determine if the icon is close to 16x16 or 32x32. See select_favicon_frames.cc. Icons without a sizes attribute are optimistically scored 1.0.
3. Fetch icons orderd by score. Ties are broken bottom to top, per the [rel icon spec].
4. For each fetched icon, recalculate score based on actual image sizes.
5. Keep going as long as scores are increasing.

Chromium also has some bugs - it doesn't support sizes="any" properly, for example, and ironically treats it like a 0x0 icon.

Appendix: From the Top 1000

icon extension	count
.png	2210
.ico	635
.svg	21
.jpg	13
.gif	1

link rel="xxx"	count
apple-touch-icon	1166
icon	836
shortcut icon	498
apple-touch-icon-precomposed	376
alternate icon	7
SHORTCUT ICON	7
Shortcut Icon	3
Shortcut icon	2
shortcut icon mask-icon	1
shortcut alternate icon	1
favorite icon	1

sizes="xxx"	count
any	5
1x1	1
16x16	154
16x16 24x24 32x32 48x48	2
16x16 32x32	3
16x16 32x32 48x48	1
16x16 32x32 48x48 64x64	1
16x16 64x64	1
24x24	1
32x32	192
36x36	4
48x48	23
57x57	100
60x60	68
64x64	8
72x72	148
76x76	125
96x96	79
100x100	2
114x114	142
114×114	1
120x120	141
128x128	24
144x144	146
144×144	1
152x152	158
152x512	1
160x160	6
167x167	23
180x180	193
192x192	126
194x194	12
195x195	1
196x196	22
228x228	5
256x256	11
384x384	6
512x512	15
1024x1024	3

[mat]

Hey @gurgeous , thank you for the extensive research and the good ideas!

My thoughts:

1. Add support for more formats. Not sure. I once removed support for e.g. .gif IIRC because it caused some problems and I decided to follow the KISS rule and simply removed this and went on with life. .svg would be great to have but hard, haven't look for a long time how feasible this is.
2. Make the parser a bit more flexible. Definitely, this could be an easy win with regard to supporting more pages where besticon still fails to deliver the correct icon! The parsing lib I'm using used to not support case insensitive queries, haven't checked if this is still the case.
3. Add support for the Chromium favicon finder, or an approximation (see below). Haven't look in detail into this, while this sounds like a lot of work, it could (or could not) be worth it. Like you said, adding it as an 'additional retrieval strategy' controlled by a query param and/or ENV would be the way to go with this
4. Richer test suite. Maybe we can include some of the top 1000 in the suite? Great idea, can't see a reason not to do this 👍:)
5. Image normalization: While I agree image normalization is important, I don't think it belongs into besticon. (Separation of concerns, there are many CDNs out there which offer exactly this already and can be used for this.)

If you want to tackle any of it, I'd say: give it a go, would be more than happy to accept contributions on these 👍

[gurgeous]

Excellent, glad to hear it! I'll try to explore these one at a time, maybe starting with (2). Stay tuned...

[mat]

I lean towards 1), cause it seems like the most KISS approach and I don't really like that with 2) we would have different fonts for the non A-Z cases, which is not a nice UX.

[gurgeous]

OK, sounds good. I'll start with svg Icon support and see how that goes. Then we can do lettericons after.

[gurgeous]

SVG is mostly working in my branch. Question - how should we handle SVG when picking the best icon for a SizeRange? A few options come to mind:

1. If we have a perfect raster match, use that. Otherwise fallback to SVG. Seems like this would always result in high quality icons.
2. Prefer SVG, always. Many sites seem to use the SVG to generate the other sizes. Not all, though.
3. Treat the SVG as though it has a large width & height. Maybe 100px so that SVG acts primarily as a fallback for the larger sizes.

Not sure there is a right answer, but I'm happy to let you decide :) Chromium is still wrestling with this issue too - https://bugs.chromium.org/p/chromium/issues/detail?id=1162276

[mat]

Nice, that's great news!

Wow, not sure. I hoped for a perfect world were we can assume the SVG is always the best option. In what way where the SVGs different to the other sized raster version on the examples you saw? I'm leaning towards 2.)

Thinking about 1.) I'm wondering whether it would be worthwhile to introduce an explicit optional fallbackFormat=[svg|png]. Previously, with only raster formats you'd always get a PNG back. But now, do we want to support the case formats=svg where a user would always get an SVG back. (I don't want to overcomplicate things, just not sure what's best.)

Overall 2) seems the most straight forward to me in a way. What's your preference?

[gurgeous]

I added an =IMAGE(xx) column to my top 1000 spreadsheet, which worked beautifully. See below. In each case I've moved the svg to the top. What can we conclude? Usually the svg is the same as the other icons, but not always. None of the svgs are ugly, though. How about this:

1. Prefer SVG, always. Simple and satisfies most use cases, especially larger sizes.
2. Add a new formats flag [gif|ico|jpg|png|svg]. I haven't read the code closely, but it seems like we almost have this already via defaultFormats and FormatsAllowed. Remove svg from formats to revert to png lettericons, if we want to keep supporting that use case.
3. When (if) we decide to mimic Chrome, that's a good opportunity to favor perfect raster matches over SVG.

[gurgeous]

So @mat, what should we work on next? Chrome compat? Cleanup? Richer test suite / top 1000? I can do more, I think :)

[mat]

Sorry, quite a busy week... Would start with Test suite top 1000, Chrome compatibility as additional 'fetch strategy' would be awesome to try out and have to think about which cleanups I support (nostalgia kicking in;-)).

[gurgeous]

No worries. Rule 1 of being an open source maintainer, no need to ever apologize or feel guilty! We all benefit from you volunteerism over the years :)

I'll start poking around with adding more domains to the test suite. Should I stick with VCR? Seems to work great. I'll probably start by adding TestXXXX methods for more of the top1000 to besticon_test.