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

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

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

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch v1

Pretty conservative patch. Don't want to accidentally leave sync in a strange state.

Comment 2

[Martijn Wargers (dead)]

Wesley, did you want to ask for review for the patch?

Comment 3

[Wesley Johnston (:wesj)]

Really simple, low risk polish patch. Should be fine for 4.0. Asking for approval

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Alternative Patch

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.

Comment 5

[Matt Brubeck (: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.

Comment 6

[Wesley Johnston (:wesj)]

pushed: http://hg.mozilla.org/mobile-browser/rev/46df9b7faa51

Comment 7

[Aaron Train [:aaronmt]]

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