721242 - After using Firefox migrator, the password manager sometimes can't decrypt passwords

Status: VERIFIED FIXED

(Firefox :: Migration, defect)

Description

[Matthew N. [:MattN]]

My theory is that PSM? is initialized before the key3.db file is copied to the destination profile.  Password manager outputs "Initializing key3.db with default blank password."

Mano wants to remove the ability to call DoStartup in bug 718586 so simply delaying the startup like was traditionally done in these situations would block that.

Comment 1

[Matthew N. [:MattN]]

Attachment: v.1 delay the doStartup call

I personally think we should take this patch for now to avoid having broken password migration on Mozilla 12.   We can address the removal of the startup argument in bug 718586 after branching so that there's more time to find a workaround to this problem, if necessary.

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 591641 [details] [diff] [review]
v.1 delay the doStartup call

Review ping

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 591641 [details] [diff] [review]
v.1 delay the doStartup call

Review of attachment 591641 [details] [diff] [review]:
-----------------------------------------------------------------

It's ok, for the sake of having this working in the 12-13 releases, though please add a comment explaning the problem in bug 718586 (if you didn't yet), so that it is somehow tracked for when that bug will be addressed.

and sigh, we really need to move on with making some tests :( As it is this may break at any time.

::: browser/components/migration/src/FirefoxProfileMigrator.js
@@ +233,5 @@
>  
>      if (aItems & MIGRATOR.PASSWORDS)
>        this._migratePasswords();
>  
> +    // The password manager encryption key must be copied before startup

nit: end with period

Comment 4

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/18fda283b571

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/18fda283b571

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 591641 [details] [diff] [review]
v.1 delay the doStartup call

[Approval Request Comment]
Regression caused by (bug #): Not a regression, new code.
User impact if declined: password migration fails from an existing Firefox profile.  There is no UI to activate the feature in 12 but I will make an extension for SUMO to use.
Testing completed (on m-c, etc.): Tested on m-c for a few days.
Risk to taking this patch (and alternatives if risky): Low risk, trivial, tiny patch.
String changes made by this patch: None

Comment 7

[Alex Keybl [:akeybl]]

Comment on attachment 591641 [details] [diff] [review]
v.1 delay the doStartup call

[Triage Comment]
Low risk and fixes a regression in a new FF12 feature. Approving for Aurora.

Comment 8

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/17f98701eca2

Comment 9

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20120318 Firefox/13.0a2
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120319 Firefox/13.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20120318 Firefox/13.0a2