1844150 - Firefox suggest uses generic favicon with wrong color-scheme.

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: image.png

See screenshot.

The url of the <img> element seems wrong, has a bunch of nested protocols:

moz-anno:favicon:moz-anno:favicon:moz-anno:favicon:moz-anno:favicon:https://github.com/fluidicon.png"

If I leave this as moz-anno:favicon:https://github.com/fluidicon.png then it uses the right image.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I can't repro this on a clean profile, maybe it comes from here? Marco, do you know what might be going on?

Comment 2

[Marco Bonardo [:mak]]

If tab.icon is already a moz-anno:favicon, GetFaviconLinkForIcon should not really add another prefix, same for getIconForUrl... That's a simple fix and I will just do that.

I think it is being added multiple times because remote tabs are cached, and every time we go through the list we add a moz-anno:favicon to tab.icon... that doesn't make sense.

Thank you for finding this.

Comment 3

[Marco Bonardo [:mak]]

https://searchfox.org/mozilla-central/rev/ea550f0d1afaf7747ebd988325ea10cf4e3e4de4/browser/components/urlbar/UrlbarProviderRemoteTabs.sys.mjs#131,138,146,150-153

Comment 4

[Marco Bonardo [:mak]]

Attachment: Bug 1844150 - Don't add moz-anno:favicon to local protocols. r=daisuke

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46add7a02063
Don't add moz-anno:favicon to local protocols. r=daisuke,sessionstore-reviewers

Comment 6

[Cristina Horotan [:chorotan]]

Backed out changeset 46add7a02063 (bug 1844150) for causing bc failures at browser_readerMode.js

Backout: https://hg.mozilla.org/integration/autoland/rev/2d245598e3d515a6d740af0b7df954aa1e6e6a50

Failure push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=46add7a02063af4d55dca797af32f8b376deb293

Failure log: https://treeherder.mozilla.org/logviewer?job_id=423795429&repo=autoland&lineNumber=6642

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/094d4087bf81
Don't add moz-anno:favicon to local protocols. r=daisuke,sessionstore-reviewers

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/094d4087bf81

Comment 9

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I tried to reproduce this issue but I was unsuccessful. Would it be possible to have reproduction steps in order to properly verify this issue?
Thank you.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

I couldn't repro on a clean profile, so unfortunately no concrete STR. Daisuke maybe you know?

Comment 11

[Marco Bonardo [:mak]]

I suspect you need a tab synced from another desktop device

Comment 12

[Daisuke Akatsuka (:daisuke)]

Attachment: Screenshot 2023-08-14 at 12.42.31.png

Hello!
Yes, as Marco said, I could reproduce it with remote tabs in my env.

STRs:

1. Launch Firefox Release (116.0.2).
2. Login to Firefox account in Release.
3. Input git on the urlbar in Release, and confirm that there are no suggestions for any remote tabs.
4. Launch Firefox Nightly (118.0a1).
5. Login to Firefox account in Nightly, and confirm that the opening tabs will be shared in Sync Settings.
6. Open https://github.com in Nightly.
7. Sync Firefox account in Nightly (Click Sync now).
8. Sync Firefox account in Release (Click Sync now).
9. Input git on the urlbar in Release, and confirm there is the suggestion for github.com that is opening in Nightly.

Then, confirm the url of the icon of the suggestion using Browser Toolbox in Release.
(To confirm will be easier if set ui.popup.disable_autohide to keep the urlbar result)
I attached the screenshot. (Sorry, Japanese!)

Comment 13

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Attachment: repro beta.png

Thank you so much for taking your time to provide such detailed STR, Daisuke!
However, using these STR I can still reproduce the wrong color scheme icon. Affecting both Nightly 118.0a1 and Firefox 117.0b8 on Windows 10, macOS 12, Ubuntu 22. Please refer to the attached screenshot for more details.

Comment 14

[Daisuke Akatsuka (:daisuke)]

Thank you very much, Ardelean!
Yeah, I had checked only whether the protocol was correct or not. And indeed, the fallback icon color was wrong.
I investigated this one, and it seems the reason was that the -moz-context-properties: fill; is not applied to the image whose protocol starts with moz-anno.
https://searchfox.org/mozilla-central/rev/98d0035da36d786b7ca9191b8a23de9c2c943465/layout/svg/SVGContextPaint.cpp#65-66

I reopen this bug, and I will fix the issue by applying moz-anno protocol as well.

Comment 15

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1844150: Use context properties in SVG of moz-anno protocol

Comment 16

[Marco Bonardo [:mak]]

please don't reopen this. we fixed an actual bug here where the icon url was completely broken, if there's leftover issues to handle that should happen in a new bug.

Comment 17

[Phabricator Automation]

Comment on attachment 9349845 [details]
Bug 1844150: Use context properties in SVG of moz-anno protocol

Revision D186644 was moved to bug 1849791. Setting attachment 9349845 [details] to obsolete.