Closed Bug 602077 Opened 9 years ago Closed 9 years ago

WeaveGlue.init() shouldn't touch Weave.Service

Categories

(Firefox for Android Graveyard :: General, defect)

x86
macOS
defect
Not set

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: mconnor, Assigned: mfinkle)

References

Details

Attachments

(1 file)

http://mxr.mozilla.org/mobile-browser/source/chrome/content/sync.js#45

need to fix bug 590743 to make the first bit go away, and loadInputs needs to be called much more selectively, and not during init.
blocking2.0: --- → ?
loadInputs uses Weave.Service to load the username, password and sync key into the UI. Without that, the user assumes those fields were never entered.

I guess we could look at a different field or preference and just hide the UI if we know an auto-connect will happen. That way the user doesn't freak out thinking the data they previously entered is now lost.
Note bug 590763 is intending to move away from autoconnect entirely, and only call login if a sync is requested.  For Fennec, this means we should always appear to be connected unless there is an auth failure.

Solution:

a) hide the inputs by default
b) If Weave.Status.checkSetup() == Weave.CLIENT_NOT_CONFIGURED, show the inputs, but you won't have any useful data on Weave.Service in that case, so it should be blank. (this explicitly doesn't touch Weave.Service to avoid pulling in everything unless we definitely need it)
c) otherwise, the client has registered credentials, and we should assume those credentials are correct until there's a login failure.  If there's a login failure, we can fill in the inputs then.
blocking2.0: ? → ---
tracking-fennec: --- → ?
Assignee: mconnor → mark.finkle
tracking-fennec: ? → 2.0b2+
Attached patch patchSplinter Review
I talked to Madhava about this and we settled on: If the Sync credentials have been previously entered and the system is set to autoconnect, then show the prefs UI in a state of "connecting..."

Once we connect or fail to connect, the UI reverts to whatever is appropriate, based on existing code.

The patch does not use any Weave.Service or Weave.Str code in the WeaveGlue.init() code path. It does access the client name pref directly and the sync.properties file using Services.strings
Attachment #482872 - Flags: review?(mconnor)
Comment on attachment 482872 [details] [diff] [review]
patch

A good chunk of this will go away when we remove login/logout, but that timeline isn't 100% clear, so this is good for now.
Attachment #482872 - Flags: review?(mconnor) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/d4f1702e7903
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
How can I verify this ?
I've connected to Sync using valid credentials and then I've restarted the app. When the app has reopened, I've opened the Preference panel > Sync. As Mark Finkle said in comment 3#, Sync was trying to autoconnect and the UI was displaying "connecting..." instead of Connect button. I'll mark this bug as verified fixed.

--
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110912
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.