HTMLLinkElement not in DOM is used for favicon

VERIFIED FIXED in Firefox 34



5 years ago
5 years ago


(Reporter: dev, Assigned: Gijs)


({regression, testcase})

33 Branch
Firefox 36
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox33 wontfix, firefox34+ verified, firefox35+ verified, firefox36+ verified)



(2 attachments)

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:

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)
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
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+
Closed: 5 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,

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.
Also verified using latest Aurora, build ID: 20141021004001.
You need to log in before you can comment on or make changes to this bug.