Allow for the setting of application level preferences from a distribution

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox51 affected, firefox55 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
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).
(Assignee)

Updated

2 years ago
Blocks: 1314038
Comment hidden (mozreview-request)

Comment 2

2 years ago
mozreview-review
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
Attachment #8850272 - Flags: review?(s.kaspari) → review+
Keywords: checkin-needed

Comment 3

2 years ago
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/565c6c66205d
Allow app level preferences. r=sebastian
Keywords: checkin-needed
Created attachment 8850857 [details]
patch.txt

Sorry I've added the hook to check for style before I push for review so this won't happen aggain.
Flags: needinfo?(cnevinchen) → needinfo?(ihsiao)
Please reopen the reviewboard, get review+, and then ask for checkin.
Flags: needinfo?(ihsiao)
Comment hidden (mozreview-request)
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
Flags: needinfo?(s.kaspari)
Comment on attachment 8850857 [details]
patch.txt

I've resent the review-request so this patch is not needed.
Attachment #8850857 - Attachment is obsolete: true
Flags: needinfo?(s.kaspari)
Keywords: checkin-needed

Comment 11

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f62c008a7b9f
Allow app level preferences. r=sebastian
Keywords: checkin-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
Flags: needinfo?(mozilla)
(Assignee)

Comment 13

2 years ago
mozreview-review
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
Attachment #8850272 - Flags: review-
(Assignee)

Comment 14

2 years ago
Test failure was caused by a typo in the patch.

I've added the issue to the review request.
Flags: needinfo?(mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 18

2 years ago
This was backed out, so you'll need to submit a complete patch again.
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!
Flags: needinfo?(mozilla)
(Assignee)

Comment 20

2 years ago
Yes, to make it easier for someone else to check in the code, it's better to have one complete patch.
Flags: needinfo?(mozilla)
Comment hidden (mozreview-request)
Attachment #8851936 - Attachment is obsolete: true
Attachment #8851936 - Flags: review?(mozilla)
How about it now? Thanks!
Flags: needinfo?(mozilla)
(Assignee)

Comment 23

2 years ago
Awesome, thanks. I'll review and check in.
Flags: needinfo?(mozilla)
(Assignee)

Comment 24

2 years ago
mozreview-review
Comment on attachment 8850272 [details]
Bug 1295675 - Allow app level preferences.

https://reviewboard.mozilla.org/r/122908/#review127312
Attachment #8850272 - Flags: review?(mozilla) → review+
(Assignee)

Comment 25

2 years ago
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

2 years ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/9c6514e9b04b
Allow app level preferences. r=mkaply,sebastian

Comment 27

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9c6514e9b04b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.