423211 - Migration of SeaMonkey profile can fail when folder in source profile is missing

Status: RESOLVED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

On trunk/windows, importing bookmarks from a branch SeaMonkey does not seem to work, i.e. I get the default bookmarks.

I triggered the import by just running trunk SM for the first time, so I got asked to import.

File|Open bookmarks file seems to be a workaround.

Comment 1

[Andrew Schultz]

I was able to import bookmarks from a 1.1 profile into a trunk profile on Linux.  I also tried with no existing trunk profiles and that also worked.

Comment 2

[Marcus]

Bug 432664 is a duplicate of this one. I could reproduce it. From my Comment to 432664 #1 

Last working Build is Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.9b3pre) Gecko/2008010102 SeaMonkey/2.0a1pre

Next windows Build is broken: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.9b3pre) Gecko/2008010203 SeaMonkey/2.0a1pre

Additional Info:
I guess the patch for Bug 386337 broke this (the only patch related to profile migration between this builds).

There was also a confirmation for this bug from a linux user 
Mozilla/5.0 (X11; U; Linux i686; rv:1.9.0.1pre) Gecko/2008070101 Mnenhy/0.7.5.20005 SeaMonkey/2.0a1pre

One of the Bugs should be duped and blocking seamonkey2.0a1 + - it's no fun for ppl. to test without bookmarks.

Comment 3

[Mark Banner (:standard8)]

Marcus, in your source profile, do you have a "searchplugins" directory? If you don't, please could you try adding one and redoing the migration.

I bet http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/profile/migration/src/nsNetscapeProfileMigratorBase.cpp&rev=1.19&mark=648-653#640 is the problem (called from line 971) - if the source directory doesn't exist, then it returns a failure, that then causes CopyOtherData to abort, and in turn, causes other things not to happen because of the reported "failure".

Comment 5

[Peter Weilbacher]

(In reply to comment #3)
> I bet
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/profile/migration/src/nsNetscapeProfileMigratorBase.cpp&rev=1.19&mark=648-653#640
> is the problem (called from line 971) - if the source directory doesn't exist,
> then it returns a failure, that then causes CopyOtherData to abort, and in
> turn, causes other things not to happen because of the reported "failure".

Interesting, so possibly fixes are:
1. ignore the return code of RecursiveCopy() as called from CopyOtherData()
2. move the CopyBookmarks line up a bit in nsSeamonkeyProfileMigrator::Migrate
   http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/profile/migration/src/nsSeamonkeyProfileMigrator.cpp&rev=1.23&mark=107#102

I like option 2 better...

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> Interesting, so possibly fixes are:
> 1. ignore the return code of RecursiveCopy() as called from CopyOtherData()
> 2. move the CopyBookmarks line up a bit in nsSeamonkeyProfileMigrator::Migrate

Neither of which won't actually resolve the problem fully - in this case, RecursiveCopy is "incorrectly" returning a missing directory as a failure. We should either handle that specific case when we get the return code, or RecursiveCopy shouldn't return failure.

I haven't looked at where it is used to work out what we should do.

It'd still be nice if someone could actually test the theory as I haven't got time to at the moment.

Comment 7

[Steve Wendt]

In my Seamonkey profile directory, I have the following subdirectories:
  Cache
  chatzilla
  chrome
  Mail
  News

I added:
  searchplugins

I did a fresh profile migration with Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.0.2pre) Gecko/2008070613 SeaMonkey/2.0a1pre

Resulting profile has the following subdirectories:
  Cache
  chrome
  extensions
  Mail
  News

The bookmarks from the old profile are not present.  However, the Tools->Import in the bookmark manager found my old profile and successfully imported them.

Comment 8

[Marcus]

(In reply to comment #3)
> Marcus, in your source profile, do you have a "searchplugins" directory? If you
> don't, please could you try adding one and redoing the migration.

No, the directory is missing in the old profile. 

Test with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1a2pre) Gecko/2008073000 SeaMonkey/2.0a1pre:
Without the searchplugins directory the bookmarks are missing, working when i create the [empty] directory.

I think a missing directory / file in the source profile shouldn't fail - you can't migrate what's missing.

Comment 9

[lenochod]

Is Bug 452620 duplicate?

Comment 10

[Frank Wein [:mcsmurf]]

Attachment: Patch

Comment 11

[Karsten Düsterloh]

Comment on attachment 338211 [details] [diff] [review]
Patch

>+  if (NS_FAILED(rv))
>+    return rv;

That's usually (and should be) written as

NS_ENSURE_SUCCESS(rv, rv);

Comment 12

[Frank Wein [:mcsmurf]]

Attachment: Patch 2

Fixed a few occurrences of this right below the new code.

Comment 13

[Frank Wein [:mcsmurf]]

Comment on attachment 338282 [details] [diff] [review]
Patch 2

:)

Comment 14

[[e-mail]]

Comment on attachment 338282 [details] [diff] [review]
Patch 2

I've got nothing against if (NS_FAILED(rv)) return rv; although it depends on how unusual failure is.

Comment 15

[Frank Wein [:mcsmurf]]

Pushed to comm-central, changeset 104de1a05154.