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

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 59
Iteration:
1.25
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- verified

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.
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.
See Also: → 1362774
Priority: -- → P2
Duplicate of this bug: 1411125
Duplicate of this bug: 1414045
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
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 :)
Duplicate of this bug: 1408739
Blocks: 1421456
Iteration: --- → 1.25
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+
Blocks: 1422074
(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 jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/179f6c267f84
Prevent favicon flickering on about:home and about:newtab. r=florian
https://hg.mozilla.org/mozilla-central/rev/179f6c267f84
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
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
Attached video 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?
Flags: needinfo?(jhofmann)
(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)
Depends on: 1422903
(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: 1422906
Depends on: 1422962
No longer depends on: 1422962
No longer depends on: 1422906
Blocks: 1425449
No longer blocks: 1425449
Depends on: 1425449
Too late for Beta58. Mark 58 won't fix.
Depends on: 1427031
Depends on: 1426916
Blocks: 1442997
See Also: → 1446751
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.