59.0a1 (2017-12-04) (64-Bit) macOS 10.12.6 STR: 1.) Open a Private Window 2.) Open New Tabs in this Private Window 3.) Take a look at the favicon and the title Actual: Flicker is to see. Expected: To see no flicker. Maybe you can suppress the Globe icon and the default New Tab title in Private Mode and show instead the Private Window Favicon and the Private Session Title directly.

59.0a1 (2017-12-04) (64-Bit) Windows 10 x64 This issue occur on Win10 too.

This looks like it's caused by the favicon being set after the DOMContentLoaded event: https://searchfox.org/mozilla-central/rev/ba2b0cf4d16711d37d4bf4d267b187c9a27f6638/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js#45-63

Is this issue also "Depends on: 1422074" ?

No, this one is just a regression that we should fix :)

Marking P3 for regular priority.

Comment on attachment 8936560 [details] Bug 1422903 - Prevent about:privatebrowsing favicon from flickering. https://reviewboard.mozilla.org/r/207300/#review213780 ::: browser/base/content/tabbrowser.xml:809 (Diff revision 2) > + // Don't switch to the default icon on about:home or about:newtab, > + // since these pages get their favicon set in browser code to > + // improve perceived performance. > + let isNewTab = originalLocation && > + (originalLocation.spec == BROWSER_NEW_TAB_URL || > + originalLocation.spec == "about:home"); I just tried this patch (before the == "about:home" check was just added) testing bug 1425449, and the first new tab (about:home) still flickers but the second new tab (not-preloaded about:newtab) does not flicker. I wonder if we just want to say all about pages don't use a default icon `!(originalLocation && originalLocation.scheme == "about")`? A quick test from about:about seems to show no icon for about:credits and about:logo. Acceptable to have no favicon for those without needing to list out these specs?

(In reply to Ed Lee :Mardak from comment #9) > Comment on attachment 8936560 [details] > Bug 1422903 - Prevent about:privatebrowsing favicon from flickering. > > https://reviewboard.mozilla.org/r/207300/#review213780 > > ::: browser/base/content/tabbrowser.xml:809 > (Diff revision 2) > > + // Don't switch to the default icon on about:home or about:newtab, > > + // since these pages get their favicon set in browser code to > > + // improve perceived performance. > > + let isNewTab = originalLocation && > > + (originalLocation.spec == BROWSER_NEW_TAB_URL || > > + originalLocation.spec == "about:home"); > > I just tried this patch (before the == "about:home" check was just added) > testing bug 1425449, and the first new tab (about:home) still flickers but > the second new tab (not-preloaded about:newtab) does not flicker. Can you try it again? Note that there could still be a minimal flicker (I still can't seem to satisfy the automated tests), but it should be much better (I can't notice it with my eyes anymore). > I wonder if we just want to say all about pages don't use a default icon > `!(originalLocation && originalLocation.scheme == "about")`? That sounds unnecessary to me, we're just special-casing about:home and about:newtab and I'd like to avoid unintended side effects on the other pages :)

I noticed in bug 1423256 comment 7 attachment 8937531 [details] that about:blank doesn't have a favicon. Should it be consistent with the other "New Tab"s or maybe it should stay "blank" (other than the title).

Comment on attachment 8936560 [details] Bug 1422903 - Prevent about:privatebrowsing favicon from flickering. https://reviewboard.mozilla.org/r/207300/#review214518 r=me because when testing locally this is such a nice improvement :-). But I have some concerns about the code... Also, do we have any idea of what still causes the flicker caught by the test? I would feel much better about this once the whitelist entry is gone in the flickering test. And are you still considering declaring the favicons of about: urls in a central location so that we can stop having all these hacks in front-end code? ::: browser/base/content/tabbrowser.xml:808 (Diff revision 2) > if (!this.mBrowser.mIconURL && !ignoreBlank) { > + // Don't switch to the default icon on about:home or about:newtab, > + // since these pages get their favicon set in browser code to > + // improve perceived performance. > + let isNewTab = originalLocation && > + (originalLocation.spec == BROWSER_NEW_TAB_URL || This use of BROWSER_NEW_TAB_URL makes me uncomfortable, as the new tab page could have been customized and be an https URL. I wonder if we should just check for about:newtab || about:privatebrowsing instead. ::: browser/base/content/tabbrowser.xml:2824 (Diff revision 2) > throw e; > } > > // Hack to ensure that the about:newtab favicon is loaded > // instantaneously, to avoid flickering and improve perceived performance. > if (aURI == BROWSER_NEW_TAB_URL) { But we already had this problem here...

(In reply to Florian Quèze [:florian] from comment #12) > Comment on attachment 8936560 [details] > Bug 1422903 - Prevent about:privatebrowsing favicon from flickering. > > https://reviewboard.mozilla.org/r/207300/#review214518 > > r=me because when testing locally this is such a nice improvement :-). > > But I have some concerns about the code... Hmmm, looking into BROWSER_NEW_TAB_URL you may be right that it's safer to use about:newtab instead. It's really what we want. I'll change it! > Also, do we have any idea of what still causes the flicker caught by the > test? I would feel much better about this once the whitelist entry is gone > in the flickering test. Right, I agree, I will give it another look before pushing this. > And are you still considering declaring the favicons of about: urls in a > central location so that we can stop having all these hacks in front-end > code? Yes, I would file a new bug (and maybe another one for about:preferences) because I'd like to finish this bug in 59 to fix the associated issues.

I think the remaining flicker on about:home (which is really hard to notice by now) comes from the late promise resolution we're talking about in bug 1403648. So that bug is basically blocking removing the two flicker whitelist items for the tab favicon.

Pushed by jhofmann@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a618b142d9dc Prevent about:privatebrowsing favicon from flickering. r=florian

FYI: I created a follow up bug report (bug 1427186) regarding the "New Tab" Title which appears, before the title "Private Browsing" appears.

I managed to reproduce the bug using an older version of Nightly (2017-12-04) on Windows 10 x64 using the steps from comment 0. I retested everything using Latest Nightly and beta 59.0b6 on Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13 and the bug is not reproducing anymore.