1772264 - Distinguish "rich" icons in the favicons service and when asking for a small sized icon prefer the classic ones

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[James]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:101.0) Gecko/20100101 Firefox/101.0

Steps to reproduce:

1. Loaded the following page: https://www.nhl.com/penguins
2. Notice in the 'tab', that the favicon is the Pittsburgh Penguins logo.
3. Now, attempt to bookmark the website.
4. Notice that you can see in the initial bookmark creation window, that it shows the Penguins logo.
5. Save the bookmark (doesn't matter where, but save it in the Bookmark toolbar for easy access).

Actual results:

Once the bookmark is saved, the 'favicon' is that of the NHL's main website.

Expected results:

Once the bookmark is saved, the 'favicon' should be the Pittsburgh Penguins logo, as you can see in the tab for the website.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Peter Magyari (Desktop QA)]

Reproducible on Firefox 101 through Firefox 103.0a1 on Windows 10 and MacOS 11 as well.

Thank you for reporting!

Comment 3

[Marco Bonardo [:mak]]

The site is doing something particular here.
The icon shortcut points to the Penguins icon, that is a 256x256 png
Then it also provides apple-touch-icons in 57x57, 72x72, 114x114, 144x144 sizes, but those are generic nhl icons, not specific.

The tab just ignores apple-touch-icon entries, but the favicon service doesn't, it wants all the plausible icon sources, to do a better job at upscaling/downscaling. After all the service may have to provide an icon for the New Tab page or other similar UIs.
When we ask to the favicon service a 32px icon, it will try to find one close to the requested size, and it will return the 57x57 apple-touch-icon.

Now, this could be fixed on the site itself by providing a multi-resolution ico file or an svg in "shortcut icon", that would better adapt to multiple resolutions.

On our side, probably a solution would be to store whether an icon is "rich" or "classic", and when asking for common sized icons (16 to 64px) give priority to the classic ones, only use rich ones as fallback.

Comment 9

[Ben]

This issue is reported many times.

The tab just ignores apple-touch-icon entries, but the favicon service doesn't

They should use the same icon, not do something different.

When we ask to the favicon service a 32px icon, it will try to find one close to the requested size, and it will return the 57x57 apple-touch-icon.

That is not a reasonable strategy. Suppose that a 36px icon is closest; resizing that one will probably give a lousy result. It would be better to use an icon that is a multiple of the requested one (64px, 128px) or an SVG icon.

Personally I would prefer the SVG always, if present, because it may contain a style sheet that adapts to dark mode, for instance. And, important, store the SVG and not a converted bitmap version, otherwise switching between light and dark mode still does not work.

Comment 10

[Sola]

(In reply to Ben from comment #9)

This issue is reported many times.

I'm not so sure if this bug will ever be fixed.

Again, the reason using apple-touch-icon as the first preference is a bad idea is that it's not even supposed to be used inside a browser. It's the icon when you drag a webpage to an Apple device's homepage. For example, when you drag mozilla.org to your iPhone homepage, it appears next to icons like "Settings" and "App Store". If you've ever used an Apple device, you would notice that such icons have a specific shape (they don't have corners). In fact, they also have specific colour requirements (Apple devices will autofill any apple-touch-icon transparency with black), that's why it's not supposed to be used anywhere else.

Comment 11

[Yazan Al Macki [:yazan]]

Attachment: Bug 1772264 - Part 1: Database schema change adding flags field to moz_icons table - r?mak

Comment 12

[Yazan Al Macki [:yazan]]

Attachment: Bug 1772264 - Part 2: Identifying a rich icon and passing down the info - r?mak

Comment 13

[Yazan Al Macki [:yazan]]

Attachment: Bug 1772264 - Part 3: Use rich info to filter icons returned - r?mak

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc492653034f
Part 1: Database schema change adding flags field to moz_icons table - r=mak
https://hg.mozilla.org/integration/autoland/rev/0a79fdeabfd0
Part 2: Identifying a rich icon and passing down the info - r=mak
https://hg.mozilla.org/integration/autoland/rev/bc9dcd2cf7c9
Part 3: Use rich info to filter icons returned - r=mak

Comment 15

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/fc492653034f
https://hg.mozilla.org/mozilla-central/rev/0a79fdeabfd0
https://hg.mozilla.org/mozilla-central/rev/bc9dcd2cf7c9