1295675 - Allow for the setting of application level preferences from a distribution

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android partner distribution, defect)

Description

[Mike Kaply [:mkaply]]

Currently the distribution mechanism can only be used to set profile level preferences.

We should allow app level preferences to be set as well (for instance, session restore).

Comment 1

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1295675 - Allow app level preferences.

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

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

https://reviewboard.mozilla.org/r/122908/#review125314

Nice!

Please update our wiki page:
https://wiki.mozilla.org/Mobile/Distribution_Files

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/565c6c66205d
Allow app level preferences. r=sebastian

Comment 4

[Iris Hsiao [:ihsiao]]

sorry had to back this out for android-checkstyle failure like https://treeherder.mozilla.org/logviewer.html#?job_id=86095946&repo=autoland&lineNumber=3545

https://hg.mozilla.org/integration/autoland/rev/78c1934f6a77

Comment 5

[Nevin Chen(Not active on Bugzilla)]

Attachment: patch.txt

Sorry I've added the hook to check for style before I push for review so this won't happen aggain.

Comment 6

[Iris Hsiao [:ihsiao]]

Please reopen the reviewboard, get review+, and then ask for checkin.

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122908/diff/1-2/

Comment 8

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

Hi Sebastian.
I've added two spaces for check style.
Please help review it.. thank you! (per comment 6)


Best regards,
Nevin

Comment 9

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8850857 [details]
patch.txt

I've resent the review-request so this patch is not needed.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

https://reviewboard.mozilla.org/r/122908/#review125834

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f62c008a7b9f
Allow app level preferences. r=sebastian

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for testdistribution failures like https://treeherder.mozilla.org/logviewer.html#?job_id=86303408&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/f9acfdca68a4

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

https://reviewboard.mozilla.org/r/122908/#review126480

::: mobile/android/base/java/org/mozilla/gecko/preferences/DistroSharedPrefsImport.java:34
(Diff revision 2)
> +        if (appPref.length() != 0) {
> +            applyPreferences(appPref, GeckoSharedPrefs.forApp(context).edit());
> +        }
>  
> -        final JSONObject preferences = distribution.getAndroidPreferences();
> -        if (preferences.length() == 0) {
> +        final JSONObject profilePref = distribution.getPreferences(Distribution.PREF_KEY_PROFILE_PREFERENCES);
> +        if (appPref.length() != 0) {

This should be profilePref

Comment 14

[Mike Kaply [:mkaply]]

Test failure was caused by a typo in the patch.

I've added the issue to the review request.

Comment 15

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1295675 - Check profilePref's length before applying preferences.

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

Comment 16

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122908/diff/2-3/

Comment 17

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8851936 [details]
Bug 1295675 - Check profilePref's length before applying preferences.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124188/diff/1-2/

Comment 18

[Mike Kaply [:mkaply]]

This was backed out, so you'll need to submit a complete patch again.

Comment 19

[Nevin Chen(Not active on Bugzilla)]

Sorry I don't understand.
Do you mean I should combine the two patch in this bug and submit a complete patch again?
Thank you!

Comment 20

[Mike Kaply [:mkaply]]

Yes, to make it easier for someone else to check in the code, it's better to have one complete patch.

Comment 21

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122908/diff/3-4/

Comment 22

[Nevin Chen(Not active on Bugzilla)]

How about it now? Thanks!

Comment 23

[Mike Kaply [:mkaply]]

Awesome, thanks. I'll review and check in.

Comment 24

[Mike Kaply [:mkaply]]

Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

https://reviewboard.mozilla.org/r/122908/#review127312

Comment 25

[Mike Kaply [:mkaply]]

Nevin:

Can you reopen the review request?

https://reviewboard.mozilla.org/r/122906/

Then we can resubmit to autoland.

I've already done successful try runs.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c6514e9b04b
Allow app level preferences. r=mkaply,sebastian

Comment 27

[(PTO May 7-12) Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9c6514e9b04b