1322719 - Replace about:home with about:newtab as the default homepage for the feb 2017 funnelcake

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[:Gijs (he/him)]

This should be pref-off-able in some way so we can keep the normal behaviour for the funnelcake

Comment 1

[:Gijs (he/him)]

This might be harder than I thought it was going to be. browser.startup.homepage is a localized pref, with the default being in   https://dxr.mozilla.org/mozilla-central/source/browser/branding/official/locales/browserconfig.properties . From what I can tell from the implementation in nsPrefBranch ( https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/modules/libpref/nsPrefBranch.cpp#217 ) this basically means that for:
- default values it reads the value of the pref and then tries to read the value of the relevant string from the .properties file that the pref value points to (ie the pref value *must* point to a .properties file). There doesn't seem to be a fallback for the default value not being a pointer to a .properties file.
- non-default values, it just returns the (char) value of the pref.

Nick, for funnelcakes/repacks, can we change these values, ie can you override browserconfig.properties or similar? (The pref in question is browser.startup.homepage)

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

I don't know about the underlying code which handles distribution.ini, but here is an example of an old funnelcake which sets browser.startup.homepage:
 https://github.com/mozilla-partners/funnelcake/blob/master/old/funnelcake41/distribution/distribution.ini#L13

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Or https://hg.mozilla.org/build/partner-repacks/file/default/partners/yahoo-en-US_keyword/distribution/distribution.ini#l40 if that isn't visible.

Comment 4

[:Gijs (he/him)]

Ah, so there's a [LocalizablePreferences] ini section. That works. I think we might still need to write some other code here. I'll look in more detail tomorrow. Thanks!

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1322719 - treat about:newtab like about:home in our UI when it's the default,

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

Comment 6

[:Gijs (he/him)]

I could be convinced to wontfix this bug instead of taking this patch, but if we do want to do something it feels like we should treat about:newtab the same as about:home in being the "Firefox start page" or whatever. For about:home, I explicitly check that it's the default value so that we don't confuse users who have explicitly set it to about:newtab on a "normal" build. I didn't want to just use the default value because then for partner builds where e.g. www.foo.com is the default, we shouldn't pretend that that's the "Firefox start page" and not tell them the URL.

Comment 7

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #6)
> For
> about:home, I explicitly check that it's the default value so that we don't
> confuse users who have explicitly set it to about:newtab on a "normal"
> build.

Err, that should have read "For about:newtab". Note that I didn't bother with the same logic for the home button tooltip where that distinction seemed less important (as the prefs have UI to reset, and the home button in the main browser doesn't).

Comment 8

[Dão Gottwald [:dao]]

Not to say that your patch is wrong, but here's another idea: How about we make about:home an alias to about:newtab at a lower level? Can we do that conditionally?

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8819342 [details]
Bug 1322719 - treat about:newtab like about:home in our UI when it's the default,

https://reviewboard.mozilla.org/r/99164/#review99460

I suppose we'll need this patch either way.

Comment 10

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #8)
> Not to say that your patch is wrong, but here's another idea: How about we
> make about:home an alias to about:newtab at a lower level? Can we do that
> conditionally?

Not easily... there are other conditions elsewhere in the code (for instance, what loads in the parent/non-parent process, what loads with chrome privileges and what without) that are different, and then there's code that depends on that (like the about: tabsprogresslistener code in browser.js that I didn't know existed until a few hours ago) that would need changing... so aliasing it seems scary in terms of side-effects/complexity. :-(

With the existing patch, I would assume we basically just land this patch + have the funnelcake set browser.startup.homepage, and AIUI that Should Just Work. Does that sound sane?

Comment 11

[Dão Gottwald [:dao]]

Sounds okay.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd570e5a9b45
treat about:newtab like about:home in our UI when it's the default, r=dao

Comment 13

[Phil Ringnalda (:philor)]

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

Comment 14

[Dão Gottwald [:dao]]

Is this ready for uplift?

Comment 15

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #14)
> Is this ready for uplift?

Waiting to hear from Dolske, going to clear ni for now.

Comment 16

[:Gijs (he/him)]

Comment on attachment 8819342 [details]
Bug 1322719 - treat about:newtab like about:home in our UI when it's the default,

Approval Request Comment
[Feature/Bug causing the regression]: bug 1322718 comment #1
[User impact if declined]: not having this patch will affect the experience of users who run the funnelcake.
[Is this code covered by automated tests?]: No. This code only takes effect if the homepage pref's default is changed. I decided it was enough of an edgecase to not bother with an automated test.
[Has the fix been verified in Nightly?]: no, as it needs a custom build with a pref changed.
[Needs manual test from QE? If yes, steps to reproduce]: Yes. We'll need a custom build set up. With the custom build, we should check that the tooltip of the 'home' button is "Mozilla Firefox start page", and ditto for the contents of the 'home page' setting in the preferences if the default homepage is set to about:newtab.
[List of other uplifts needed for the feature/fix]: this is related to all the other deps of bug 1322718, but the patch is standalone and can be uplifted independently.
[Is the change risky?]: no
[Why is the change risky/not risky?]: because it will mostly affect the funnelcake (ie it's behind a pref). The only effect on normal builds is that the tooltip of the homepage button will say "Mozilla Firefox Home Page" for users who have configured their homepage to be "about:newtab" manually (without these patches, the tooltip will say "about:newtab".
[String changes made/needed]: no.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8819342 [details]
Bug 1322719 - treat about:newtab like about:home in our UI when it's the default,

Small changes in newtab for the funnelcake onboarding experiment.
Thanks for explaining the behavior changes in details, Gijs!

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d2f676e88f05

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/571c1eba5b6a

Comment 20

[Andrei Vaida [:avaida]]

Setting the flag for verification, instructions available in Comment 16.

Comment 21

[Alexandru Simonca [:asimonca], Release Desktop QA]

Hi Gijs,
Could you possibly provide the custom build you mentioned in Comment #16? I want to verify the fix for 52 & 53. Thanks.

Comment 22

[:Gijs (he/him)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #21)
> Hi Gijs,
> Could you possibly provide the custom build you mentioned in Comment #16? I
> want to verify the fix for 52 & 53. Thanks.

53: remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=d25f298024796f3d33a2892d4ecff5e2bed6ed98
52: remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=736a9d8dec8febf77ebd346fb0ed2e1fe39bcc79

Comment 23

[Alexandru Simonca [:asimonca], Release Desktop QA]

Based on Comment #16 and on the try builds Gijs provided I consider this issue to be VERIFIED FIXED.

The tooltip for the Home button in the try builds reads "Nightly Start Page" and the contents of the 'home page' setting in the preferences is the same while the default homepage is set to "Show my homepage". 
This applies to Windows 10 x64 and Mac OS X 10.11.6.