Open Bug 1453467 Opened 8 years ago Updated 2 years ago

Reader Mode can use different favicon from our choice for the webpage due to the favicon service returning apple-touch-icon but the tab never using that

Categories

(Toolkit :: Reader Mode, defect, P3)

Product:
Toolkit
Component:
Reader Mode
Platform:
Unspecified
All
Type:
defect

Tracking

()

Project Flags:
Webcompat Priority P3
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox-esr91 --- affected
firefox59 --- affected
firefox60 --- affected
firefox61 --- affected
firefox99 --- affected
firefox100 --- affected
firefox101 --- affected

People

(Reporter: cpeterson, Unassigned)

References

(
URL
)

Details

(Keywords: webcompat:needs-contact, Whiteboard: [needscontact])

Attachments

(2 files)

favicon-screenshot.jpg
829.55 KB, image/jpeg
Details
screenshot-2022-04-11.png
4.37 KB, image/png
Details
Attached image favicon-screenshot.jpg —
STR: 1. Load https://www.express.co.uk/life-style/science-technology/944551/Windows-10-Spring-Creators-Update-Release-Date-Delay-Stop 2. Click Firefox's "Toggle reader view" button (or press F9). ACTUAL RESULT: The favicon's white background color changes from white to dark gray. See the attached screenshot. This doesn't affect all favicon's with white backgrounds. This bug started in Firefox 59 after bug 1420714 was fixed. Before that, reader view pages displayed no favicon, so this gray background bug was probably already latent in the favicon code. EXPECTED RESULT: The favicon should look the same in web view and reader view. This is likely a separate bug, but when I was testing other page's with white favicons, I noticed that Wikipedia shows a different (smaller W) favicon in reader view: https://en.wikipedia.org/wiki/Firefox
The express's apple-touch-icon favicon and its "normal" favicon have a different background. We show the 'normal' favicon for the normal webpage, and the other one for reader mode. Reader mode uses the one that we get from the favicon service, because the link is in the <head> and that's not included in the stuff we render in about:reader. We manually get the favicon out of the favicon service and then append it into the head once we have it. It looks like the apple icon is also the one we use for history results. So either this is a bug in the places favicon service for returning those icons, or alternatively we can chalk it up as a bug in the browser code for not using the apple-touch-icon favicon for the tab. I expect there are other bugs on file for this discrepancy. Maybe. On the other hand, perhaps there's an option we can/should be passing to the favicon service to avoid getting the hidpi icon or something. Marco, what do you think?
Component: Reader Mode → Places
Flags: needinfo?(mak77)
Summary: Favicon's white background turns dark gray in reader view → Reader Mode can use different favicon from our choice for the webpage due to the favicon service returning apple-touch-icon but the tab never using that
(In reply to :Gijs (he/him) from comment #1) > It looks like the apple icon is also the one we use for history results. So > either this is a bug in the places favicon service for returning those > icons, or alternatively we can chalk it up as a bug in the browser code for > not using the apple-touch-icon favicon for the tab. I expect there are other > bugs on file for this discrepancy. Maybe. The tab doesn't use rich favicons for a choice, because it's unlikely it will ever need anything more than a 16px icon and we can't use the favicons service for it (because it wouldn't work in PB mode). Other UI consumers instead may often need larger icons, thus the favicons service doesn't make a distinction and just passed out the best sized icon for the requested size. The service doesn't have a way to distinguish "rich" icons from normal ones, and if the site has bad icons, it can't do much. > On the other hand, perhaps there's an option we can/should be passing to the > favicon service to avoid getting the hidpi icon or something. Marco, what do > you think? The page only defines 2 icons, the basic favicon.ico is not even an ICO file, it's a png, 15x16, not even square. Looks like they didn't put much attention into that. We end up using the apple-touch-icon because on hi-dpi screens we need an icon bigger than 16px to avoid showing a blurry icon, and that's the only one provided, it's a 57x57. To sum up, I think we are doing our best here, the site should provide better icons. => Tech Evangelism.
Component: Places → Desktop
Flags: needinfo?(mak77)
Product: Toolkit → Tech Evangelism
Regarding wikipedia, there is indeed a bug in reader mode, where it requires a favicon through promiseFaviconLinkUrl but without specifying a preferred size. This can be fixed and I will file it apart.
I filed bug 1453580 and cc-ed you there.
(In reply to Marco Bonardo [::mak] from comment #3) > Regarding wikipedia, there is indeed a bug in reader mode, where it requires > a favicon through promiseFaviconLinkUrl but without specifying a preferred > size. This can be fixed and I will file it apart. So the express.co.uk and Wikipedia icons are the same issue? Reader mode doesn't specify a preferred icon size and thus selects the apple-touch-icon (which has a gray background for express.co.uk and a different W size for Wikipedia)? If bug 1453580 will fix the icon inconsistency, then this Tech Evangelism bug is just about express.co.uk using a gray background for their apple-touch-icon. That was likely a conscious design decision (because they are assuming iOS devices) and we can resolve this bug as INVALID.
(In reply to Chris Peterson [:cpeterson] from comment #5) > So the express.co.uk and Wikipedia icons are the same issue? No, I filed them differently because it's different issues. The express just defined bad favicons, it's a tech evangelism problem, they should provide better favicons. Wikipedia is bug 1453580. > If bug 1453580 will fix the icon inconsistency, then this Tech Evangelism > bug is just about express.co.uk using a gray background for their > apple-touch-icon. That was likely a conscious design decision (because they > are assuming iOS devices) and we can resolve this bug as INVALID. The fact is that as a basic favicon they only provide a 16x16 favicon, that is sub-par on today's hi-dpi screens, they should at least also provide a 32x32, or better an ico file with multiple sizes, or an svg. Yes, likely the grey background on the apple-touch icon is by design.
Priority: -- → P3
Whiteboard: [needscontact]
Product: Tech Evangelism → Web Compatibility

See bug 1547409. Moving webcompat whiteboard tags to keywords.

Keywords: webcompat:needs-contact

The issue is no longer reproducible on my side. Toggling the reader mode on and off multiple times does not change the background of the favicon icon:

https://prnt.sc/btj2YCKA3yvw

Refreshing the page returns the same result.

Chris, can you please confirm that the issue has been fixed on your side as well?

Tested with:

Browser / Version: Firefox Nightly 101.0a1 (2022-04-10) (64-bit)
Operating System: Windows 10 PRO x64

Flags: needinfo?(cpeterson)

I can still reproduce this bug in a clean profile in Firefox 99 and Nightly 101 on Windows 11. I tried switching to different dark and light Firefox themes and switching Windows between light and dark mode. I see the same bug in all cases.

Flags: needinfo?(cpeterson)

I can reproduce on both Windows 11 and macOS 11.6.5 (Big Sur).

This bug is a very low priority since it only affects reader mode and only for some sites.

status-firefox100: --- → affected
status-firefox101: --- → affected
status-firefox99: --- → affected
status-firefox-esr91: --- → affected
OS: Unspecified → All

Chris, this seems to reproduce differently on my side, as requesting the reader mode feature fails to show the favicon. Returning to the normal view mode of the page, the favicon is displayed as expected. Requesting the reader mode again, the favicon can not be seen. Chrome does not display this behavior, rather the favicon gets shrunk when requesting reader mode. This has been observed on both Windows 10 and Windows 11 devices, with dark theme enabled, disabled, or when toggling between the themes. When the favicon reappears (on my side, only when in normal mode), no issues are encountered regarding the change of background color in the favicon.

Karl, any chance of reproducing the issue? And do reader mode issues fall under our competency in Webcompat product?

Tested with:

Browser / Version: Firefox Release 99.0.1 (64-bit)/ Firefox Nightly 101.0a1 (2022-04-12) (64-bit)
Operating System: Windows 10 PRO x64, Windows 11

Flags: needinfo?(kdubost)

While different it has similarities with Bug 1647173
And it's definitely not a webcompat issue.

I'm moving it to Toolkit: Reader Mode.
And let them decide about the Priority as Chris mentioned it seems low priority.

Webcompat Priority: --- → P3
Component: Desktop → Reader Mode
Flags: needinfo?(kdubost)
Product: Web Compatibility → Toolkit
See Also: → 1647173
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: