1381982 - Firefox onboarding shows duplicate about:home pages

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Peter Dolanjski [:pdol]]

Due to a change with the firstrun webpages (bug 1380137), where a skip button is included for Firefox account sign in, the foreground tab loads about:home (but Firefox also loads about:home in the background tab).

My understanding is that signing in/up to a Firefox account will also result in about:home loading (and thus two about:homes).

See attached video.

Two about:home pages should not be shown.  We should kill the load of about:home from Firefox.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The button that sends the user to about:home located inside an iframe from https://account.firefox.com/.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://github.com/mozilla/bedrock/blob/a2114d69c702e2cb43fb737ab449061b01a8c281/media/js/firefox/firstrun/onboarding.js#L35

Looks like bug 1381051 was deploy to production prematurely?

Comment 3

[Alex Gibson [:agibson]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2)
> https://github.com/mozilla/bedrock/blob/
> a2114d69c702e2cb43fb737ab449061b01a8c281/media/js/firefox/firstrun/
> onboarding.js#L35
> 
> Looks like bug 1381051 was deploy to production prematurely?

Just to be transparent here we we're asked to deploy the new onboarding variations as the new defaults, which have already been undergoing testing for considerable time. What we didn't anticipate was a last second request to change behaviour (loading about:home instead of about:newtab).

Comment 4

[Peter Dolanjski [:pdol]]

(In reply to Alex Gibson [:agibson] from comment #3)
> (In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment
> #2)
> > https://github.com/mozilla/bedrock/blob/
> > a2114d69c702e2cb43fb737ab449061b01a8c281/media/js/firefox/firstrun/
> > onboarding.js#L35
> > 
> > Looks like bug 1381051 was deploy to production prematurely?
> 
> Just to be transparent here we we're asked to deploy the new onboarding
> variations as the new defaults, which have already been undergoing testing
> for considerable time. What we didn't anticipate was a last second request
> to change behaviour (loading about:home instead of about:newtab).

Alex is right.  That's my fault.  I didn't anticipate that that would double up the about:home loads.

Comment 5

[Peter Dolanjski [:pdol]]

Tim, is it simple to kill the about:home load that comes from Firefox? (instead of the one from the iframe)

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

This is what gijs said on IRC:

timdream> Tim Chien Does anyone know where we control the background tab to launch when Firefox first launches, i.e. answer to this question? https://bugzilla.mozilla.org/show_bug.cgi?id=1381982#c5
11:36 PM 
<firebot> Bug 1381982 — NEW, [e-mail] — Firefox onboarding shows duplicate about:home pages
11:39 PM 
<Gijs> timdream: nsBrowserContentHandler.js
11:41 PM 
<timdream> Tim Chien Gijs: did we keep the URL to load in a pref? The file does not contain the string |about:home|. Or, was it passed from the stub installer?
11:41 PM 
<Gijs> timdream: the homepage is in a pref, yes
11:42 PM timdream: start reading in get defaultArgs()
11:42 PM timdream: it's hairy
11:42 PM mostly because people keep wanting different things
11:43 PM timdream: but without actually doing work here (so please please please carefully check this), looks like there's a pref for what that wants
11:43 PM https://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserContentHandler.js#545-548
11:44 PM browser.startup.firstrunSkipsHomepage
11:44 PM hey look, I wrote that, dao reviewed

(The bug he refers to is bug 1322720)

will keep looking tomorrow (keeping needinfo)

Comment 7

[Peter Dolanjski [:pdol]]

Here is the exact expected behaviour:

- Download Firefox Installer
- Run the installer
- Firefox should load after the installer completes
- Two tabs load
- The foreground tab loads the start of the sunrise animation which takes you to the Firefox Account sign in flow where the copy is "Already using Firefox?"
- If the user does any one of the following:
-- Clicks "Skip this step"
-- Creates a Firefox Account
-- Signs in using an existing Firefox Account
 then the sunrise animation completes and about:home is loaded
- The background tab loads (at the same time the foreground tab is initially loaded) the Firefox Privacy Policy (such that if the user switches to this tab or closes the first one, they will see the Privacy Policy)

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

[Tracking Requested - why for this release]: According Peter this is critical enough and needs to be fixed ASAP.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

According to verdi we should apply these funnelcake changes to release:

https://gist.github.com/Verdi/7f03b7f8988e608c66d1a9fcae7f9a38

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Again, this is really not part of photon-onboarding. But I am fine if we want to track it this way....

Given the urgency per pdol I am going to take it on. It should take too much time to do so, simply verify the prefs in comment 9 and comment 7 works as expected.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1381982 - Show Privacy Policy instead of about:home at the background page on first launch,

Review commit: https://reviewboard.mozilla.org/r/159100/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/159100/

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

It turned out the pref to flip is indeed |browser.startup.firstrunSkipsHomepage|, to get exactly what Peter wants in comment 7. I've built Firefox with branding locally and manually verified that (on a Mac).

I wonder how I can geneate FirefoxSetup.exe from Try? That will help making sure this patch workes on Windows too.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #12)
> It turned out the pref to flip is indeed
> |browser.startup.firstrunSkipsHomepage|, to get exactly what Peter wants in
> comment 7. I've built Firefox with branding locally and manually verified
> that (on a Mac).
> 
> I wonder how I can geneate FirefoxSetup.exe from Try? That will help making
> sure this patch workes on Windows too.

I am told that I should be able to get it by asking Try to run in non-artifect build mode. Just did and we will find out soon enough.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94935858a725

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I've manually verify the Try build on Windows and it works as expected.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8888175 [details]
Bug 1381982 - Show Privacy Policy instead of about:home at the background page on first launch,

This has not landed yet but to work with the timezone I want to fill the form first.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1381051, First-run experience deployed on mozilla.org but left the Firefox part out.
[User impact if declined]: The user will left with two about:home tabs if s/he decided to skip signing up sync.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 9.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just a pref flip. The pref has previously deployed with funnelcake.
[String changes made/needed]: None.

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8888175 [details]
Bug 1381982 - Show Privacy Policy instead of about:home at the background page on first launch,

This seems not a critical driver for 54 dot release and we also don't have plan to have 54 dot release. Release54- and mark 54 won't fix.

Comment 17

[Francis Lee [:frlee]]

based on comments, it is not specific to onboarding scope. remove whiteboard tag.

Comment 18

[:Gijs (he/him)]

Comment on attachment 8888175 [details]
Bug 1381982 - Show Privacy Policy instead of about:home at the background page on first launch,

https://reviewboard.mozilla.org/r/159100/#review164578

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0aca5856fa8
Show Privacy Policy instead of about:home as the background tab on first launch, r=gijs

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a0aca5856fa8

Comment 21

[Julien Cristau [:jcristau]]

Comment on attachment 8888175 [details]
Bug 1381982 - Show Privacy Policy instead of about:home at the background page on first launch,

pref flip for onboarding, beta55+

should be in 55.0b12, it would be good to have some verification on this

Comment 22

[Justin [:JW_SoftvisionQA]]

Hey Peter, is there something I am missing here?

I do not see: The foreground tab loads the start of the sunrise animation which takes you to the Firefox Account sign in flow where the copy is "Already using Firefox?"

see video below:

https://jwilliams-softvision.tinytake.com/sf/MTgwMTE3NV81ODY3MjQ5

Comment 23

[Justin [:JW_SoftvisionQA]]

This is with a fresh install and no other instances of Firefox on my machine.

Comment 24

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The sunrise animation only take place on release build, so what in place of that the "Welcome to Firefox Nightly" is the correct page on Nightly channel.

*However*, Nightly opens 3 tabs on 00:29 and the second tab loads about:home, which is not expected... I am sure about that's because of Nightly or there is something wrong with my local tests in comment 14.

I don't really know how we can verify the fix on official Nightly build.

Comment 25

[Dão Gottwald [:dao]]

(In reply to Justin [:JW_SoftvisionQA] from comment #22)
> Hey Peter, is there something I am missing here?

As far as I can tell, this patch isn't in today's nightly build. It should be in tomorrow's.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/456adb45062d

Comment 27

[Justin [:JW_SoftvisionQA]]

I have verified this is fixed on 55 & 56 using the steps in comment 7.

Comment 28

[Andrei Vaida [:avaida]]

Updating status flags per Comment 27.