The default bug view has changed. See this FAQ.

Kick off a sync immediately after setting up Sync on Fennec

VERIFIED FIXED in Firefox 9

Status

Fennec Graveyard
General
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: philikon, Assigned: philikon)

Tracking

({verified-aurora, verified-beta})

Trunk
Firefox 9
verified-aurora, verified-beta

Details

(Whiteboard: [verified in services])

Attachments

(1 attachment, 1 obsolete attachment)

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.
Created attachment 560054 [details] [diff] [review]
v1

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.
status-firefox7: --- → affected
status-firefox8: --- → affected
Comment on attachment 560054 [details] [diff] [review]
v1

>-    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);

Weave.Service.sync()

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
Created attachment 561838 [details] [diff] [review]
v2

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.
status-firefox9: --- → affected
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.
https://hg.mozilla.org/services/services-central/rev/95bee1fc8117

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).
Status: NEW → ASSIGNED
Whiteboard: [fixed in services]
https://hg.mozilla.org/integration/mozilla-inbound/rev/9561bb68864b
Whiteboard: [fixed in services] → [fixed in services][inbound]
https://hg.mozilla.org/mozilla-central/rev/9561bb68864b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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
Status: RESOLVED → VERIFIED
status-firefox9: affected → fixed
Comment on attachment 561838 [details] [diff] [review]
v2

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):

https://hg.mozilla.org/releases/mozilla-beta/rev/be7de1a5bbf1
status-firefox7: affected → wontfix
status-firefox8: affected → fixed
Verified

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
Keywords: verified-aurora, verified-beta
You need to log in before you can comment on or make changes to this bug.