40316 - No error when migrating profile having no user files, can crash.

Status: VERIFIED FIXED

(MailNews Core :: Profile Migration, defect, P2)

Description

[laurel]

Using 2000-05-23-09m1 commercial build on NT 4.0
Haven't yet tried this on linux or mac...will report back

Not sure if this is really Seth's or the profile manager/selection dialog
people.

After removing 4.x user files from disk then doing the following steps to
migrate that profile then another, a crash consistently occurs.  We should
instead display an error that the profile's files cannot be located and stop
gracefully.  This problem should not pass through to the next profile to be
migrated.

Steps:

1.  4.x has several profiles.
2.  Through NT/Win32 file manager (NOT through the Communicator 4.x profile
manager) remove the user directory for a profile.
3.  Run seamonkey using the -installer argument.
4.  When seamonkey profile picker appears, select the profile which now has no
user director.  Click Start Netscape which will launch the migration
confirmation dialog, click OK to begin migration.
    Result #1:  No error dialog appears about missing files.  User is returned
to the profile picker.
5.  Still in the profile picker, select another profile to migrate (one that
DOES have its user files intact). Click Start Netscape which will launch the
migration confirmation dialog, click OK to begin migration.
    Result #2:  Crash occurs.

Result:  No error is displayed to user about the profile's missing user files
when migration attempted.  If you choose another profile which has all files
present, you crash.

Comment 1

[laurel]

Happens on Mac (OS 9.0), too. Will attach macsbug report.

Can't happen on linux since we can't have multiple profiles for migration?

Comment 2

[laurel]

Attachment: macsbug stdloginto file for migration crash

Comment 3

[selmer (gone)]

Target nominated bugs to M16.  Please update as necessary.

Comment 4

[(not reading, please use [e-mail] instead)]

moving to m17, since it is not a m16 blocker.

Comment 5

[scottputterman]

moving to future milestone.

Comment 6

[(not reading, please use [e-mail] instead)]

accepting all "future" bugs to get them off my "new" bug radar.

Comment 7

[(not reading, please use [e-mail] instead)]

migration bugs to racham

Comment 8

[scottputterman]

nominating for nsbeta1.  If we want to improve migration, this would be worth
looking into.

Comment 9

[scottputterman]

marking nsbeta1+ and moving to mozilla0.9

Comment 10

[racham]

I remember Don Bragg fixing this problem.

Please try to reproduce this with the latest build and update the bug with results.

thanks
bhuvan

Comment 11

[racham]

If you can't reproduce this, then it is a dup of 47097.

Comment 12

[racham]

Added code to check the existence of the 4.x profile directory and throw an
alert informing the user that the source profile directory does not exists and
that particular profile can't be migrated. User is back on profile picker and
can migrate another 4.x profile (whose directory exists) successfully without
crashing... 

Patch coming up..

Comment 13

[racham]

Attachment: patch

Comment 14

[racham]

I had to add some code int he profielmanager component itself. Adding Conrad and
Seth for reviews.

Please do let me know, if you feel that the alert message need to be more
informative. Thanks.

Comment 15

[Conrad Carlen (not reading bugmail)]

Bhuvan, I like it and the error msg is fine. One thing though. I'm not sure
about profileDirExists(). Internally the impl calls nsProfile::GetProfileDir().
The problem with GetProfileDir() is that, in the case of a current profile, it
will create the directory if it doesn't exist so it will always return TRUE. It
will work for this case since we're talking about unmigrated profiles but I
don't think we should rely on this hidden behavior. I've never liked this -
"getters" should just "get". If profileDirExists() is put on the public API,
people may use it and it will not do what they expect. See bug 62418. I have a
patch there for stripping down GetProfileDir() to what it really should do.
Bhuvan, you reviewed that but it stalled waiting for sr from Ben which never
happened :-/ The two problems are closely related - we could combine some things. 

Comment 16

[racham]

Conrad, I agree with you totally. Though we call this (profileDirExists) on only
migrated cases, there is a good chance that others can use as general purpose
routine and end doing something they don't want. As you suggested, it will
certainly better to combine these two. It has been a while since I reviewed
62418. We shall get that in...it is also an important improvement for
profilemanager in terms of error handling. Adding dependency for tracking purposes.

For bug 62418, send a mail to Ben about the review again.

Comment 17

[racham]

Seth,

If you have any cycles, can you super-review bug 62418 for Conrad, so that we
can knock these 2 bugs down. I will send you a mail as reminder.thanks.

bhuvan

Comment 18

[scottputterman]

any luck on this one?

Comment 19

[(not reading, please use [e-mail] instead)]

just so I understand:

when you do "profileDirExists()", it checks to see if the 4.x profile 
directory exists.  that works, because before we migrate, the profile directory 
associated with a profile is the 4.x profile.

if so, sr=sspitzer

before (or after) you land this, you should run your alert text by robinf or 
someone on the docs / ui team.

Comment 20

[racham]

Attachment: patch based on Conrad's fix for bug 62418

Comment 21

[racham]

Conrad's patch in bug 62418 will do more good things for profilemanager. Patch 
based that code (posted in previous update) is simpler and cleaner. I will talk 
to Robin about the alert text.

Comment 22

[scottputterman]

moving to mozilla0.9.1

Comment 23

[robinf]

Bhuvan, for the missing profile directory, here's what I'd suggest for the error
message text: "This profile cannot be migrated because the directory containing
the profile could not be found. Choose another profile or create a new one."

Comment 24

[racham]

Attachment: updated with new error message

Comment 25

[Conrad Carlen (not reading bugmail)]

+            promptService.alert(window, profileDirMissingTitle, errorMessage);
+              return;          

When, it returns here, shouldn't it return false?

With that (or if that's OK), r=ccarlen

Comment 26

[racham]

will return false. that's better. thanks.

Comment 27

[racham]

Fix checked in.

Comment 28

[laurel]

OK using 2001-05-09-04 commercial trunk build win98.
OK using 2001-05-08-08 commercial trunk build mac OS 9.0.

Alert displays with the text robinf suggested.