Closed Bug 1503951 Opened 6 years ago Closed 6 years ago

Reduce title flickering for initial new tab page (land other patch from bug 1362774)

Categories

(Firefox :: Tabbed Browser, enhancement, P3)

enhancement

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox65 --- affected

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Original patch: https://hg.mozilla.org/mozreview/gecko/rev/5435b0d6ec9d32772f5bbe8b955bf16c42764a1e

This seems worth rebasing and getting landed. Not entirely sure how hard it'll be, hopefully easier than the rest of that work in terms of having to adjust tests.
(In reply to :Gijs (he/him) from comment #0)
> Original patch:
> https://hg.mozilla.org/mozreview/gecko/rev/
> 5435b0d6ec9d32772f5bbe8b955bf16c42764a1e

Bug 1460423 should have covered some of this.
Depends on: 1460423
Priority: -- → P3
The changes in the trypush means the tab goes from not having a label at all to then having the new tab label a bit later.

This fails a bunch of flicker tests.

I don't really understand what to do about this - the express goal of the relevant patch in bug 1362774 seemed to have been to set the 'new tab' label later, such that we don't first set that, and then have to remove it in favour of something else (like "Google.com" or whatever). I don't think we can have it both ways. Thoughts?
Flags: needinfo?(florian)
I can try to answer that.

What should happen here is that we set the `New Tab` title *early* if the url we are loading is known to be about:blank/about:home etc.

And we *don't* set it at all if we know that this is not one of them (which will result in the title from the document to be set later, when the document is loaded).

Is that not what is happening?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> I can try to answer that.
> 
> What should happen here is that we set the `New Tab` title *early* if the
> url we are loading is known to be about:blank/about:home etc.
> 
> And we *don't* set it at all if we know that this is not one of them (which
> will result in the title from the document to be set later, when the
> document is loaded).
> 
> Is that not what is happening?

OK, so I didn't just rebase the patch, in part because I knew it would just net me loads of conflicts, in part because a bunch of surrounding stuff change, in part because comment #1 implied that some of it happened elsewhere, and those changes actually looked really different.

Looking at the original patch again, it directly checks window.arguments. This strategy was something Dão seemed to feel pretty strongly was not the correct approach over in bug 1362774 when deciding about uriIsAboutBlank, and additionally, just checking window.arguments manually makes bug 1485961 worse, and doesn't work in the presence of session restore (ie the argument might be "about:blank" or "about:home" or about:newtab" but we might not load that page, if we're restoring a session).

I'm not really sure how to proceed here because I don't think we have a guarantee that the  _uriToLoadPromise has resolved at the point where the tabbrowser constructor runs. It might not even exist yet. Either way, I'm not sure we can have it both ways (ie set "New Tab" really early in the right cases, and never have to change it). If not, we need to consider trade-offs.

Dão, have I missed something here? Is there a simpler solution?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dao+bmo)
(Also, if we think the tab going from no title to "New Tab" counts as "flicker", then I don't really see what we're trying to do here - after all, the best case scenario for this bug would be that that behavior is what we'll end up with for non-default homepages, where the current behavior is it going from "New Tab" to "somewhere.com", and the new behavior is it going from "" to "somewhere.com".)
Take a close look at bug 1362774 comment 0; that bug was filed at a time when about:home was different from about about:newtab, including the page title. So this problem doesn't exist anymore except for non-default homepages.

(In reply to :Gijs (he/him) from comment #6)
> we'll end up with for non-default homepages, where the current behavior is
> it going from "New Tab" to "somewhere.com", and the new behavior is it going
> from "" to "somewhere.com".)

Yeah, and remote pages can take a while to load; I think we do want "New Tab" there as a placeholder rather than no title.

To sum it up, I think we can probably close this.
Flags: needinfo?(dao+bmo)
OK, going to close this out per comment #7, then. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: needinfo?(florian)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.