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

RESOLVED FIXED

Status

()

Toolkit
Password Manager
--
blocker
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: sgautherie, Assigned: neil@parkwaycc.co.uk)

Tracking

({dogfood, regression})

Trunk
dogfood, regression
Points:
---
Bug Flags:
blocking-thunderbird3 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Comment 1

9 years ago
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
}
Summary: comm-central + mozilla-central builds do not compile after bug 380917 landing → [SeaMonkey] mozilla-central builds do not compile after bug 380917 landing
This code is broken and should be removed.
(Reporter)

Updated

9 years ago
Keywords: dogfood
(Reporter)

Comment 3

9 years ago
Irc, asked Justin to back out in the meantime ... but he doesn't feel like it :-|
Created attachment 395740 [details] [diff] [review]
Bustage fix
[Backout: Comment 8]

Pushed this, should fix the bustage.
Attachment #395740 - Attachment is patch: true
Attachment #395740 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Comment 5

9 years ago
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
Attachment #395740 - Attachment description: Bustage fix → Bustage fix [Checkin: Comment 4]
(Reporter)

Comment 6

9 years ago
(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()'
}
(Reporter)

Comment 7

9 years ago
(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*)'
}
Created attachment 395758 [details] [diff] [review]
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
Attachment #395740 - Attachment is obsolete: true
(This compiles fine locally, btw)
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.
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.
(Assignee)

Comment 12

9 years ago
(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.
(Assignee)

Comment 13

9 years ago
dolske, what's to stop us calling initWithFile on the sqlite storage service?
(Reporter)

Updated

9 years ago
Attachment #395758 - Attachment description: Bustage fix v.2 → Bustage fix v.2 [Checkin: Comment 8]
(Reporter)

Updated

9 years ago
Attachment #395740 - Attachment description: Bustage fix [Checkin: Comment 4] → Bustage fix [Backout: Comment 8]
(Reporter)

Updated

9 years ago
Flags: blocking-thunderbird3?
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).
Flags: blocking-thunderbird3? → blocking-thunderbird3-
(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.
(Assignee)

Comment 16

9 years ago
Created attachment 396068 [details] [diff] [review]
Fix password import (untested)

Is it really as easy is this?
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #396068 - Flags: review?(dolske)
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 on attachment 396068 [details] [diff] [review]
Fix password import (untested)

Looks right.
Attachment #396068 - Flags: review?(dolske) → review+
(Reporter)

Comment 19

9 years ago
Is there a way to create tests for this migration feature?
Is there already some, somewhere?
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.
(Assignee)

Comment 21

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

Pushed changeset 45cd7a8e23e5 to comm-central.
(Assignee)

Comment 22

9 years ago
I'll resolve this as fixed and new bugs can be filed for any remaining issues.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.