Closed Bug 1500114 Opened 3 years ago Closed 3 years ago

First run page is displayed blank on esr and release builds after updating to the latest version (broken/missing triggering principal on the restart after the update causes us to fail to load about:newtab)

Categories

(Firefox :: Session Restore, defect, P2)

defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix

People

(Reporter: ciprian_georgiu, Assigned: jkt)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

[Affected versions]:
- 63.0-build1 (20181015152800)
- 60.3.0esr-build1 (20181017185317)

[Affected platforms]:
- Windows 10 x64
- macOS 10.13
- Ubuntu 16.04 x64

[Steps to reproduce]:
1. Download Firefox 55.0.3 `en-US`, or any other locale: https://archive.mozilla.org/pub/firefox/candidates/55.0.3-candidates/build2/win64/es-ES/
2. Launch the browser on a clean profile.
3. Sign in to Firefox Accounts via “Welcome to Firefox” page.
- confirm the account if necessary and wait until sync is completed  
4. Update Firefox to 63.0 from “About Firefox” option.

[Expected result]:
- The first run/new tab page is properly displayed in Firefox.

[Actual result]:
- The first run/new tab page (for the en-US/es-ES locale) is displayed as a blank page and also there’s no link populated in the URL bar.

[Regression range]:
- I don't think this is a regression at least not a recent one, I managed to reproduce this issue while updating to FF 62.0.3 as well. I will investigate further, asap; in case of an old regression.

[Additional notes]:
- Please note, that this issue is reproducible on Firefox Release builds only if the user is logged into Sync before updating the browser to its latest version, and it happens with other builds too; e.g. 56.0-build6, `de` locale.
- On Esr builds, I’ve managed to reproduce it, without login into FxA first, e.g. when updating from 52.1.1esr `ko` and 52.3.0esr `fr` locales to 60.3.0-build1. It appears that the first run page is displayed blank in Firefox if the user is coming from an ESR version <52.x.x.
- If this helps please see logs from browser console: https://pastebin.com/bir3GcUT
(In reply to Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA from comment #0)
<snip>
> [Actual result]:
> - The first run/new tab page (for the en-US/es-ES locale) is displayed as a
> blank page and also there’s no link populated in the URL bar.
Hi Ciprian, the screenshot has the url in the url bar which is contrary to this statement. Can you confirm? Thanks!
Flags: needinfo?(ciprian.georgiu)
For the first run page all that app update does is save the xml data provided by balrog which includes the url as well as set a pref and there is Firefox specific code that opens the url. The screenshot shows that the url is opened so it appears that balrog is correctly providing the data and that the pref is set.

Given the above and with this only happening "on Firefox Release builds only if the user is logged into Sync before updating the browser to its latest version" I highly suspect this is caused by a recent change to Sync that is interacting badly with opening a new page on startup from the following Firefox code that provides the url.
https://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserContentHandler.js#118

needinfoing dolske to give him a heads up.
Flags: needinfo?(dolske)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #1)
> (In reply to Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA from
> comment #0)
> <snip>
> > [Actual result]:
> > - The first run/new tab page (for the en-US/es-ES locale) is displayed as a
> > blank page and also there’s no link populated in the URL bar.
> Hi Ciprian, the screenshot has the url in the url bar which is contrary to
> this statement. Can you confirm? Thanks!

Hey Robert, I think I had in mind this screenshot https://drive.google.com/open?id=11fwVKztlY3lJnYbRV8Unc9gdt53bhmKM when I wrote the last part of that AR; taken from an `es-ES` locale build, where the about:newtab page was displayed blank, after updating to the latest version. An issue that I have also encountered during this testing.

But yes, looking again at all the screenshots I’ve made for the first run page, the URL was present in the URL bar.

Let me know if I can help with more info. Thanks!
Flags: needinfo?(ciprian.georgiu)
Thanks Ciprian, this doesn't appear to be due to app update per comment #2 so changing component
Component: Application Update → General
Product: Toolkit → Firefox
Julie, is there someone on the Sync team who can take a look at this?
Flags: needinfo?(jmccracken)
Hi Ciprian, would you mind testing with add-ons and preferences unchecked in about:preferences#sync, please? Desktop Sync is in maintenance mode; I can't think of any recent changes we've made here, but it might be that we're syncing a bogus pref or extension around. Thanks!
Flags: needinfo?(jmccracken) → needinfo?(ciprian.georgiu)
(In reply to Lina Cambridge (she/her) [:lina] from comment #6)
> Desktop Sync is in maintenance mode; I can't think of any recent changes we've made here...

Hmm, I was wrong. :-) This might be related to bug 1499766. Ciprian, instead of add-ons and preferences, could you please try disabling addresses and credit cards?
(In reply to Lina Cambridge (she/her) [:lina] from comment #7)
> (In reply to Lina Cambridge (she/her) [:lina] from comment #6)
> > Desktop Sync is in maintenance mode; I can't think of any recent changes we've made here...
> 
> Hmm, I was wrong. :-) This might be related to bug 1499766. Ciprian, instead
> of add-ons and preferences, could you please try disabling addresses and
> credit cards?

It seems that the issue is still reproducible even with the following prefs set on false, on FF 56.0: "extensions.formautofill.addresses.enabled" and "extensions.formautofill.creditCards.enabled" before updating to FF 63.0.

Also, it appears the issue is not repro if the user is signed into FxA via about:preferences, only from the first run page. 

Starting with FF 57.0 the user is redirected to about:newtab, after syncing with FxA via first run page, and the issue is no longer encountered, since upon restart about:newtab is opened instead of the first run page.
Flags: needinfo?(ciprian.georgiu)
The log has:

SessionHistory: Couldn't deserialize the triggeringPrincipal, falling back to NullPrincipal
Security Error: Content at moz-nullprincipal:{cd018427-ac2f-40c9-a11e-7014a45cdad4} may not load or link to about:home.

I think this needs a fix in session restore to use a different fallback value, or to realize that the session file comes from an old version that doesn't store triggering principals (or doesn't store them in a compatible format).

Passing this to Mike. This isn't a great experience for users, so if we could get a fix into 64 I think that'd be a Good Thing.
Component: General → Session Restore
Flags: needinfo?(mdeboer)
I kind of expect this is fallout from bug 1461407, but then it doesn't really make sense that 60esr is affected.

Can you get a browser console log from the 60esr case and see if the same error crops up there?
Flags: needinfo?(ciprian.georgiu)
Summary: First run page is displayed blank on esr and release builds after updating to the latest version → First run page is displayed blank on esr and release builds after updating to the latest version (broken/missing triggering principal on the restart after the update causes us to fail to load about:newtab)
> I think this needs a fix in session restore to use a different fallback value

Ah does ESR not have the triggeringPrincipal?
(In reply to Jonathan Kingston [:jkt] from comment #11)
> > I think this needs a fix in session restore to use a different fallback value
> 
> Ah does ESR not have the triggeringPrincipal?

60esr would do, but 52 wouldn't, I guess. It's possible that the builds from which this suggests upgrading (52, 55, 56) didn't, I guess. I'm not sure off-hand exactly when we started storing these in session restore.

Thinking about it more, comment #10 is probably a red herring in that the *target* page is about:home/about:newtab, and the triggering principal might have been the first run page or something? I dunno.

A dump of the sessionstore file from the profile right before clicking the 'restart' button for the upgrade (waiting a minute or two before clicking it to ensure it's been written out) may be helpful in figuring out exactly what's missing. I don't have a setup where I can easily test this myself right now.
(In reply to :Gijs (he/him) from comment #10)
> I kind of expect this is fallout from bug 1461407, but then it doesn't
> really make sense that 60esr is affected.
> 
> Can you get a browser console log from the 60esr case and see if the same
> error crops up there?

I can see some errors related to session restore on esr build as well, e.g.: SessionHistory.jsm:456
TypeError: this._historyListener is null  ContentRestore.jsm:286:5. I have pasted here the entire browser console output from 60.3.0esr: https://pastebin.com/TuVUTH05. Hope this helps!
Flags: needinfo?(ciprian.georgiu)
(In reply to Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA from comment #13)
> (In reply to :Gijs (he/him) from comment #10)
> > I kind of expect this is fallout from bug 1461407, but then it doesn't
> > really make sense that 60esr is affected.
> > 
> > Can you get a browser console log from the 60esr case and see if the same
> > error crops up there?
> 
> I can see some errors related to session restore on esr build as well, e.g.:
> SessionHistory.jsm:456
> TypeError: this._historyListener is null  ContentRestore.jsm:286:5. I have
> pasted here the entire browser console output from 60.3.0esr:
> https://pastebin.com/TuVUTH05. Hope this helps!

Thanks, this log also has:

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISerializationHelper.deserializeObject]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/sessionstore/Utils.jsm :: deserializePrincipal :: line 143"  data: no]
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISHEntry.triggeringPrincipal]  SessionHistory.jsm:456

So yeah, somehow our principal serialization code changed and that's breaking this stuff.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs (he/him) from comment #9)
> The log has:
> 
> SessionHistory: Couldn't deserialize the triggeringPrincipal, falling back
> to NullPrincipal
> Security Error: Content at
> moz-nullprincipal:{cd018427-ac2f-40c9-a11e-7014a45cdad4} may not load or
> link to about:home.
> 
> I think this needs a fix in session restore to use a different fallback
> value, or to realize that the session file comes from an old version that
> doesn't store triggering principals (or doesn't store them in a compatible
> format).
> 
> Passing this to Mike. This isn't a great experience for users, so if we
> could get a fix into 64 I think that'd be a Good Thing.

I agree, but I have no clue about how this _should_ work and/ or what a proper fallback value would be here. I'm looping in the experts here, if I'm right.
Flags: needinfo?(mdeboer)
Flags: needinfo?(jkt)
Flags: needinfo?(ckerschb)
My view is that using system explicitly for the builds that we know are broken is completely OK. Is there any way we can target the change just to ESR though? I would us rather use the new improved security for all other releases where we can.
Do we have a pref for the session storage changes or a way to detect that?
Flags: needinfo?(jkt)
(In reply to Mike de Boer [:mikedeboer] from comment #15)
> (In reply to :Gijs (he/him) from comment #9)
> > The log has:
> > 
> > SessionHistory: Couldn't deserialize the triggeringPrincipal, falling back
> > to NullPrincipal
> > Security Error: Content at
> > moz-nullprincipal:{cd018427-ac2f-40c9-a11e-7014a45cdad4} may not load or
> > link to about:home.

Can we somehow limit the attack surface and just generate a different principal (other than a nullprincipal) as the fallback principal in case we are loading whatever about: URL? A general fallback to using the SystemPrincipal sounds possibly dangerous to me and I would like to just do 'enough' to make that corner case work.

Or are we going to face more problems similar to this one for other URLs? I would imagine not, because a NullPrincipal should be able to load whatever top-level web URL.
Flags: needinfo?(ckerschb)

Jonathan, I think Christoph meant to n-i you here...

Priority: -- → P2
Flags: needinfo?(jkt)

This likely can be solved in Bug 1521878 and if not a follow up here.

Assignee: nobody → jkt
Depends on: 1521878
Flags: needinfo?(jkt)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dolske)

Jonathan, can you check if this is fixed by bug 1521878, by using a 55 nightly and then updating that to today's nightly with the steps from comment #0?

Flags: needinfo?(jkt)

This works for me, I followed the steps to update and didn't see the blank page through the upgrade process.

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(jkt)
Resolution: --- → WORKSFORME
Blocks: 1594162
You need to log in before you can comment on or make changes to this bug.