1502547 - Removing the favicon via JavaScript no longer works

Status: RESOLVED WONTFIX

(Firefox :: Tabbed Browser, defect, P5)

Description

[12Me21.MC]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

Create a page using this html:
<link rel="icon" type="image/x-icon" sizes="16x16" href="https://bugzilla.mozilla.org/favicon.ico">
Using Javascript or the inspector, remove the <link> (or change its href to "").



Actual results:

Nothing
(Changing the href to a valid url will update the favicon correctly)


Expected results:

The favicon disappears

Comment 1

[IdmFoundInHim]

I had never tried doing this before but reproduced it on Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

So, I ran a regression test:
> 2018-10-26T21:21:56: DEBUG : Using url: https://hg.mozilla.org/mozilla-central/json-pushes?changeset=28ad9a9e95d518e1163e550ae19c972aabb44df5&full=1
> 2018-10-26T21:21:56: DEBUG : Found commit message:
> Backed out changeset 64f3290fac6e (bug 1471532) on reqest by decoder a=backout

It appears changeset 28ad9a9e95d518e1163e550ae19c972aabb44df5 caused the regression.

Reporter 12Me21.MC,
I recommend changing:
1. Version to "Trunk"
2. Summary to "Deleting link node to favicon in devtools does not make favicon disappear"
3. Has Regression Range to "yes"

Comment 2

[Gingerbread Man]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=52baefdfbeca055fccb7518db3fd51ceea0649dc&tochange=40ed437da7ae4407ece2ad47cd8fe3c7943f9eb9

Comment 3

[Dave Townsend [:mossop]]

How were you testing to find the regression range, removing the link node or setting its href to ""? The two are quite different operations. Removing the link node seems to have been broken well before bug 1453751 landed.

Comment 4

[12Me21.MC]

In the past, I don't think changing the href or removing the link node would ever update the favicon (you would have to remove the old <link> and insert a new one) But now, changing the href DOES update the favicon (as long as the file exists; otherwise it just stays the same) but it doesn't appear to be possible to remove the favicon no matter what you do. (before, you could insert a new <link> with the href set to "")

Comment 5

[Dave Townsend [:mossop]]

(In reply to 12Me21.MC from comment #4)
> In the past, I don't think changing the href or removing the link node would
> ever update the favicon (you would have to remove the old <link> and insert
> a new one) But now, changing the href DOES update the favicon (as long as
> the file exists; otherwise it just stays the same) but it doesn't appear to
> be possible to remove the favicon no matter what you do. (before, you could
> insert a new <link> with the href set to "")

I'm a little confused. You filed this report saying that removing the favicon no longer works but now you say it never worked? Which is it?

Comment 6

[Dave Townsend [:mossop]]

In testing it looks like Chrome behaves the same as current Firefox. Neither removing the link tag or changing its href to "" clears the icon from the tab so any change we make now would cause us to behave differently to Chrome.

Comment 7

[Gingerbread Man]

Attachment: remove_favicon.html

(In reply to Dave Townsend [:mossop] (he/him) from comment #3)
> How were you testing to find the regression range, removing the link node or
> setting its href to ""? The two are quite different operations. Removing the
> link node seems to have been broken well before bug 1453751 landed.

Prior to bug bug 1453751, setting href to "" updated to no favicon. Removing the link element didn't update the favicon even before that. Here's a testcase.

Comment 8

[Dave Townsend [:mossop]]

So really this is two different bugs. But since we're behaving the same as Chrome now I'm inclined to just say we should leave this alone now., What do you think Marco?

Comment 9

[IdmFoundInHim]

Yes, I also tested by setting href to "" rather than deleting the node.
Personally, I feel that this would be worth fixing. It just seems intuitive and convenient to be able to unlink the favicon. Someone may potentially want to use js to remove the favicon in certain cases (low data mode, etc.), and this bug would prevent that.

Comment 10

[Marco Bonardo [:mak]]

Regardless of what the tab does, the favicon service never provided a way to remove a favicon. Also expiring a favicon doesn't mean it gets removed, just that the next load will overwrite whatever favicon was present (indeed we never remove favicons).

How does this matter? Basically if we make the tab update to no favicon, we end up with an inconsistency in the UI, where some parts show an empty icon and others don't.
That means if we fix this, we likely must change the backend to cope with the change, that makes the cost of this change non-trivial.

Comparing that cost to the benefit doesn't seem to give a positive outcome, there aren't many use-cases to allow the tab to remove the favicon.
And if the current behavior is also coherent with other browsers (as comment 6 states), by fixing this we'd also create a compatibility debt.

So, unless you have a strong use-case for this, I'd vote for a wontfix.

Comment 11

[12Me21.MC]

It has always been possible (and intended) for a tab to have no favicon.
The alternative (used by Chrome) is to have a "default" favicon for pages that don't supply their own, but this is very ugly and a waste of space.
A generic default favicon doesn't give you any useful information, and takes up space that could be used to show more of the page's <title>.

Until recently, it was possible to remove the favicon using JavaScript, but now it's not.

I use a script on a certain chat website that updates the tab's <title> and favicon to show new messages.
(The favicon is set to the user's profile picture, and the title is set to the contents of the message.)
For messages that aren't associated with a user, it removes the favicon (to give more space for text).

Comment 12

[Dave Townsend [:mossop]]

I actually found the spec that details how to load favicons for sites. Surprise surprise it doesn't specify what exactly to do when a favicon link is changed or removed. It merely encourages browsers "to update icons when the list of icons changes". So by doing nothing we're not breaking spec. And we're matching the behaviour of other browsers. Given that I don't think it is worth making a change here.

Comment 13

[12Me21.MC]

Even if it's not *required* by the spec, I think the fact that it was possible in previous versions makes it worth fixing.
It's not OK to remove a useful feature like this.

And yes, no other browser allows you to dynamically remove a tab's favicon, but that is (...was) one of the reasons I use Firefox.