Rob Wu [:robwu] Reporter
	Description • 6 years ago

I opened the global JS console of Firefox 63 and saw a bunch of entries like this:

Attempt to set a remote URL https://assets-cdn.github.com/favicon.ico as a tab icon without a loading principal. tabbrowser.js:782:7
Attempt to set a remote URL https://bugzilla.mozilla.org/extensions/BMO/web/images/favicon.ico as a tab icon without a loading principal. tabbrowser.js:782:7
setIcon
chrome://browser/content/tabbrowser.js:782:7
updateTabLabelAndIcon
resource:///modules/sessionstore/SessionStore.jsm:2783:7
restoreTab
resource:///modules/sessionstore/SessionStore.jsm:3961:5
restoreTabs
resource:///modules/sessionstore/SessionStore.jsm:3780:9
ssi_restoreWindow
resource:///modules/sessionstore/SessionStore.jsm:3573:7
_restoreWindowsFeaturesAndTabs
resource:///modules/sessionstore/SessionStore.jsm:3648:7
_restoreWindowsInReversedZOrder
resource:///modules/sessionstore/SessionStore.jsm:3665:5
ssi_restoreWindows/<
resource:///modules/sessionstore/SessionStore.jsm:3720:7

The error is from:
https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/browser/base/content/tabbrowser.js#809

The caller (updateTabLabelAndIcon) is:
https://searchfox.org/mozilla-central/rev/79bc1ca39f99a8bafa57687c6716c12f11c8520f/browser/components/sessionstore/SessionStore.jsm#2781
(Note that the latest version of the code looks only a bit different):
https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/browser/components/sessionstore/SessionStore.jsm#2791

When I scroll to the right of my tab strip, I see that a whole bunch of tabs have no icon. I guess that this is caused by this bug.


I can consistently reproduce this from scratch as follows:

1. Create an empty directory and use it as the profile directory for Firefox 62.
   /path/to/firefox62/firefox --no-remote -profile /tmp/profdir
2. Visit a website with a favicon, e.g. https://bugzilla.mozilla.org/.
3. Visit about:preferences and select the "Restore previous session" checkbox.
4. Close Firefox 62.
5. Start Firefox 63 (I also tested Nightly 65) with this profile directory
   firefox --no-remote -profile /tmp/profdir

   Firefox should start with the tab from step 2 in a lazy state (and an icon) and the tab from step 3 is activated.
   All good so far.
6. Close Firefox.
7. Start Firefox again.

Expected:
- The restored tab should have an icon.

Actual:
- The restored tab from step 2 has no icon any more, and the console contains errors like the one at the top of this comment.

( I can also trigger the same error by clicking on the tab (to load the lazy tab) after step 5. The icon will be restored, but only because the site sends the new favicon. )

Considering that the loadingPrincipal check was added in bug 1472476, I think that this is a regression caused by bug 1472476.

	Dave Townsend [:mossop]
	Comment 1 • 6 years ago

This is really a regression from bug 1453751 that wasn't properly corrected in bug 1472476. What's happening is that since bug 1453751 we only store the icon url into session store (assumed to be a data or local uri) and not the icon's loading principal. For normal tabs that is fine but for tabs that were restored from a Firefox 62 session and not yet loaded they have a normal web url. We could make session restore store the principal in that case, but given that 63 is out the door already I imagine most users will have already reloaded all such tabs which will have corrected the problem.

I'm not sure it is worth implementing a fix here. What do you think marco?

	Marco Bonardo [:mak]
	Comment 2 • 6 years ago

My opinion didn't change from https://bugzilla.mozilla.org/show_bug.cgi?id=1472476#c3, with the additional fact you added that 63 has been released one month ago already, and 64 is in very late beta stage. That means we could at a maximum fix this in 65, so it would only help a minority of users skipping versions.
It could make some sense to fix it in 63 but now imo it's a wontfix.

	Dave Townsend [:mossop]
	Comment 3 • 6 years ago

Ok, given that this should effect a vanishingly small number of users let's close this.