Closed Bug 1254268 Opened 4 years ago Closed 4 years ago

Regression: distribution setting user prefs instead of default prefs

Categories

(Firefox :: Distributions, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: mkaply, Assigned: mkaply)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Attached patch Proper fix (obsolete) — Splinter Review
Caused by my checkin for bug 1252466.

I put Preferences.set instead of default.set
Blocks: 1252466
Keywords: regression
Comment on attachment 8727571 [details] [diff] [review]
Proper fix

r=me, as long as you fix the appropriate tests in http://mxr.mozilla.org/mozilla-central/source/browser/components/tests/unit/test_distribution.js to look for default values, rather than user values.
Attachment #8727571 - Flags: review+
(In reply to Mike Connor [:mconnor] from comment #1)
> r=me, as long as you fix the appropriate tests in
> http://mxr.mozilla.org/mozilla-central/source/browser/components/tests/unit/
> test_distribution.js to look for default values, rather than user values.

Brilliant. And had I done that in the first place, it would have caught this.
Carrying over mconnnor r=
Attachment #8727571 - Attachment is obsolete: true
Attachment #8727605 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/e330626de260b21a6ea62d741131d04715ef39a1
Bug 1254268 - Distribution should use default prefs, regression from 1252466; r=mconnor
https://hg.mozilla.org/mozilla-central/rev/e330626de260
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment on attachment 8727605 [details] [diff] [review]
Patch with tests corrected

Approval Request Comment
[Feature/regressing bug #]: 1252466
[User impact if declined]: Distribution prefs set incorrectly.
[Describe test coverage new/current, TreeHerder]: Testcases updated for this case
[Risks and why]: Low. Only affects distributions. Restores old behavior.
[String/UUID change made/needed]: None
Attachment #8727605 - Flags: approval-mozilla-aurora?
Comment on attachment 8727605 [details] [diff] [review]
Patch with tests corrected

Has automated tests, taking it.
Attachment #8727605 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
has problems with uplift to aurora

Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r e330626de260
grafting 332418:e330626de260 "Bug 1254268 - Distribution should use default prefs, regression from 1252466; r=mconnor"
merging browser/components/tests/unit/test_distribution.js
warning: conflicts while merging browser/components/tests/unit/test_distribution.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(mozilla)
That's weird. I'll figure out what's going on and put a new patch together.
Flags: needinfo?(mozilla)
Attached patch Patch for AuroraSplinter Review
I've built and tested this on Aurora.
(In reply to Wes Kocher (:KWierso) from comment #11)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/c3bcee5d608c

Bah, I ended up as the patch author on that. Oops.
You need to log in before you can comment on or make changes to this bug.