1845035 - Invalid blank favicon causes the page to inherit favicon from a previously visited page

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

[Rimas Kudelis [use needinfo to ping]]

Attachment: favicon_demo.html

+++ This bug was initially created as a clone of Bug #577447 +++

When a newly visited page specifies data:image/x-icon with no content as its favicon, it inherits the favicon from the previously visited page.

I've noticed this a while ago with https://savitarna.perlasgo.lt/, but I've just reported them that their favicon is missing, so I hope they might fix this soon, so I'm attaching a really minimal test case which illustrates the problem.

If you visit a website that has a favicon (e.g. https://www.google.com), and then open the attached file in that same tab, you'll see that the tab keeps the favicon that was previously shown.

Comment 1

[Rimas Kudelis [use needinfo to ping]]

Attachment: Screenshot of the page in question

Comment 2

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

The severity field is not set for this bug.
:dao, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Screenshot of tab DOM

This is a screenshot of the Firefox tab's DOM at the time that the bug is reproduced. The src on the image looks pretty fishy - it's data:image/x-icon, which isn't a valid Data URI, and yet the old image is still displaying.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

mak said he wanted to take a look at this.

Comment 5

[Marco Bonardo [:mak]]

Sorry for the delay. I was worried that this could be one of the reasons for which sometimes we store wrong favicons, but it's apparently only a visual problem.
The url here is parsable, so an nsIURI can be built out of it. That means for us it's valid enough (we don't want to preload the image to check if it's effectively valid, and introducing sanitization code to detect strangely built icons sounds like excessive complexity).
So we end up setting this url into mIconURL, but loading it does nothing and the previous favicon remains.

The easiest solution is to clear the image attribute before loading the new icon, then we'll properly clear the previously shown image. The small downside is that after the "pending" animation we'll briefly show empty space before the new icon appears, but I don't find it jarring or problematic as the icon is about to change anyway. By doing so we ensure only the newly loaded icon can be shown.

I'll try to see how complex is to write a test that visually checks the icon

Comment 6

[Marco Bonardo [:mak]]

Attachment: Bug 1845035 - Invalid favicon causes the page to inherit favicon from the previously visited page. r=dao

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f53853d2c59b
Invalid favicon causes the page to inherit favicon from the previously visited page. r=dao,tabbrowser-reviewers

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f53853d2c59b