Closed
Bug 1401955
Opened 7 years ago
Closed 7 years ago
The favicon for the new tab page (and about:home) should be displayed at first paint
Categories
(Firefox :: New Tab Page, defect, P2)
Firefox
New Tab Page
Tracking
()
People
(Reporter: florian, Assigned: johannh)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
Currently when opening a new window (and sometimes a new tab), the activity stream favicon appears a couple frames later, causing the tab title position to shift to the right.
Ideally that favicon should already be there at first paint, but if that's not possible we should at least reserve the space for it.
Comment 1•7 years ago
|
||
It's using <link rel="icon" type="image/png" id="favicon" href="chrome://branding/content/icon32.png"/>
Not sure if it would just be hardcoded somewhere probably for both about:home and about:newtab.
Updated•7 years ago
|
Reporter | ||
Comment 2•7 years ago
|
||
Yes, I think it would be fine to make the browser UI show it when we are loading about:home or about:newtab, instead of waiting for the <link> tag to be processed.
Reporter | ||
Updated•7 years ago
|
Updated•7 years ago
|
status-firefox58:
--- → affected
Priority: -- → P2
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
The fix is as hacky as expected, but given that it's all pretty obvious and straightforward I don't think I'm committing an atrocity to our code base, considering the benefits.
Note that the about:home code isn't technically _before_ first paint, but rather whenever that promise resolves, which I think is the reason why I can still reproduce some tiny flickering when opening a ton of new windows quickly, on my machine. This effect might be more perceivable on slower machines, I haven't tested it. I don't know a better way to solve it, though, and that issue doesn't block this patch IMO :)
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Updated•7 years ago
|
Iteration: --- → 1.25
Assignee | ||
Comment 9•7 years ago
|
||
Any particular reason why you set this to 58 wontfix? As the author of the patch, I'd like to request uplift if we can get it reviewed in time.
Reporter | ||
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8929723 [details]
Bug 1401955 - Prevent favicon flickering on about:home and about:newtab.
https://reviewboard.mozilla.org/r/201000/#review209880
I'm r+'ing because this is a nice incremental improvement over the current situation, but:
- this is unfortunately not enough to remove the related exceptions in my flickering test (bug 1421456). I ran the test a couple times with your patch applied. In most cases the favicon isn't there at first paint and appears later. In one case, the favicon wasn't there, then appeared, and then was replaced by the default icon: https://i.imgur.com/aJx22Q5.png We should investigate where this comes from.
- we should remove the <link> tag from AS. Not sure which process this should use exactly and I don't want to block on it, so let's just open a follow-up.
- some of the remaining flicker that you observed likely comes from bug 1403648 that we should really fix.
Attachment #8929723 -
Flags: review?(florian) → review+
Comment 11•7 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #9)
> I'd like to request uplift if we can get it reviewed in time.
Sure. I was just mass marking bugs as 58=wontfix as gchang has been pushing back on uplifts for activity stream stuff.
Assignee | ||
Comment 12•7 years ago
|
||
Thanks for the review! As mentioned in comment 6 it was to be expected that this wouldn't perfectly solve the about:home case, but I agree that it gets us to a point where finding a perfect solution is no longer super urgent. I'll land it and maybe we can still make 58.
Comment 13•7 years ago
|
||
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/179f6c267f84
Prevent favicon flickering on about:home and about:newtab. r=florian
Comment 14•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Comment 15•7 years ago
|
||
I have verified that the issue is no longer reproducible using the latest Nightly 59.0a1 (Build ID 20171204100103) on Windows 10 x64, Mac OS 10.12.6 and Arch Linux x64.
Status: RESOLVED → VERIFIED
OS: Unspecified → All
Hardware: Unspecified → All
Comment 16•7 years ago
|
||
Hey Johann, in Private Windows it flickers and looks a bit strange now with the globe icon that is to see before the Private Window favicon appears.
Please find attached a screencast.
Should I file a separate report for this issue?
Flags: needinfo?(jhofmann)
Reporter | ||
Comment 17•7 years ago
|
||
(In reply to Mehmet from comment #16)
> Should I file a separate report for this issue?
Yes, please! And thanks for catching this :-).
Flags: needinfo?(jhofmann)
Comment 18•7 years ago
|
||
(In reply to Florian Quèze [:florian] from comment #17)
> (In reply to Mehmet from comment #16)
>
> > Should I file a separate report for this issue?
>
> Yes, please! And thanks for catching this :-).
You're welcome. Done --> Bug 1422903.
Updated•7 years ago
|
Comment 19•7 years ago
|
||
Too late for Beta58. Mark 58 won't fix.
Updated•7 years ago
|
Updated•5 years ago
|
Component: Activity Streams: Newtab → New Tab Page
You need to log in
before you can comment on or make changes to this bug.
Description
•