403463 - ImportNetscapeCookies can use new nsICookieManager2 API

Status: RESOLVED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[[e-mail]]

Bug 397238 exposed the code that the cookie manager uses to migrate a cookies.txt file to cookies.sqlite for profile migration to hook into.

Comment 1

[[e-mail]]

i'm confused as to the logic here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/suite/profile/migration/src/nsSeamonkeyProfileMigrator.cpp&rev=1.21#103

|aReplace| is true if |nsIProfileStartup* aStartup| is non-null, which i'm guessing means the profile isn't started yet. if this is the case, then we can't call up the cookieservice to import things. is this correct, or does |aReplace| really truly mean the user wants to nuke any existing data and import afresh?

Comment 2

[Mark Banner (:standard8)]

(In reply to comment #1)
> i'm confused as to the logic here:
> http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/suite/profile/migration/src/nsSeamonkeyProfileMigrator.cpp&rev=1.21#103
> |aReplace| is true if |nsIProfileStartup* aStartup| is non-null, which i'm
> guessing means the profile isn't started yet. if this is the case, then we
> can't call up the cookieservice to import things. is this correct, or does
> |aReplace| really truly mean the user wants to nuke any existing data and
> import afresh?

If I remember correctly, aReplace is true if we're starting up. So the aStartup profile is the new profile (or -migration has been specified), and I believe its generally assumed that aReplace nukes the exsiting data.

I tried the new function out the other day, but it didn't work, unfortunately I haven't had time to debug it yet to work out if its sending things to the wrong places or not...

Comment 3

[[e-mail]]

ok, great... if we have a profile then things should be happy. i'm still not clear from your comment though if aReplace is *intended* to nuke data. in this case, that'll involve specifically saying "remove all cookies first".

patch coming up that does just that.

Comment 4

[[e-mail]]

Attachment: something like this

yay codesize win!

Comment 5

[[e-mail]]

Comment on attachment 288902 [details] [diff] [review]
something like this

>-  if (NS_FAILED(rv))
>-    return rv;
dwitte: note file style ;-)

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 288902 [details] [diff] [review]
something like this

As discussed with dwitte on irc, this patch doesn't work in the startup case as the cookie manager can't get the NS_GetSpecialDirectory as that hasn't been created at migration time.

Comment 7

[[e-mail]]

Attachment: patch v2

this reverts to file delete/copy operations if aReplace is true, and regular import if it's false.

Comment 8

[[e-mail]]

oh, forgot to change that |if (NS_FAILED...| part again, will do on checkin. ;)

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 289449 [details] [diff] [review]
patch v2

r=me with the +  if (NS_FAILED(rv)) return rv;
 fixed.

Comment 10

[[e-mail]]

Comment on attachment 289449 [details] [diff] [review]
patch v2

>+    PRBool exists = PR_FALSE;
>+    targetFile->Exists(&exists);
>+    if (exists)
>+      targetFile->Remove(PR_FALSE);
If you're ignoring the result of Remove then why do you care if it exists?

Comment 11

[[e-mail]]

(In reply to comment #10)
> If you're ignoring the result of Remove then why do you care if it exists?

good point... fixed and checked in.