Closed Bug 638818 Opened 9 years ago Closed 9 years ago

Prompted for Sync account removal on Clear Private Data without Sync account setup

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: aaronmt, Assigned: wesj)

References

Details

(Keywords: regression, Whiteboard: [has patch][has review][has approval][can land])

Attachments

(2 files)

Mozilla/5.0 (Android; Linux armv7l; rv:2.0b13pre) Gecko/20110304
Firefox/4.0b13pre Fennec/4.0b6pre

With a new profile, I had hit the Clear Private Data option in preferences and was prompted that my Sync account would be removed. With a new profile and Sync not actively set up, this prompt should not be visible.

STR:
1. Create a new profile
2. Under preferences, tap Clear Private Data
Summary: Prompted for Sync account removed on Clear Private Data without Sync account setup → Prompted for Sync account removal on Clear Private Data without Sync account setup
Attached patch Patch v1Splinter Review
Pretty conservative patch. Don't want to accidentally leave sync in a strange state.
Wesley, did you want to ask for review for the patch?
Attachment #516962 - Flags: review?(mbrubeck)
Really simple, low risk polish patch. Should be fine for 4.0. Asking for approval
tracking-fennec: --- → ?
Attachment #516962 - Flags: review?(mbrubeck) → review+
Assignee: nobody → wjohnston
Blocks: 636223
Status: NEW → ASSIGNED
Whiteboard: [has patch][has review][needs approval]
Question from Stuart on IRC:

(05:21:46 PM) stuart: wesj: in 638818, this own't cause sync to start up for this check right?
(05:22:02 PM) stuart: i know we did something to check if sync was going that was cheaper than some other way we had been doing it
(05:22:19 PM) wesj: stuart: not sure. let me look
(05:22:36 PM) stuart: also, do we want that disconnect thing in the if() {}?

I think we just wanted to avoid Weave.Service calls in Fennec (bug 590615), which the other patch doesn't add (but also doesn't remove). I was avoiding changing much more because I was/am worried there are other settings that need to be reset.

This patch is a bit more drastic, but will avoid all calls into the Sync service.  Not sure if there is a reason the check is the way it was, but hoping mbrubeck can enlighten me.
Attachment #517600 - Flags: review?(mbrubeck)
Comment on attachment 517600 [details] [diff] [review]
Alternative Patch

I was trying to match the check in WeaveGlue.observe.  Looks like this is more correct, which is fine.
Attachment #517600 - Flags: review?(mbrubeck) → review+
Whiteboard: [has patch][has review][needs approval] → [has patch][has review][has approval][can land]
pushed: http://hg.mozilla.org/mobile-browser/rev/46df9b7faa51
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Verified fixed
Mozilla/5.0 (Android; Linux armv7l; rv:2.0b13pre) Gecko/20110316 Firefox/4.0b13pre Fennec/4.0b6pre
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.