Closed Bug 1594704 Opened 5 months ago Closed 5 months ago

Sync preferences changed after reconnecting the sync are not saved

Categories

(Firefox :: Sync, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 + verified
firefox72 --- verified

People

(Reporter: vasilica.mihasca, Assigned: markh)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(4 files)

Attached image sync options.gif

[Affected versions]:
Firefox 72.0a1 (2019-11-06)
Firefox 71.0b7

[Affected platforms]:
Windows 10 64-bit
MAC OS X 10.14.6

[Steps to reproduce]:

  1. Launch Firefox and login using a Firefox Account.
  2. Navigate to about:preferences#sync
  3. Uncheck some Sync options and click “Save Changes”
  4. Perform a Sync.
  5. Disconnect Sync.
  6. Click on ”Set Up Sync” and check some of the unselected options.
  7. Click “Save Changes” button.

[Expected Results]:
Sync preferences changes are successfully saved.

[Actual Results]:
Sync preferences changed after reconnecting the sync are replaced by the sync options saved before sync disconnection.

Attached video Sync pref change.mov

This is pretty bizarre, in the attached video you can see where preferences become deselected while it is syncing.

I initially thought this was the same as it had always been, but Janet put me on the right track. What has changed is that now it is possible to choose what to sync as part of connecting sync to this device. This works fine if it is the first device to sync, but fails if it is not - we replace the selections on this device with what's on the server. This was fine before because the user didn't have a chance to choose locally in this scenarion - but now they do.

Priority: -- → P1
Component: Firefox Accounts → Sync
Assignee: nobody → markh

The fix to this is very simple - when we check the server for current choices and update the engine states to match (bug 1584249), our machinery thinks the changes made by that process were actually made by the user. The patch is a bit bigger than necessary so we can get sane test coverage.

Ed did bug 1584249, so he draws the review short-straw :)

Try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=f79991e2774f786c40b50d6cded8c0419b451ec4 - Ed, if you give this r+, could you please also land it? That will save waiting until next week because we also need to request uplift.

Status: NEW → ASSIGNED

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression
Pushed by mhammond@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf1d04bbfc0b
ensure sync data choices made before connecting sync are honored. r=eoger
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

Comment on attachment 9108927 [details]
Bug 1594704 - ensure sync data choices made before connecting sync are honored. r?eoger

Beta/Release Uplift Approval Request

  • User impact if declined: Users may sync things which are different from what they chose to sync.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Risk is limited to the sync connection flow. Patch is small and testing is relatively easy.
  • String changes made/needed: None
Attachment #9108927 - Flags: approval-mozilla-beta?

Comment on attachment 9108927 [details]
Bug 1594704 - ensure sync data choices made before connecting sync are honored. r?eoger

P1, patch has tests, I'd like it to be manually verified by QA on beta since this was manually found by QA, uplift approved for 71 beta 11, thanks.

Attachment #9108927 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]
Attached image syncoptions.gif

Verified this issue as fixed on Firefox 72.0a1 (2019-11-19) and Firefox 71.0b11 ( 20191118154140) under Windows 10 64-bit and Mac OS X 10.14.6. Sync changes are successfully saved after reconnecting to sync.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.