Closed
Bug 1124956
Opened 10 years ago
Closed 10 years ago
Fix Sync engine selection after migration to FxA
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
Tracking | Status | |
---|---|---|
firefox36 | --- | unaffected |
firefox37 | + | fixed |
firefox38 | + | fixed |
People
(Reporter: adw, Assigned: markh)
References
Details
Attachments
(1 file)
(deleted),
patch
|
adw
:
review+
Gavin
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug is the result of bug 1119578. We should just rename the services.sync.ui.showCustomizationDialog pref so that Svc.Prefs.resetBranch doesn't clear it. See bug 1119578 for context. Renaming the pref does cause the customize.xul dialog to open after migration as expected, but there's still some problem because all its checkboxes are checked regardless of which engines you selected before migration. So that needs to be investigated as part of this bug. Bug 1113493 should have fixed that.
Flags: qe-verify-
Flags: firefox-backlog+
Assignee | ||
Comment 1•10 years ago
|
||
There are 3 parts to this patch:
* Rename the pref as we discussed.
* Introduces a new observer notification weave:service:start-over:init-identity which is sent by .startOver() after everything has been reset but before the new identity has been initialized. The root of the problem was that this modal dialog (and hence reading prefs etc) happened *before* our weave:service:start-over:finish observer is called - so we were correctly resetting the prefs, but *after* they had been acted on (doh!).
* Adds the sync migration module's log to the sync log initialization code - this is technically unrelated to this patch, but it addresses something I forgot to do previously.
With this patch applied things work perfectly.
Assignee: nobody → mhammond
Attachment #8555041 -
Flags: review?(adw)
Assignee | ||
Comment 2•10 years ago
|
||
[Tracking Requested - why for this release]:
We are hoping to enable Sync migration on 37, but it isn't going to work correctly without this patch.
Updated•10 years ago
|
Updated•10 years ago
|
Iteration: --- → 38.2 - 9 Feb
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8555041 [details] [diff] [review]
0004-Bug-1124956-Fix-Sync-engine-selection-after-migratio.patch
Review of attachment 8555041 [details] [diff] [review]:
-----------------------------------------------------------------
Cool, thanks.
::: services/sync/modules/policies.js
@@ +591,5 @@
> let fapp = this._logAppender = new Log.StorageStreamAppender(formatter);
> fapp.level = Log.Level[Svc.Prefs.get("log.appender.file.level")];
> root.addAppender(fapp);
>
> + // Arrange for the a number of other sync-related logs to also go to our
Typo nit: the a -> a
Attachment #8555041 -
Flags: review?(adw) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Comment 5•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8555041 [details] [diff] [review]
0004-Bug-1124956-Fix-Sync-engine-selection-after-migratio.patch
Approval Request Comment
[Feature/regressing bug #]: Sync migration.
[User impact if declined]: Users migrating sync may lose their sync options.
[Describe test coverage new/current, TreeHerder]: Tests pass
[Risks and why]: Low, limited to sync migration
[String/UUID change made/needed]: None
Attachment #8555041 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8555041 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 7•10 years ago
|
||
Needs a bit of rebasing around bug 1121325 (or an uplift on that too).
Flags: needinfo?(mhammond)
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #7)
> Needs a bit of rebasing around bug 1121325 (or an uplift on that too).
Yep, I've an uplift request on that bug - hopefully it will be approved soon.
Flags: needinfo?(mhammond)
Comment 9•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•