Closed
Bug 1083895
Opened 10 years ago
Closed 10 years ago
HTMLLinkElement not in DOM is used for favicon
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
People
(Reporter: dev, Assigned: Gijs)
References
Details
(Keywords: regression, testcase)
Attachments
(2 files)
1.81 KB,
text/html
|
Details | |
6.96 KB,
patch
|
bzbarsky
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 2•10 years ago
|
||
I expect this is an actual regression, yes. Redirecting to bz to be sure.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(bzbarsky)
Comment 3•10 years ago
|
||
Yeah. We should probably not fire DOMLinkChanged for <link>s that are not in the DOM.
Flags: needinfo?(bzbarsky)
Status: UNCONFIRMED → NEW
status-firefox33:
--- → affected
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Ever confirmed: true
Keywords: testcase
Updated•10 years ago
|
Assignee | ||
Comment 4•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
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+
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/719a055fe22b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Assignee | ||
Comment 7•10 years ago
|
||
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?
Updated•10 years ago
|
Iteration: --- → 36.1
Flags: firefox-backlog+
Updated•10 years ago
|
Points: --- → 2
Updated•10 years ago
|
Flags: qe-verify+
Comment 8•10 years ago
|
||
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+
Updated•10 years ago
|
QA Contact: cornel.ionce
Comment 9•10 years ago
|
||
Confirming the fix for latest Nightly, build ID: 20141019030207 using the attached test case.
Comment 10•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/8cd2d57ee2f6 https://hg.mozilla.org/releases/mozilla-beta/rev/73bc0bc9343b
Comment 11•10 years ago
|
||
Verified fixed on Firefox 34 beta 2, build ID: 20141020184313.
Status: RESOLVED → VERIFIED
Comment 12•10 years ago
|
||
Also verified using latest Aurora, build ID: 20141021004001.
You need to log in
before you can comment on or make changes to this bug.
Description
•