Closed Bug 686579 Opened 11 years ago Closed 11 years ago

Kick off a sync immediately after setting up Sync on Fennec


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox7 wontfix, firefox8 fixed, firefox9 fixed)

Firefox 9
Tracking Status
firefox7 --- wontfix
firefox8 --- fixed
firefox9 --- fixed


(Reporter: philikon, Assigned: philikon)


(Keywords: verified-aurora, verified-beta, Whiteboard: [verified in services])


(1 file, 1 obsolete file)

OMG I just found out that WeaveGlue only calls Weave.Service.login(), but it doesn't kick off a sync. It really should. With Instant Sync, we may even not kick off a Sync at all until our idle status changes.

Also, we shouldn't use Service.login() at all. "Logging in" is no longer a UI-facing feature and we want to deprecate this API. We should just set username + password + Sync Key on the Service object and then kick off a sync.
Attached patch v1 (obsolete) — Splinter Review
Not tested yet, but this is pretty much what we should be doing (and what the desktop already does).
Attachment #560054 - Flags: review?(mark.finkle)
With the current situation we would never sync immediately, but I think with Instant Sync (new in Firefox 7) we may actually not start syncing until something triggers a sync interval change. Marking Firefox 7 and 8 as affected accordingly.
Comment on attachment 560054 [details] [diff] [review]

>-    Weave.Service.login(Weave.Service.username, this.setupData.password, this.setupData.synckey);
>+    Weave.Service.password = this.setupData.password;
>+    Weave.Service.passphrase = this.setupData.synckey;
>     Weave.Service.persistLogin();
>+    setTimeout(function () { Weave.Serivce.sync(); }, 0);


r- for this fix and I want a mobile dev to take this off your hands and do some testing. Thanks for filing and getting the patch started.
Attachment #560054 - Flags: review?(mark.finkle) → review-
Philipp - re-assigning to Wes so he can make the tweak and do some testing. Feel free to grab it back if you have time to wrap it up.
Assignee: philipp → wjohnston
Wes, any progress on this? I'll be happy to fix the patch and do the testing, too. This is a pretty annoying bug exacerbated by Instant Sync. We should fix it asap.
(In reply to Philipp von Weitershausen [:philikon] from comment #5)
> Wes, any progress on this? I'll be happy to fix the patch and do the
> testing, too. This is a pretty annoying bug exacerbated by Instant Sync. We
> should fix it asap.

Wes got loaded with some other stuff, so if you want to wrap it up, go ahead.
Gotcha. Thanks.
Assignee: wjohnston → philipp
Attached patch v2Splinter Review
Fixed typo in v1 and actually tested this patch on a local Fennec build. It works!

Also send the 'weave:service:setup-complete' notification. Weave.Service.login() sends that notification (we did that to retrofit the Fennec UI's behaviour), but since we're no longer using Weave.Service.login() thanks to this patch, we should send it ourselves.

There are some deeper running issues in this code. I really don't understand the Enable Sync checkbox (bug 669199). It still used the no-longer used loggedIn flag (bug 688574). And we probably also want to audit its use of error messages and notifications. I will follow up with more bugs.
Attachment #560054 - Attachment is obsolete: true
Attachment #561838 - Flags: review?(mark.finkle)
Also marking Firefox 9 as affected since it's like that by the time this lands, Firefox 9 will probably be in Aurora.
Attachment #561838 - Flags: review?(mark.finkle) → review+
I also want to get bug 669199 in fx9 too. Lucas is working on a patch there. CC'ing Lucas so he is aware of this patch.

The next Services QA train will probably not make the Aurora cut-off, but I'm
hopefull we can convince release drivers to allow this to go into Aurora (9)
and Beta (8).
Whiteboard: [fixed in services]
Whiteboard: [fixed in services] → [fixed in services][inbound]
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][inbound]
Target Milestone: --- → Firefox 9
Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110925 Firefox/9.0a1 Fennec/9.0a1
Comment on attachment 561838 [details] [diff] [review]

Asking for Aurora and Beta approval as this severely undermines the first setup UX on mobile right now. As a bonus it also fixes an error in the error console. Patch was verified by QA.
Attachment #561838 - Flags: approval-mozilla-beta?
Attachment #561838 - Flags: approval-mozilla-aurora?
verified on fennec s-c
Whiteboard: [verified in services]
Attachment #561838 - Flags: approval-mozilla-beta?
Attachment #561838 - Flags: approval-mozilla-beta+
Attachment #561838 - Flags: approval-mozilla-aurora?
Attachment #561838 - Flags: approval-mozilla-aurora+
Patch was landed before the Aurora cut-off, so not landing in Aurora (Firefox 9). Marking Firefox 7 as wontfix since it's released. Landed for Beta (Firefox 8):

Mozilla/5.0 (Android; Linux armv7l; rv:8.0) Gecko/20111006 Firefox/8.0 Fennec/8.0 ID:20111006003041

Mozilla/5.0 (Android; Linux armv7l; rv:9.0a2) Gecko/20111010 Firefox/9.0a2 Fennec/9.0a2 ID:20111010042022
You need to log in before you can comment on or make changes to this bug.