511797 - [SeaMonkey] mozilla-central builds do not compile after bug 380917 landing

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Comment 1

[Serge Gautherie (:sgautherie)]

Examples:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1250806845.1250814281.14046.gz
WINNT 5.2 comm-central-trunk build on 2009/08/20 15:20:45

e:/builds/slave/comm-central-trunk-win32/build/suite/profile/migration/src/nsThunderbirdProfileMigrator.cpp(42) : fatal error C1083: Cannot open include file: 'nsIPasswordManagerInternal.h': No such file or directory

e:/builds/slave/comm-central-trunk-win32/build/suite/profile/migration/src/nsNetscapeProfileMigratorBase.cpp(56) : fatal error C1083: Cannot open include file: 'nsIPasswordManagerInternal.h': No such file or directory
}

Comment 2

[Justin Dolske [:Dolske]]

This code is broken and should be removed.

Comment 3

[Serge Gautherie (:sgautherie)]

Irc, asked Justin to back out in the meantime ... but he doesn't feel like it :-|

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Bustage fix [Backout: Comment 8]

Pushed this, should fix the bustage.

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 395740 [details] [diff] [review]
Bustage fix
[Backout: Comment 8]

(In reply to comment #4)

http://hg.mozilla.org/comm-central/rev/02df186e049a

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)

I had look at it too, but feared that would not be so simple.

Indeed:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1250817201.1250818422.27619.gz
Linux x86-64 comm-central-trunk build on 2009/08/20 18:13:21

/builds/slave/comm-central-trunk-linux64/build/suite/build/nsSuiteModule.cpp:69: undefined reference to `nsThunderbirdProfileMigrator::nsThunderbirdProfileMigrator()'
}

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)

Also
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1250816148.1250818180.24743.gz
Linux comm-central-trunk leak test build on 2009/08/20 17:55:48

Lots of errors, like:
../profile/migration/src/libsuitemigration_s.a(nsSeamonkeyProfileMigrator.o):(.data.rel+0x18fc): undefined reference to `nsNetscapeProfileMigratorBase::SetBool(nsNetscapeProfileMigratorBase::PrefTransform*, nsIPrefBranch*)'
}

Comment 8

[Justin Dolske [:Dolske]]

Attachment: Bustage fix v.2 [Checkin: Comment 8]

Bah. This reverts the last "fix", removes the #include for the deleted interface, and ifdefs out the code that wasn't working anyway.

If someone wants to make nsNetscapeProfileMigratorBase.cpp actually import passwords, there will need to be a followup to make it do so.

Pushed in http://hg.mozilla.org/comm-central/rev/b748a1b91122

Comment 9

[Justin Dolske [:Dolske]]

(This compiles fine locally, btw)

Comment 10

[Mark Banner (:standard8)]

So this needs a follow up.

In the SeaMonkey profile migrator case, I think we just need to copy the relevant password file into the profile directory and ensure we're copying the signon.SignonFileName pref - the toolkit profile manager will do the rest on startup.

For Thunderbird, we should:

1) See if signons.sqlite exists, if so copy that and make sure we copy the key3.db.
2) Else, check signon.SignonFileName3 copy the file if it exists and ensure pref set.
3) Else, check signon.SignonFileName2 copy the file if it exists and ensure pref set.
4) Else, check signon.SignonFileName copy the file if it exists and ensure pref set.

Comment 11

[Mark Banner (:standard8)]

So I should point out that the case that has been removed I think is for when you're effectively importing passwords into an existing profile (aReplace is false).

I believe we don't actually hit that case with our current code. However, we should check that (and probably remove the old code/add a comment) and we need to do what I said in comment 10 to fix the Thunderbird profile migrator.

Comment 12

[[e-mail]]

(In reply to comment #8)
> If someone wants to make nsNetscapeProfileMigratorBase.cpp actually import
> passwords, there will need to be a followup to make it do so.
<dolske> you should be able to create an instance of @mozilla.org/login-manager/storage/legacy;1 and use initWithFile().
<dolske> though then you'll have to iterate over the logins it imports, and addLogin() each to the normal login manager instance.

Comment 13

[[e-mail]]

dolske, what's to stop us calling initWithFile on the sqlite storage service?

Comment 14

[Mark Banner (:standard8)]

This doesn't block Thunderbird 3. The effect is small, worst case passwords won't get migrated from SeaMonkey 2 the number of account passwords is likely to be small and they can always be entered by hand (and I doubt there will be many people migrating from SM 2 for a while).

Comment 15

[Justin Dolske [:Dolske]]

(In reply to comment #13)
> dolske, what's to stop us calling initWithFile on the sqlite storage service?

Oh, yeah, that should work too. I forgot we made that take a legacy format file as input.

Comment 16

[[e-mail]]

Attachment: Fix password import (untested)

Is it really as easy is this?

Comment 17

[Mark Banner (:standard8)]

Comment on attachment 396068 [details] [diff] [review]
Fix password import (untested)

Should work in the SeaMonkey 1.x -> 2 case, it ignores the issue of different files in the Thunderbird 3 -> SM case.

Comment 18

[Justin Dolske [:Dolske]]

Comment on attachment 396068 [details] [diff] [review]
Fix password import (untested)

Looks right.

Comment 19

[Serge Gautherie (:sgautherie)]

Is there a way to create tests for this migration feature?
Is there already some, somewhere?

Comment 20

[Justin Dolske [:Dolske]]

There are currently no migration tests (at least in Firefox land). The pwmgr import API is fairly well tested, though not specifically from Wallet sources.

Comment 21

[[e-mail]]

Comment on attachment 396068 [details] [diff] [review]
Fix password import (untested)

Pushed changeset 45cd7a8e23e5 to comm-central.

Comment 22

[[e-mail]]

I'll resolve this as fixed and new bugs can be filed for any remaining issues.