Closed Bug 1083895 Opened 10 years ago Closed 10 years ago

HTMLLinkElement not in DOM is used for favicon

Categories

(Firefox :: Tabbed Browser, defect)

33 Branch
x86_64
Linux
defect
Not set
normal
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 36
Iteration:
36.1
Tracking Status
firefox33 --- wontfix
firefox34 + verified
firefox35 + verified
firefox36 + verified

People

(Reporter: dev, Assigned: Gijs)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files)

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.
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
Blocks: 577892
Component: Untriaged → Tabbed Browser
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: regression
I expect this is an actual regression, yes. Redirecting to bz to be sure.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(bzbarsky)
Yeah.  We should probably not fire DOMLinkChanged for <link>s that are not in the DOM.
Flags: needinfo?(bzbarsky)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
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...
Attachment #8506345 - Flags: review?(bzbarsky)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
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.  :(
Attachment #8506345 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/719a055fe22b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
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
Attachment #8506345 - Flags: approval-mozilla-beta?
Attachment #8506345 - Flags: approval-mozilla-aurora?
Iteration: --- → 36.1
Flags: firefox-backlog+
Points: --- → 2
Flags: qe-verify+
Comment on attachment 8506345 [details] [diff] [review]
favicon should not change if link element isn't in DOM,

Beta+
Aurora+
Attachment #8506345 - Flags: approval-mozilla-beta?
Attachment #8506345 - Flags: approval-mozilla-beta+
Attachment #8506345 - Flags: approval-mozilla-aurora?
Attachment #8506345 - Flags: approval-mozilla-aurora+
QA Contact: cornel.ionce
Confirming the fix for latest Nightly, build ID: 20141019030207 using the attached test case.
Verified fixed on Firefox 34 beta 2, build ID: 20141020184313.
Status: RESOLVED → VERIFIED
Also verified using latest Aurora, build ID: 20141021004001.
See Also: → 1843267
You need to log in before you can comment on or make changes to this bug.