1401955 - The favicon for the new tab page (and about:home) should be displayed at first paint

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P2)

Description

[Florian Quèze [:florian]]

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

[Ed Lee :Mardak]

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.

Comment 2

[Florian Quèze [:florian]]

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.

Comment 5

[Johann Hofmann [:johannh]]

Attachment: Bug 1401955 - Prevent favicon flickering on about:home and about:newtab.

This sets the hardcoded favicon paths before painting the tab or window
to ensure they're painted right away.

Review commit: https://reviewboard.mozilla.org/r/201000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201000/

Comment 6

[Johann Hofmann [:johannh]]

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 7

[Johann Hofmann [:johannh]]

Comment on attachment 8929723 [details]
Bug 1401955 - Prevent favicon flickering on about:home and about:newtab.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201000/diff/1-2/

Comment 9

[Johann Hofmann [:johannh]]

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 10

[Florian Quèze [:florian]]

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.

Comment 11

[Ed Lee :Mardak]

(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.

Comment 12

[Johann Hofmann [:johannh]]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/179f6c267f84
Prevent favicon flickering on about:home and about:newtab. r=florian

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/179f6c267f84

Comment 15

[Ciprian Muresan [:cmuresan], Ecosystem QA]

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.

Comment 16

[Mehmet]

Attachment: PrivateMode.mov

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?

Comment 17

[Florian Quèze [:florian]]

(In reply to Mehmet from comment #16)

> Should I file a separate report for this issue?

Yes, please! And thanks for catching this :-).

Comment 18

[Mehmet]

(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.

Comment 19

[Gerry Chang [:gchang]]

Too late for Beta58. Mark 58 won't fix.