Showing start wizard can't be overridden via distribution

RESOLVED FIXED in Firefox 51

Status

()

Firefox for Android
First Run
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

Trunk
Firefox 51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
The pref for the start wizard is explicitly written as true:

https://dxr.mozilla.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java#974

So it can't be overridden at all.

There's no need to explicitly set the preference, just treat it as true if it isn't there.

Comment 1

a year ago
This refers to the first run screen so I'm changing it to First Run component.
Component: General → First Run
(Assignee)

Comment 2

a year ago
Created attachment 8786000 [details] [diff] [review]
Don't set startpage_enabled on profile creation

Now that we have an intent to not show the first run wizard for testing, we should not be explicitly setting the startpage_enabled pref on profile creation.

This allows distributions to turn off the wizard.

I didn't make a change to the first run intent, because it specifically says this:

// Note that we don't set the pref, so subsequent launches can result
// in the firstrun pane being shown.

https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#899
Attachment #8786000 - Flags: review?(liuche)
Comment on attachment 8786000 [details] [diff] [review]
Don't set startpage_enabled on profile creation

Review of attachment 8786000 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me.

::: mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
@@ -1080,5 @@
>          persistClientId(generateNewClientId());
>  
> -        // Initialize pref flag for displaying the start pane for a new profile.
> -        final SharedPreferences prefs = GeckoSharedPrefs.forProfile(mApplicationContext);
> -        prefs.edit().putBoolean(FirstrunAnimationContainer.PREF_FIRSTRUN_ENABLED, true).apply();

Where was the code where you set this off by default in the distribution code? Just curious.
Attachment #8786000 - Flags: review?(liuche) → review+
(Assignee)

Comment 4

a year ago
> Where was the code where you set this off by default in the distribution code? Just curious.

We have an AndroidPreferences section where I set startpane_enabled to false.
This also gets rid of the dependency on the org.mozilla.gecko.firstrun package in GeckoProfile :) Don't forget to remove the `import org.mozilla.gecko.firstrun.FirstrunAnimationContainer` line at the start of GeckoProfile.java!
(Assignee)

Comment 6

a year ago
> We have an AndroidPreferences section where I set startpane_enabled to false.

Actually, this doesn't work after all, but I still think this is a good patch.

The reason it doesn't work is because startpane_enabled is queried very early.

New patch coming because of tree reorg
(Assignee)

Comment 7

a year ago
Created attachment 8788926 [details] [diff] [review]
New patch with removal of dependency and file location change

Carrying over r+
Assignee: nobody → mozilla
Attachment #8786000 - Attachment is obsolete: true
Attachment #8788926 - Flags: review+
(Assignee)

Comment 9

a year ago
FYI, I tested:

New install.
Second start after install.
The "no wizard" intent.
Start after the "no wizard" intent.

Everything worked as before.

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cf134ae84918
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
(Assignee)

Updated

a year ago
Blocks: 1330714
You need to log in before you can comment on or make changes to this bug.