Closed Bug 670082 Opened 9 years ago Closed 9 years ago

When setting Sync options for the first time, clicking Done does nothing [error: Weave.Engines.get("history") is undefined]


(Firefox :: Sync, defect)

Not set



Tracking Status
firefox7 --- fixed
firefox8 --- fixed
firefox9 --- fixed


(Reporter: dana, Assigned: rnewman)



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


(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110615151330

Steps to reproduce:

In trying to reproduce an unrelated Sync bug, i have had need to re-create my Firefox profile and enable/disable Sync many times. In doing so, i have noticed that frequently when i try to configure Sync for the first time, clicking 'Done' at the Sync Options screen does nothing.

This is the procedure i tend to use:

1. Start up Firefox, go to Firefox > Set Up Sync...

2. Click Connect ('I already have a Firefox Sync account.')

3. Click 'Sync Options' at the lower left

4. Enter computer name, untick Preferences and Tabs, then tick 'Replace all data on this computer with my Sync data' (i reckon this last part may be important)

5. Click Done

Actual results:

Nothing. I keep clicking the button but nothing apparent happens. I end up having to close the window (via the X button at the top) and then re-open it. Usually when i go back and do it the second time, it will work. Why that is, i'm not sure.

It occurred to me to check the Error Console when this happens, and this is what i find:

Weave.Engines.get("history") is undefined
Line: 824

This error appears every time i click 'Done'.

If i had to guess i would say that it is probably trying to obtain my browser history so that it can tell me how many days of history the 'Replace all data on this computer' option is going to overwrite. Since this usually (although not always!) happens when i'm setting up a new Firefox profile, there are probably no history items to obtain.

Expected results:

It should progress straight to the 'Warning: The following Firefox data on this computer will be deleted' screen.
Weird, I can't reproduce this at all. Do you have history sync enabled?
Well, that's the goal, yes, but the error occurs before any of that stuff is turned on. Usually if i go back and do it a second time i'm able to perform the same steps and it will work -- at that point i do synch history in addition to bookmarks and passwords. (Sorry if i've misinterpreted the question)

What i've been doing for the most part is creating a new profile and then going straight to Set Up Sync, and since i always want Tabs and Preferences disabled, and i always want it to overwrite the handful of bookmarks that come with FF, i untick/tick those options (respectively) before i turn Sync on. That's when the error occurs.
I've actually just installed the FF6 beta on my work computer (different machine and OS from the one i'd been using before) and have been able to reproduce this problem on there as well -- see attached screenshot. As before, this is with a new profile (thus the need to set up Sync again, of course).

In this particular case i had visited a handful of sites before trying to set up Sync, so perhaps my guess about Firefox not being able to retrieve my history because it is non-existent is not accurate.
One more update on this for completeness's sake: I've been able to reproduce this on Aurora as well. :)
Keywords: qawanted
This WFM on mozilla central, beta and release.
Keywords: qawanted
kine: is there anything else in the error console?

Can you still reproduce this on latest Aurora or Nightly?
I'm afraid so. Attached: Screenshot on Aurora 8.0a2.

Exact steps i took to reproduce:

1. Applied Aurora update on my normal profile (i was two updates behind), then restarted.

2. Closed Aurora when it came back up.

3. Ran Aurora with Profile Manager, created a new profile, and launched with it.

4. After it started, i went immediately to Aurora > Set Up Sync.

5. I selected 'I don't have the device with me'.

6. I was prompted to set a new master password. I guess this is a recent feature, because i've never done it before. Anyway, i set the password and clicked OK.

7. On the next screen (where i was asked to enter the device code or whatever), i clicked Sync Options in the corner.

8. I unticked Preferences and Tabs and ticked 'Replace all data on this computer' (see screenshot).

9. I clicked Done. Nothing happened.

10. I clicked Done again. Nothing happened.

11. I went back to the main Aurora window, opened the Error Console from the Aurora menu, and noted two Weave errors at the bottom.

12. I returned to the Sync Options window and clicked Done again. An additional error was added to the Console (see screenshot).
Whoa, I was just able to reproduce once on aurora.  but not reliably.   However, I do see the same error and warning in the error console that have line: 260 and line 1348 in them from your screenshot.

by the way, the Master Password issue you noted in step 6 is bug 653335.  Just close it with OK, then OK to the alert, instead of giving it a password.
Ever confirmed: true
I just reproduced on a Nightly build on Mac. Investigating.
Ever confirmed: false
I also managed to get this on exiting the wizard:

* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "Weave is not defined" {file: "chrome://browser/content/syncSetup.js" line: 621}]' when calling method: [nsIStreamListener::onStopRequest]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "<unknown>"  data: yes]
My suspicion is that this path through the wizard doesn't involve accessing Weave.Service. That means service.js isn't lazily loaded, so it doesn't invoke _registerEngines, and so Engines is empty.

A slightly hacky fix might be to just touch Weave.Service before reaching this code, ensuring that everything necessary has been loaded. Hacky is probably appropriate here, given that we want to alter both the wizard and the engines code...

More after I run out for an hour or two.
Assignee: nobody → rnewman
Ever confirmed: true
(at the dentist replying from Fennec lol)

i approve of this plan.

the network error is very much unrelated, btw. we make network calls to check username validity and if they fail or are aborted they unfortunately still log an exception. i believe restrequest may fix this. which reminds me that we should use it in the new wizard.
Attached patch Proposed patch. v1 (obsolete) — Splinter Review
This minimal patch allows me to proceed through the wizard.

It looks like there might be a gap in our QA coverage here: this code hasn't changed in a while, and so it's likely been broken for a while.

QA chaps: when you verify this, would you be so kind as to go through permutations of setup options and each leg of the wizard? The STR in this bug should be a great starting point.

/me resolves to brush up on mozmill...
Attachment #557928 - Flags: review?(philipp)
Comment on attachment 557928 [details] [diff] [review]
Proposed patch. v1

>@@ -437,16 +437,21 @@ var gSyncSetup = {
>     if ((this.wizard.pageIndex >= 0) &&
>         !Weave.Utils.ensureMPUnlocked()) {
>       return false;
>     }
>     if (!this.wizard.pageIndex)
>       return true;
>+    // Parts of the wizard rely on access to engines via the Engines object,
>+    // which is populated by Service._registerEngines. Cause that to occur
>+    // early by lazy-loading Weave.Service. See Bug 670082.

Yuuup. Though I would rather not mention implementation details of Service like that (mentioning _registerEngines). The bug has enough details at this point.

The bigger nit, however, is: why do this every single time in gSyncSetup.onWizardAdvance() and not once in gSyncSetup.init()? r- for that.
Attachment #557928 - Flags: review?(philipp) → review-
Further to IRC discussion...
Attachment #557928 - Attachment is obsolete: true
Attachment #557953 - Flags: review?(philipp)
Comment on attachment 557953 [details] [diff] [review]
Proposed patch. v2

Would prefer window.setTimeout instead of just setTimeout (that's also the style used in the rest of the file.) r=me with that
Attachment #557953 - Flags: review?(philipp) → review+
Fixed in services:
Whiteboard: [fixed in services]
Comment on attachment 557953 [details] [diff] [review]
Proposed patch. v2

This problem is present in all existing released code (it's just a rarely used page in the wizard and wasn't part of our QA test suite, unfortunately). The fix is extremely low-risk and noninvasive.
Attachment #557953 - Flags: approval-mozilla-beta?
Attachment #557953 - Flags: approval-mozilla-aurora?
Comment on attachment 557953 [details] [diff] [review]
Proposed patch. v2

Approved for mozilla-beta and mozilla-aurora. Please land asap
Attachment #557953 - Flags: approval-mozilla-beta?
Attachment #557953 - Flags: approval-mozilla-beta+
Attachment #557953 - Flags: approval-mozilla-aurora?
Attachment #557953 - Flags: approval-mozilla-aurora+
Target Milestone: mozilla7 → mozilla9
verified on s-c from 20110907 and aurora from 20110908
Keywords: verified-aurora
Whiteboard: [fixed in services] → [verified in services]
Closed: 9 years ago
Resolution: --- → FIXED
Setting status9 as fixed given comment #22, and tracking for QA verification on the client.
Whiteboard: [verified in services] → [verified in services],[qa+]
> Setting status9 as fixed given comment #22, and tracking for QA verification 
> on the client.

I tested to check if this bug is verified since its status was set to resolved, on all platforms (Win, Linux and Mac), on three of the Firefox channels:

Windows - VERIFIED FIXED (steps in Description):
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0 (Beta)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a2) Gecko/20110918 Firefox/8.0a2 (Aurora)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110918 Firefox/9.0a1 (Nightly)

Mac - VERIFIED FIXED (steps in Description):
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0) Gecko/20100101 Firefox/7.0 (Beta)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:8.0a2) Gecko/20110919 Firefox/8.0a2 (Aurora)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0a1) Gecko/20110915 Firefox/9.0a1 (Nightly)

Linux - VERIFIED FIXED (steps in Description):
Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0 (Beta)
Mozilla/5.0 (X11; Linux i686; rv:8.0a2) Gecko/20110919 Firefox/8.0a2 (Aurora)
Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110919 Firefox/9.0a1 (Nightly)

Having tested accordingly, I will take the opportunity to set this bug's status to Verified. Thanks
Keywords: verified-beta
Whiteboard: [verified in services],[qa+] → [verified in services],[qa!]
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.