Closed Bug 1401955 Opened 2 years ago Closed 2 years ago
The favicon for the new tab page (and about:home) should be displayed at first paint
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.
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.
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.
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 :)
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.
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+
(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.
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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/179f6c267f84 Prevent favicon flickering on about:home and about:newtab. r=florian
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.
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?
(In reply to Mehmet from comment #16) > Should I file a separate report for this issue? Yes, please! And thanks for catching this :-).
(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.
Depends on: 1444288
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.