When we start up, the icons for app tabs often aren't yet loaded, so we use a default icon. What we need to do is listen for attrModified and update the icons when they get loaded, very similar to how we do it in TabItems.
Here's a very simple first stab. It works, in as far as it appears to set the icon. But, your apptabs patch doesn't really seem to work very robustly. When I create an apptab, it doesn't get put on the apptab list of the GroupItem. And when I change the webpage, it doesn't look like attrModified is getting called. Am I doing something obviously wrong?
Attachment #474007 - Flags: feedback?(ian)
Comment on attachment 474007 [details] [diff] [review] v1 Yes, the pinning not having any effect is a known issue (bug 593871); sorry if it was getting in the way. A couple of issues with your work in progress: * AllTabs watches all tabs in all windows; you only need to register once, rather than once per group. * Once you've registered, you should also unregister at the end (to avoid leaks) * App tabs don't have .tabItems, so you've got to deal with them differently. Anyway, I'll take it from here.
Attachment #474007 - Flags: feedback?(ian) → feedback-
I'm hoping to get this in b6
Attachment #474113 - Flags: review?(dietrich)
Attachment #474007 - Attachment is obsolete: true
Comment on attachment 474113 [details] [diff] [review] patch v1 since you're already using DOMAttrModified, as we talked about on irc, not going to r- this over doing it here. if we start getting main-window responsiveness complaints before final, going to look here first though. testing favicons is non-trivial, so set the litmus flag on the bug to request a manual test here. r=me assuming the existing tests pass w/ this change.
Attachment #474113 - Flags: review?(dietrich) → review+
(In reply to comment #4) > Comment on attachment 474113 [details] [diff] [review] > patch v1 > > since you're already using DOMAttrModified, as we talked about on irc, not > going to r- this over doing it here. if we start getting main-window > responsiveness complaints before final, going to look here first though. AllTabs.register("attrModified", ...) is the TabAttrModified event, which is different from DOMAttrModfied.
Summary: App tab icons are generic → App tab icons should be updated when the tab's favicon becomes available
Try server build is clean.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b6
As part of Bug Week, thanks to Mathnerd314 for the new test case in Litmus. Litmus ID 12878
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.