1083895 - HTMLLinkElement not in DOM is used for favicon

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Peter Schlömer]

Attachment: Self-contained test case

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141015181347

Steps to reproduce:

I have an application where I change the favicon of the current page using Javascript. To do so, I create a HTMLLinkElement object (using document.createElement) when the page is loaded, and replace the existing HTMLLinkElement object defining the favicon with it later, when I want to change it.

This works correctly in Firefox 31, but behaves unexpectedly in 33 (I have not tested whether it works in 32), both on Windows 7 and Linux (Gentoo). It also works correctly if I try the same with a CSS stylesheet.

I have reduced this to a simple test case, see the attached HTML file.


Actual results:

As soon as all required attributes of the newly created HTMLLinkElement object - which is not in the DOM at this time - are set, the favicon changes.


Expected results:

The favicon should have remained unchanged until the new HTMLLinkElement is in the DOM.

Comment 1

[Loic]

Not sure if it's a valid bug, anyway:

Regression range:
good=2014-07-11
bad=2014-07-12
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e1a037c085d1&tochange=b0701d069bf9

suspected bug:
Gijs Kruitbosch — Bug 577892 - allow icons to change when href attribute is set directly, with automated test, r=dolske,bz

Comment 2

[:Gijs (he/him)]

I expect this is an actual regression, yes. Redirecting to bz to be sure.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah.  We should probably not fire DOMLinkChanged for <link>s that are not in the DOM.

Comment 4

[:Gijs (he/him)]

Attachment: favicon should not change if link element isn't in DOM,

Like this? Note that I've not tried to actually think hard about shadow DOM or any other edge cases... I don't think there are any? But then, the name 'IsInUncomposedDoc' is not very clear and so I'm not sure if this should change the result here...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8506345 [details] [diff] [review]
favicon should not change if link element isn't in DOM,

I think this is what you want, yes.  IsInUncomposedDoc() means "in document, not in shadow DOM".  Naming is hard; we spent about a week trying to come up with good names for this stuff.  :(

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/719a055fe22b

Comment 7

[:Gijs (he/him)]

Comment on attachment 8506345 [details] [diff] [review]
favicon should not change if link element isn't in DOM,

Approval Request Comment
[Feature/regressing bug #]: bug 577892
[User impact if declined]: merely creating a new link element and setting its href will cause issues
[Describe test coverage new/current, TBPL]: has automated test
[Risks and why]: pretty low, specific fix to check if the new link element is in the document when the href is set
[String/UUID change made/needed]: nope

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8506345 [details] [diff] [review]
favicon should not change if link element isn't in DOM,

Beta+
Aurora+

Comment 9

[Cornel Ionce [:noni]]

Confirming the fix for latest Nightly, build ID: 20141019030207 using the attached test case.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8cd2d57ee2f6
https://hg.mozilla.org/releases/mozilla-beta/rev/73bc0bc9343b

Comment 11

[Cornel Ionce [:noni]]

Verified fixed on Firefox 34 beta 2, build ID: 20141020184313.

Comment 12

[Cornel Ionce [:noni]]

Also verified using latest Aurora, build ID: 20141021004001.