STR: 0. Add a master password to your Firefox profile, and install Weave. 1. Start up Firefox. Dismiss any initial Master-Password dialogs. 2. Choose "Connect" in weave status bar menu (or in Tools | Weave) 3. Cancel the resulting Master password dialog. ACTUAL RESULTS: A *second* master password dialog appears.... - If I dismiss it, nothing else weird happens. - If I enter my master password in it, Weave still doesn't actually connect -- I get "Error While Signing In / Weave encountered an error while signing you in: Unknown error. Please try again." EXPECTED RESULTS: No second master password dialog should appear, if I cancel the first one. VERSION INFO: * Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091123 Minefield/3.7a1pre * Weave 1.0b2 Weave 1.0b2pre is also affected -- I just tested a not-yet-upgraded machine with that version. I'm pretty sure this is a regression since 1.0b1, though. NOTE: This bug only affects manually-initiated "Connect" operations. It doesn't affect Weave's autoconnect attempt at Firefox startup.

(In reply to comment #0) > I'm pretty sure this is a regression since 1.0b1, though. Confirmed regression since 1.0b1. Weave 1.0b1 doesn't re-prompt for master password, when I choose "Connect" and then cancel the fist dialog. (Additionally: In Weave 1.0b1, when I cancel a "Connect" master password dialog, I get an "Unknown Error" Weave notification. In Weave 1.0b2 and 1.0b2pre, when I cancel both "Connect" master password dialogs, I get _no_ Weave error-notifications. I don't consider the lack-of-error-notification a regression in this case -- I'm just reporting it in case it's useful in tracking down what changed here.)

I think I know the problem here... In WeaveGlue.connect, we have: > connect: function connect() { > Weave.Service.login(this._settings.user.value, this._settings.pass.value, > this._settings.secret.value); http://hg.mozilla.org/labs/weave/file/a3a955e59f54/source/chrome/content/fennec-weave-overlay.js#l52 I'm pretty sure the use of "this._settings.pass.value" and "this._settings.secret.value" are both accesses to the password manager, and so both can trigger a master password dialog. Fix is simple: Just separate out those queries into different lines, and if the first one fails, then bail out. Now, why are we getting the "Unknown Error" notification *specifically* if I cancel the first dialog but complete the second? Well, if I cancel both dialogs, then Weave has neither my password nor my passphrase, so when we get "onLoginError", we hit this clause and skip the warning: > // Don't notify on missing passphrase errors > if (!Weave.Service.passphrase) > return; http://hg.mozilla.org/labs/weave/file/a3a955e59f54/source/chrome/content/sync.js#l136 But if I just cancel the first dialog & I complete the second, then my **passphrase** will be unlocked (since that's what the second dialog was asking for). So we'll bypass the early return above, and we'll go ahead with the warning. Anyway, I think we can avoid both issues by bailing out early if the password-value-lookup fails in "connect()".

> In WeaveGlue.connect, we have: > > connect: function connect() { > > Weave.Service.login(this._settings.user.value, this._settings.pass.value, > > this._settings.secret.value); > http://hg.mozilla.org/labs/weave/file/a3a955e59f54/source/chrome/content/fennec-weave-overlay.js#l52 So, per the naming of this file, this code is actually fennec-specific (Mardak confirmed in IRC). So this isn't the right place to patch, but there's presumably some corresponding spot in the desktop-firefox-login code...

Just kidding -- so, the second dialog is actually spawned **by the "// Don't notify on missing passphrase errors" check itself. We shouldn't be touching |Weave.Service.passphrase| there (in onLoginError), because that can trigger a master password dialog. (Or, we should only touch it if we've verified that password DB is unlocked.) If I comment out that check entirely, we revert to the (better IMHO) Weave 1.0b1 behavior that I described in comment 1: - No second master password dialog - "Unknown Error" notification if I cancel the first dialog. (The "Unknown Error" is still minorly annoying, but not as bad as the re-prompting for master password :) )

Not critical for 1.3

bug 590763 makes this a non-issue.