137886 - add support for re-migration of defunct previously migrated profiles

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect, P1)

Description

[selmer (gone)]

The 6.0 release got a lot of people to try the product and then they decided
they didn't like it.  We're starting to get more people coming back to try the
product again and these folks are having problems because the old 6.0 profile
doesn't have all their recent mail etc.  We need to detect these defunct
profiles and "make them go away" in the majority of cases.

(I thought there was a bug like this already, but I couldn't find it after
*much* searching.)

IIRC, we had said that the 6.0 profiles would be unique in some characteristics
like being in the users50 directory and not having a .slt directory and maybe
some other characteristics.  Adding the time element, if such a profile hasn't
been used in over a year I'd suggest removing it from the profile list (leaving
the files in place) and either creating a new profile or migrating over again. 
If any version since 6.1 has been installed it's probably too late to do the
right thing, but perhaps it's worth throwing up a dialog if the profile hasn't
been used for more than 3 months.

Comment 1

[selmer (gone)]

Adding Bhuvan for historical perspective, Ben for any related FE changes, Dan
for installer tie-ins, Kevin for marketing involvement and Scott for ADT
driving.  Since this is such a critical bug, I'm being presumptuous in adding
nsbeta1+ and [adt1] to it.

Everyone, please add any information we already have in this bug and any ideas
on how to proceed.  (Does an older bug actually exist somewhere?)

Dan, it's possible the user experience is improved by solving this in the
installer.  What does it take to do that?  Would we require the entire registry
library?  Is it easier/faster/better to implement an install solution than a
profile manager solution?

We need to move fast on this!

Comment 2

[selmer (gone)]

Presumably, Mozilla cares about this same issue WRT the milestones corresponding
to the 6.0 timeframe.  The fix for this should work equally for both products.

Comment 3

[selmer (gone)]

Adding Lori to get UE input going.  We need a clean way to handle this in cases
where user interaction is required.

Comment 4

[Conrad Carlen (not reading bugmail)]

I verified that the app writes out the prefs file on quitting and it's written
whether any change has happened or not.So, the mod date of prefs.js is
essentially the mod date of the profile.

I think the following would work:
Determine the "mod date" of the chosen profile.
If not used in 3 months
  See if there is a 4.x profile of the same name
  If the 4.x mod date is more recent (I need historical help here on how to know)
    Ask the user if they want to re-migrate it

Comment 5

[Conrad Carlen (not reading bugmail)]

Most of this should be implemented here:
http://lxr.mozilla.org/mozilla/source/xpfe/appshell/src/nsAppShellService.cpp#232
This behavior should not be in the profile mgr backend because that is used by
embedding apps as well and this behavior is a mozilla/ns application behavior.
We shouldn't put such code into a component which is used by embedding apps
which have no use for it.

  The profile mgr back end can have a new method GetProfileLastModifiedTime()
because that's generic enough. Given that, here's how I think it should work:

(1) nsAppShellService::DoProfileStartup() starts up the profile mgr.
(2) If a profile is chosen - it will be unless we're doing initial startup in
QuickLanuch mode with multiple profiles or the user canceled from the profile
picker dialog.
(3) Now that a profile is chosen, DoProfileStartup() checks the mod date of the
profile.
(4) If it is old, DoProfileStartup() uses Srilatha's code for getting a list of
4.x profiles, checks if the name of the chosen profile matches the name of a 4.x
profile and, if so, asks the user if they want to remigrate.
(5) If they confirm, existing profile mgr backend methods should be called to do
the work. If exactly what we need to do this isn't exposed, I'll do so.

(3) 

Comment 6

[Jimmy Lee]

Updating QA Contact.  Grace will lead the QA on this matter.

Comment 7

[kmurray1115]

Conrad - do you believe 3 months is a better number than 6 months for the last
modified date?

Comment 8

[Conrad Carlen (not reading bugmail)]

> Conrad - do you believe 3 months is a better number than 6 months for the last
modified date?

I used 3 months for example because that's the number Steve used. I'm not sure
how we determine the "stale" threshold. Whatever we decide for that value, it's
easily changeable.

Comment 9

[kmurray1115]

Unless Steve has objections, let's use 6 months. I'll consult a few more
marketing types and see if that number should change.

OT: how do you get quoting to work in bugzilla? I copy pasted from a comment,
and the quoting doesn't show up.

Comment 10

[selmer (gone)]

I think 6 months is the right threshold.  At 6 months, I'm not convinced we
actually need to prompt the user.  It's theoretically safer, but if there's an
unused 6.0 profile and a recently used 4.x profile of the same name then it's
pretty clear they gave up on 6.0 and won't even remember what's in that profile
anyway.  (Leaving the files around allows for recovery and a useful release note.)

What do others think about cleaning up without prompting?

Comment 11

[scottputterman]

Are we keying off of anything besides prefs.js?  I'd be concerned that we'd hit
6.1 and 6.2 users if we aren't.

Personally, I think it's dangerous to automatically do this without notifying
the user.  Six months means they haven't used it since December.  If they are
using 6.x through last December they are bound to have accumulated some things
they care about.

Comment 12

[selmer (gone)]

What about at 8 or 10 months?  In that range, they probably wouldn't have gotten
6.1 or higher.  It might be safer to check install versions, but I'm trying to
avoid that because it's a pain.  

My main point from the previous comment is that if it's been unused as long as 6
months the user isn't even going to recognize the contents.  Putting UI in front
of people and making them choose things isn't always in their best interests. 
Frequently, we can't describe a choice like this without confusing them and
leading to a worse result than just making the choice for them.  I worry that
any dialog we can come up with would just scare them away again muttering
"they're even telling me not to try it again!"  The other bad scenario would be
them calling for support because of the dialog.

Comment 13

[Conrad Carlen (not reading bugmail)]

> Are we keying off of anything besides prefs.js?  I'd be concerned that we'd hit
6.1 and 6.2 users if we aren't.

Looking at the code which writes out the prefs on quitting in CVS blame shows
that we have been doing that since Aug 2000. So, using the date of prefs.js is
pretty certain for this purpose.

I think we should ask the user. Because of salting and absolute path names,
backing up (leaving in place) the old profile data won't do much good as a
fallback since we don't offer a way for the user to retore it. See bug 22689 and
bug 17457.

Confusing alerts at startup are no fun, but as long as the text for this is
clear and obvious enough, I think it would be OK.

Comment 14

[kmurray1115]

If we don't show an alert in most cases, then that's fine. The conditions for
the alert to show up should be in accordance with the situation we're trying to
rectify in this bug only. Something like:

"You haven't used your <Netscape 6.0/Mozilla> profile since <xx/xx/xxxx.> Would
you like to use this profile (your profile contains bookmarks, email, address
book records and settings) for <Netscape X/Mozilla> or use the most recently
modified profile <(last modified on xx/xx/xxxx)> located in your <Communicator
4.79> directory?"

Where anything between <> is generated after we examine the modification
particulars of their profiles.

It gets ugly!

Comment 15

[Conrad Carlen (not reading bugmail)]

Status update: I have a patch close to completion. As of now, there is no prompt
but that can be added easily if we decide we need it.

Comment 16

[selmer (gone)]

Kevin, I think the dialog needs to mention that the old profile probably doesn't
have their most recent email and bookmarks.  That's the whole point of this
exercise after all.

Adding jglick since there's no UE input yet and I'm not the right person to be
making these responses.

Comment 17

[kmurray1115]

No question - I'm open to whatever. Jen, can you read the bug and give me a call
if you have questions about the problem we're trying to solve?  thx

Comment 18

[jglick]

Attachment: Proposal

The most current profile would be selected by default (for users who accept all
the defaults when installing). Profiles over X months old could be not
displayed at all? Title and buttons on this dialog would depend on how this is
implemented. Is it a separate dialog vs part of Install wizard.

Comment 19

[Conrad Carlen (not reading bugmail)]

Attachment: Patch to profile mgr

1. Adds a RemigrateProfile() method which can be used for this purpose.
2. Maintains info in the registry about
  (a) when a profile was created
  (b) when a profile was last used
  (c) when a profile was migrated, *where* it was migrated from. If we had this
info to begin with, so many problems (including, but not limited to this one)
would be so much easier.

I think this info needs to be added before another version goes out the door.
Since it's new data, low risk.

3. Various cleanup - There was code for reading profile locations which was
only used around M10-M11

Comment 20

[Conrad Carlen (not reading bugmail)]

Attachment: patch to appshell service

Comment 21

[jglick]

Attachment: #2

Removed the word "import" from dialog since it isn't correct for 6.x profiles.
Dialog would appear when user first attempts to use the product. If they pick
6.x, use that existing profile, if they pick 4.x, migrate data. If they pick
"Do not use settings..." let them start with a new profile.

Is this close to what you guys are thinking or am I completely off here? Do we
not want to give them the option for 4.x?

Comment 22

[kmurray1115]

That seems right, Jen. I'm not sure where we are though. 

Conrad, is your patch designed to act without user interaction? Is adding the
dialog and acting on it difficult to implement?

Comment 23

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

talked with ccarlend and jglick over aim.

jglick is working up a slightly different UI, for review.

In parallel, I'm going to apply / review ccarlen's patches, make it so we can
set the interval with a pref (for easier testing / development / last minute
changes) and add confirmEx code to throw up the prompt.

Comment 24

[selmer (gone)]

Jen, thanks for jumping in here!  I like the idea behind your dialog proposal. 
One thing to add: the choice the user is making is intrisically exclusive.  If
they pick 4.x, then the 6.x goes away.  If they pick 6.x, we won't ever
remigrate the 4.x.  The dialog should somehow convey the concepts of exclusivity
and permanence implied by this action.

Do we really want the "Do not use settings from a previous version checkbox"? 
The implementation of that would be to remove the 6.x profile, avoid any
remigration, and force a new profile.  Is that OK?

Kevin, Conrad is only on the hook to provide us the mechanical guts of the
implementation.  Someone in Apps will do all UI work for this.

Comment 25

[Lori Kaplan]

cc'ing mcarlson for localization impact of adding a dialog for beta.

Comment 26

[jglick]

Attachment: Updated Proposals

After chatting with Seth and Conrad, some updated proposals (in order of
preference :-)

Comment 27

[jglick]

Note: Users will only see this dialog if they select a profile in which the 
6.x profile and 4.x profile have the same name, And the 6.x profile has not been 
used for over 6 months. 

Comment 28

[kmurray1115]

If we can pull off #1, that would be great. But in the interest of time, I think
the clearest dialog is #3 because it conveys in a minimum number of words the
idea that recent profile info is good. The likely scenario is that the user will
simply hit Yes and we do the right thing. 

Comment 29

[selmer (gone)]

I agree with Kevin on #3.  I liked the wording about what's in a profile from
the previous dialog with the picker though.  We know users don't know what the
word profile means, so explaining that it's bookmarks and mail helps them get
the right answer.

Comment 30

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

Attachment: initial rough draft patch, starting with ccarlen's patches

differences:

1) pref to control interval (useful for dev and qa)
2) added some comments
3) re-arranged some logic, to prevent nested ifs
4) moved out code into private method

the UI right now is just hard codd. Once I move the strings out to a properties
file, I'll implement #3.

Comment 31

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

here's my todo list:

1)  find out what happens to the existing 6.x profile when we re-migrate
2)  verify that all the PRInt64 code is "mac friendly".  I think win32 allows
you to get away with a lot more than mac does, I think we need to use more of
the NSPR PRInt64 macros.
3)  move the UI strings out to a properties file
4)  test and build on all 3 platforms, and provide QA with the test plan that I
used.

Comment 32

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

Attachment: updated patch, strings moved to migration.properties

also, added some comments to all.js

Comment 33

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

Attachment: screen shot, from mozilla build

this is the screen shot of the dialog from a mozilla build.
for ns builds, the only difference would be the title, since both ns and
mozilla migrate from 4.5+.

Comment 34

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

Attachment: updated patch, use LL_IS_ZERO() and make it so if the pref is -1, profiles never go defunct, and we never see the UI (some vendors may not want this feature)

Comment 35

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

Attachment: ns tree diff (to set the interval to 6 months)

Comment 36

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

Attachment: updated mozilla patch

Comment 37

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

ok, I've tested the latest patch and it's working for me on win2k.  (haven't 
tested linux and mac yet.)

A brief summary of what happens on disk

my profile is sspitzer
so on disk I've got:

Application Data\Mozilla\Profiles\sspitzer\<foobar>.slt\...

After I remigrate, I'd have this on disk:

Application Data\Mozilla\Profiles\sspitzer\<foobar>.slt\... [remigrated files]
Application Data\Mozilla\Profiles\sspitzer\<foobar>.slt-old\... [old defunct 
files]

Comment 38

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

three concerns that I'll discuss with ccarlen tomorrow:

1)

+  // Now, we know that a matching 4.x profile exists
+  // See if it has any newer files in it than our defunct profile.
+  // XXX todo
+  // should we descend into child directories?

is it possible for the account to be used, but the top level files never change?
(probably not, but it's worth pointing out that using just mail might cause 
this.)

2)

the way the code is now, the user might re-migrate again.  They could decide we 
still stink, go back to 4.x and try again in another six months.  In which 
case, they'd already have a -old directory (or they might have one from a 
failed "remigration")

+    // XXX todo 
+    // what if foo-old exists?  we need to createunique or something
+    nsCAutoString newDirLeafName(origDirLeafName + NS_LITERAL_CSTRING("-old"));
+    rv = profileDir->MoveTo(nsnull, newDirLeafName.get());
+    if (NS_FAILED(rv))
+        return rv;

3)

I need to see if ccarlen saw this, as well.

+    // XXX todo
+    // on win2k [at least for me], after MoveTo(), empty folders are being 
left behind!
+    // without this call to Remove(), the call to Create() fails.  
+    rv = newProfileDir->Remove(PR_TRUE);
+    if (NS_FAILED(rv))
+        return rv;

after we resolve those issues:

1) build test on mac, linux
2) code reviews
3) provide QA with my initial test list
4) bake on the trunk

Comment 39

[scottputterman]

>the way the code is now, the user might re-migrate again.  They could decide we 
>still stink, go back to 4.x and try again in another six months.  In which 
>case, they'd already have a -old directory (or they might have one from a 
>failed "remigration") 

I figured we'd do this the first time the user used a profile, not every 6
months that they don't use it.  Maybe each release we can do this the first
time.  What do others think?

Comment 40

[Conrad Carlen (not reading bugmail)]

Comment on attachment 80540 [details] [diff] [review]
updated mozilla patch

>Index: xpfe/appshell/src/nsAppShellService.h
>===================================================================
>+#include "nsIProfileInternal.h"
> 
> class nsAppShellService : public nsIAppShellService,
>+private:
>+  nsresult CheckAndRemigrateDefunctProfile(nsIProfileInternal *aProfileMgr);
> };
>

I would just use a forward declaration of nsIProfileInternal here instead of
increasing the the number of files which now include nsIProfileInternal.h by
including this file. Also, passing it as a param is odd. It's a singleton
service and doing that just saves one getService() call. 


>+NS_IMETHODIMP 
>+nsProfile::RemigrateProfile(const PRUnichar* profileName)
>+{
>+    nsresult rv;
>+    NS_ENSURE_ARG_POINTER(profileName);
>+    
>+    nsCOMPtr<nsIFile> profileDir;
>+    rv = GetProfileDir(profileName, getter_AddRefs(profileDir));
>+    if (NS_FAILED(rv))
>+        return rv;
>+    nsCOMPtr<nsIFile> newProfileDir;
>+    rv = profileDir->Clone(getter_AddRefs(newProfileDir));
>+    if (NS_FAILED(rv))
>+        return rv;
>+
>+    // The profile list used by GetOriginalProfileDir is the one with ALL 4.x
>+    // profiles - even ones for which there's a moz profile of the same name.
>+    nsCOMPtr<nsILocalFile> oldProfileDir;
>+    rv = GetOriginalProfileDir(profileName, getter_AddRefs(oldProfileDir));
>+    if (NS_FAILED(rv))
>+        return rv;
>+
>+    // In case of error, we'll restore what we've renamed.
>+    nsXPIDLCString origDirLeafName;
>+    rv = profileDir->GetLeafName(getter_Copies(origDirLeafName));
>+    if (NS_FAILED(rv))
>+        return rv;
>+        
>+    // Backup what we're remigrating by renaming it and leaving in place
>+    // XXX todo 
>+    // what if foo-old exists?  we need to createunique or something

I guess - if we ever do this twice to the same profile. 2 different long
periods of inactivity?
Since we don't have the equiv of tmpnam on nsILocalFile, it would have to be
done with createunique, get the leaf name then, delete the file, etc.

>+    nsCAutoString newDirLeafName(origDirLeafName + NS_LITERAL_CSTRING("-old"));
>+    rv = profileDir->MoveTo(nsnull, newDirLeafName.get());
>+    if (NS_FAILED(rv))
>+        return rv;
>+    
>+    // XXX todo
>+    // on win2k [at least for me], after MoveTo(), empty folders are being left behind!
>+    // without this call to Remove(), the call to Create() fails.  
>+    rv = newProfileDir->Remove(PR_TRUE);
>+    if (NS_FAILED(rv))
>+        return rv;

Sounds like a bug in either Win2k that nsLocalFileWin should take care or or a
bug in nsLocalFileWin - doesn't happen on Mac.

>+
>+    // Create a new directory for the remigrated profile

Comment 41

[jglick]

> I liked the wording about what's in a profile from the previous dialog with 
> the picker though.  We know users don't know what the word profile means, so 
> explaining that it's bookmarks and mail helps them get the right answer.

We could go with:

"A more recent <Netscape 4.5+> version of this profile (which contains
your bookmarks, email settings, address books and preferences) was
found. Would you like to use the more recent profile?"

Comment 42

[jglick]

>I figured we'd do this the first time the user used a profile, not every 6
>months that they don't use it. 

Agree. IF they use the profile AND its > 6 months since the last time the 
ns6.0+/moz profile was used, they get the dialog.

Comment 43

[selmer (gone)]

If we're bought into showing the dialog every time, perhaps the interval doesn't
need to be so long?

The profile code already has a way of handling directory name clashes, that's
how we get all the profname-1, profname-2 directories.  Could we use that same
code to get profname.old, profname.old-1, ... rather than leave that error case
dangling?

If I run netscp.exe -mail, does prefs.js get updated?  I assume that it does,
but it's worth verifying since that file is being used to derive the last used date.

If we're using the "users50" directory as our clue that this is a defunct
profile, then a remigrated profile would never be re-remigrated since we'll have
put it in the "profiles" directory.  What clue are we using to say that it's a
profile created with a version from late 2000?

Comment 44

[kmurray1115]

I'd like to avoid showing the dialog every time. I thought that the conditions
we thought through were sound, but I don't mind discussing them again. It seems
that if the last modified date is judged to be "current" that we shouldn't
bother showing this. 

Comment 45

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

We don't show the dialog every time.

We only show the dialog:

if your 6.x profile hasn't been used in 6 months
and your 4.x profile (of the same name) has been used more recently than your 
6.x profile.

Note, if you get the dialog, and you say:  "no, don't remigrate", the 6.x 
profile has now been used.  and you won't see the dialog again.

Comment 46

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

Attachment: updated patch, more comments, fixed some micro -> milli sec issues.

Comment 47

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

one problem I'm seeing:

we never properly write out the profile's last modified time.

The reason is we are trying to update the registry on shut down, we fail to
create instance of NS_REGISTRY_CONTRACTID, because we are in shutdown.

the problem is we can't call UpdateCurrentProfileModTime(PR_TRUE); from 
nsProfile's dtor.
 
nsProfileAccess::UpdateRegistry(nsIFile * 0x00000000) line 647
nsProfile::UpdateCurrentProfileModTime(int 1) line 2008
nsProfile::~nsProfile() line 332
nsProfile::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsProfile::Release(nsProfile * const 0x01615270) line 359 + 141 bytes
nsSupportsArray::Clear(nsSupportsArray * const 0x002f8758) line 600 + 54 bytes
nsSupportsArray::DeleteArray() line 304
nsSupportsArray::~nsSupportsArray() line 147
nsSupportsArray::`vector deleting destructor'(unsigned int 1) + 81 bytes
nsSupportsArray::Release(nsSupportsArray * const 0x002f8758) line 238 + 139 
bytes
nsCOMPtr<nsISupportsArray>::~nsCOMPtr<nsISupportsArray>() line 491
nsDirectoryService::~nsDirectoryService() line 596 + 11 bytes
nsDirectoryService::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsDirectoryService::Release(nsDirectoryService * const 0x002f78a8) line 598 + 
136 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 565 + 27 bytes
main(int 3, char * * 0x00304840) line 1773 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

Comment 48

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

talked to ccarlen, we're going to move the call to UpdateCurrentProfileModTime
(PR_TRUE); from nsProfile's dtor to SetCurrentProfile().

testing that change now...

Comment 49

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

Attachment: updated patch, now we properly update the last modified time in the registry on shutdown.

Comment 50

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

Attachment: updated screenshot

Comment 51

[jglick]

"< >" aren't necessary. Just my way of indicating, "insert correct product name 
here". :-)

Comment 52

[Conrad Carlen (not reading bugmail)]

Comment on attachment 80627 [details] [diff] [review]
updated patch, now we properly update the last modified time in the registry on shutdown.

>+//pref("profile.seconds_until_defunct", -1);
>+pref("profile.seconds_until_defunct", 10);
>+

This value is good for testing but make sure you put in a real value before
checkin.

>+ 
>+  nsXPIDLString dialogText, button0Text, button1Text;
>+  rv = migrationBundle->GetStringFromName(NS_LITERAL_STRING("confirmRemigration").get(), getter_Copies(dialogText));
>+  NS_ENSURE_SUCCESS(rv,rv);
>+
>+  rv = migrationBundle->GetStringFromName(NS_LITERAL_STRING("confirmRemigrationButtonYes").get(), getter_Copies(button0Text));
>+  NS_ENSURE_SUCCESS(rv,rv);
>+
>+  rv = migrationBundle->GetStringFromName(NS_LITERAL_STRING("confirmRemigrationButtonNo").get(), getter_Copies(button1Text));
>+  NS_ENSURE_SUCCESS(rv,rv);
>+
>+  nsCOMPtr<nsIPromptService> promptService(do_GetService("@mozilla.org/embedcomp/prompt-service;1", &rv));
>+  NS_ENSURE_SUCCESS(rv,rv);
>+  PRInt32 buttonPressed;
>+  rv = promptService->ConfirmEx(nsnull, brandName.get(),
>+    dialogText.get(),
>+    (nsIPromptService::BUTTON_POS_0 * 
>+    nsIPromptService::BUTTON_TITLE_IS_STRING) + 
>+    (nsIPromptService::BUTTON_POS_1 * 
>+    nsIPromptService::BUTTON_TITLE_IS_STRING),
>+    button0Text.get(),
>+    button1Text.get(),
>+    nsnull, nsnull, nsnull, &buttonPressed);
>+  NS_ENSURE_SUCCESS(rv,rv);
>+

From the latest screen shot, the button titles are still "Yes" and "No" If
that's the final text for those buttons, you don't need to fetch strings and
use the BUTTON_TITLE_IS_STRING const with confirmEx. confirmEx was designed to
avoid that. For "Yes" and "No", you can pass BUTTON_TITLE_YES, and
BUTTON_TITLE_NO and save code, and save stuff to be localized.

>+
>+  // call ShutDownCurrentProfile() so we update the last modified time of the profile
>+  {
>+    // scoping this in a block to force release
>+    nsCOMPtr<nsIProfileInternal> profileMgr(do_GetService(NS_PROFILE_CONTRACTID, &rv));
>+    NS_ASSERTION(NS_SUCCEEDED(rv), "failed to get profile manager, so unable to update last modified time");
>+    if (NS_SUCCEEDED(rv)) {
>+      // 0 is undefined, we use this secret value so that we don't notify
>+      profileMgr->ShutDownCurrentProfile(0);

ShutDownCurrentProfile() is on plain-jane nsIProfile - no need for
nsIProfileInternal here. I'd like to keep the consumers of nsIProfileInternal
as limited as possible.

>+# LOCALIZATION NOTE: Do not translate Unicode (\uXXXX) characters.
>+confirmRemigration=A more recent <Netscape 4.5+> version of this profile (which contains your bookmarks, email settings, address books and preferences) was found.\u000aWould you like to use the more recent profile?
>+confirmRemigrationButtonYes=Yes
>+confirmRemigrationButtonNo=No

See previous comment about not needing "Yes" and "No" strings

Comment 53

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

Attachment: updated mozilla patch (ns patch stays the same), addresses ccarlen's comments.

Comment 54

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

Attachment: screen shot, no < or >

Comment 55

[Conrad Carlen (not reading bugmail)]

Comment on attachment 80659 [details] [diff] [review]
updated mozilla patch (ns patch stays the same), addresses ccarlen's comments.

>+++ profile/src/nsProfile.cpp	23 Apr 2002 21:11:11 -0000
>@@ -327,7 +327,9 @@
> 
>    if (--gInstanceCount == 0) {
>         
>+      if (gProfileDataAccess) {
>       delete gProfileDataAccess;
>+      }
> 

There used to be something else in this block, justifying the "if". Now that
there's not, don't need the "if"
This is a minor nit, I should have noticed it before, and the r= stands.
Nice.

Comment 56

[kmurray1115]

Seth, Conrad - thanks. You guys rock!

Comment 57

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

>+      if (gProfileDataAccess) {
> delete gProfileDataAccess;
>+      }
> 


those lines aren't necessary at all, since it is legal to delete null.

I've removed them locally.

Comment 58

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

status update:

I still need to test the patch on linux and mac, after that, get some reviews
and superreviews.

Just met with gbush to discuss the test plan.

Comment 59

[racham]

r=bhuvan. Looks fine. Given the changes in here, I think we do have to cover the
complete profile manager test suite. 

Comment 60

[racham]

Putting back Seth's status whiteboard updates..

Comment 61

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

setting as p1.

I've tested the patch on linux.  I needed to fix 
mozilla/profile/src/Makefile.in (to include profile) to get it to build.

Next, I'll test on mac, and then run through the profile manager test plan to 
make sure these changes won't cause regressions.

I still need to figure out why:

+    // XXX todo
+    // on win2k [at least for me], after MoveTo(), empty folders are being 
left behind!
+    // without this call to Remove(), the call to Create() fails.  
+    rv = newProfileDir->Remove(PR_TRUE);
+    if (NS_FAILED(rv))
+        return rv;

is needed for me on win2k, but not linux (and I assume, mac)  
I'll investigate today.

Comment 62

[Conrad Carlen (not reading bugmail)]

Wait, I missed something:

+    // without this call to Remove(), the call to Create() fails.  
+    rv = newProfileDir->Remove(PR_TRUE);
+    if (NS_FAILED(rv))
+        return rv;

When I tested this on Mac (in my patch) I didn't need the Remove() because the
folder was successfully renamed with nothing left behind. If you fail on the
error to Remove() on the Mac, it will fail because there will be nothing to remove.

Comment 63

[David :Bienvenu]

+      if (gProfileDataAccess) {
       delete gProfileDataAccess;
+      }
 
delete checks for null, so you don't have to. I'll look at the patch more tomorrow.

Comment 64

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

turns out, the MoveTo() issue is a known bug.  see bug #101527

There's a patch for it, I'll try to drive it in with this bug.

new patch coming with the patch for #101527

Comment 65

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

Attachment: update patch, includes fix to xpcom for moveTo() issue on win2k

Comment 66

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

just built and tested the latest patch on mac, works fine.

so good on mac, windows, and linux.

what's left before this can land:

1) land fix for bug #101527
2) get final reviews of ns and mozilla patches
3) finish running the profile function tests:
 http://www.mozilla.org/profilemanager/checklist.txt
 [internal] http://slip/projects/mojo/profile/65-profileindex.htm

eta to land, thursday, 4/25

Comment 67

[David :Bienvenu]

whoops, should have read all the comments for that null delete check.

some K&R braces style crept in here, even though the file seems to be mostly
using the One True Braces Style.
+    if (rhs.resolvedLocation) {
+        regLocationData.Truncate(0);
+        rv = rhs.resolvedLocation->Clone(getter_AddRefs(file));
         if (NS_SUCCEEDED(rv))
             resolvedLocation = do_QueryInterface(file);
+        file = nsnull;
+    }
+    else
+        regLocationData = rhs.regLocationData;
+    
+    migratedFrom = nsnull;    
+    if (rhs.migratedFrom) {

Should you use PRTime instead of PRInt64 for vars that are the result of
PR_Now() and the like? Although they are equivalent, it makes the code a little
clearer and perhaps more portable?

Why use an autostring in a class? Shouldn't we just use an nsString?  
nsAutoString mCurrentProfileName;

more later...

Comment 68

[Conrad Carlen (not reading bugmail)]

> some K&R braces style crept in here, even though the file seems to be mostly
using the One True Braces Style.

Don't even get started on the indentation and whitespace of this code. The whole
profile module is a mish-mash of various styles.  If not for wiping out CVS
blame in whatever the current rev is, I would run it though a formatting tool
and make it consistent. I might anyway.

> Should you use PRTime instead of PRInt64 for vars that are the result of
PR_Now() and the like?

The unit of time for a PRTime is a microsecond. Since we divide the value from
PR_Now to get millisecs, I think it's less confusing as a PRInt64.

Comment 69

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

> Why use an autostring in a class? Shouldn't we just use an nsString?  
> nsAutoString mCurrentProfileName;

good catch.  yes, we should be using nsString instead.  I'll fix that, look 
into the PRTime / PRInt64 issue and attach a new patch when I'm done testing.

Comment 70

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

ok, testing has gone well.

this patch introduces one assertion (see #140158).  But I think we can land 
with this assertion.

I've found other assertions when migrating, but they are not related.  (see 
#140156 and #140157)

updated patch coming soon...

Comment 71

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

Attachment: updated patch

Comment 72

[David :Bienvenu]

Comment on attachment 81052 [details] [diff] [review]
updated patch

sr=bienvenu

Comment 73

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

Comment on attachment 81052 [details] [diff] [review]
updated patch

r=sspitzer on the parts of this patch that ccarlen wrote.  I'd like him to
review the parts I wrote, and bhuvan to review the whole enchilada.

Comment 74

[Conrad Carlen (not reading bugmail)]

Comment on attachment 81052 [details] [diff] [review]
updated patch

r=ccarlen

Comment 75

[racham]

Comment on attachment 81052 [details] [diff] [review]
updated patch

r=bhuvan.

Seth, can you also just run few tests on old/new activation models ? Will send
you mail on those basic tests. thanks.

Comment 76

[racham]

Comment on attachment 80539 [details] [diff] [review]
ns tree diff (to set the interval to 6 months)

r=bhuvan

Comment 77

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

note to gbush:

bhuvan asked me to run through these tests:

1. Go through old activation. activate. Browser comes up. Close the browser and 
launch with that profile again. You should not see the activation window again
2. same thing to be repeated by cancelling activation window (cancel + cancel) 
in old activation. Close the browser widnow. Launch with that profile. You 
should not see the activation window

Above steps to be repeated with new activation. Change 
preference "browser.test_new_activation" to true in all-ns.js for new 
activation dialogs.

I did, and it worked fine.

fixed landed on trunk, will seek approval for the branch.

Comment 78

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

more notes for my pal Grace:

the pref is an int pref, "profile.seconds_until_defunct"

on mozilla, it defaults to -1, which means "never offer to re-migrate."
in ns, it defaults the number of seconds in 6 months.

to testing, I suggest setting it to 10 (in all-ns.js)

pref("profile.seconds_until_defunct", 10);

That way you don't have to mess with your system clock to test.  You will have 
to mess with the system clock once (per platform) to make sure we really do the 
right thing after six months.

Comment 79

[Grace Bush]

help!!
I cannot get this to work.
My steps: 
set system date to 7/29/01
launch 6.0 and migrate a 4.x profile, Abby (set with refdesk as home page)
change home page to mozilla
exit and verify prefs.js has correct time stamp
change system date to today 4/29/02
launch MachV
at UI requesting a new migration say Yes 
verify home page is refdesk (4.x profile) 

I am not seeing
1. the correct home page ( I still see the setting I made in 6.0)
2. I do not see a  copy of .slt  dire (.slt-old)

What I do see in my Abby dir is 4 .slt files only 1 of which is accessible 
(other 3 show as deleted and may be result of my multiple tests)

from the registry: after second migration
Common/Profiles
    CurrentProfile (S)=Abby
    OldRegDataMoved (S)=yes
    HavePregInfo (S)=
    Version (S)=1.0
Common/Profiles/Abby
    LastModTime (B)=$\0x87\0x8a\0x82\0xed\0x00\0x00\0x00
    CreationTime (B)=\0x00\0x00\0x00\0x00\0x00\0x00\0x00\0x00
    NCHavePregInfo (S)=yes
    NCEmailAddress (S)=
    NCServiceDenial (S)=1
    NCProfileName (S)=
    migrated (S)=yes
    directory (S)=C:\WINNT\PROFILES\GBUSH\APPLICATION 
DATA\Mozilla\Users50\Abby\leaoycuf.slt
Common/Profiles/gbush
    LastModTime (B)=\0x00\0x00\0x00\0x00\0x00\0x00\0x00\0x00
    CreationTime (B)=\0x00\0x00\0x00\0x00\0x00\0x00\0x00\0x00
    NCHavePregInfo (S)=
    NCEmailAddress (S)=
    NCServiceDenial (S)=
    NCProfileName (S)=
    migrated (S)=no
    directory (S)=C:\Program Files\Netscape\Users\gbush

Comment 80

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

re-opening, this is not working properly on windows.  [but appears to work on
mac and linux].

I met with grace, and here's a summary of the problems she's seeing:

win98:
on re-migration, *no migration progress dialog*, defunct 6.x profile is moved to
side (she gets a -old), 4.x profile is not re-migrated, *application hangs*.

winnt:
on remigration, *no migration progress dialog*, defunct 6.x profile is moved to
side (she gets a -old), 4.x profile appears to be migrated, but is removed, and
*-old is restored*, (failure would cause this), when app starts we're back to
our 6.x defuct profile.

win2k:
I think grace saw the same problems as winnt

I'm going to:
1) try out an optimized build on my win2k box, as I didn't see these problems on
my win2k debug build.
2) get a special win32 build to use to find out what's going on.

I'll update with my findings.

Comment 81

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

some info:

I tried the mozilla, optimized trunk bits on my win2k box.

I'm seeing something similar to what gbush saw on her win98:

the app freezes.

Here's the stack, without symbols:

XPCOM! 60e73266()
XPCOM! 60e728ff()
XPCOM! 60e4872a()
XPCOM! 60e4866a()
XPCOM! 60e48e4e()
XPCOM! 60e490e2()
APPSHELL! 60956ead()
APPSHELL! 60956926()
MOZILLA! 00402cad()
MOZILLA! 00401efc()
MOZILLA! 00401ac9()
MOZILLA! 004036a4()
KERNEL32! 77e87903()

this makes me think it's dying in nsLocalFileWin.cpp, in MoveTo(), which I 
recently tweaked.  that would explain why it's windows only.

investigating...

Comment 82

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

on second thought, it if was moveTo(), profile would be on the stack.  so it
must be something else.

I'll continue to investigate...

Comment 83

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

I'm seeing *tons* of assertions in my windows debug build, in xpcom / string. 
ones that weren't there when I checked in.

I wonder if this is related the the failure?  Still investigating...

Comment 84

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

here's the first assertion:

NTDLL! 77f9f9df()
nsDebug::Assertion(const char * 0x1012a808 `string', const char * 0x1012a848 
`string', const char * 0x1012a6d0 `string', int 0x00000164) line 291 + 13 bytes
nsWritingIterator<char>::write(const char * 0x0012fbbc, unsigned int 
0x00000008) line 356 + 37 bytes
nsCharSinkTraits<nsWritingIterator<char> >::write(nsWritingIterator<char> & 
{...}, const char * 0x0012fbbc, unsigned int 0x00000008) line 559
copy_string(nsReadingIterator<char> & {...}, const nsReadingIterator<char> & 
{...}, nsWritingIterator<char> & {...}) line 90 + 39 bytes
nsACString::do_AppendFromReadable(const nsACString & {...}) line 881 + 55 bytes
nsACString::AppendFromPromise(const nsACString & {...}) line 857
nsACString::Append(const nsAPromiseCString & {...}) line 673
nsLocalFile::AppendRelativeNativePath(nsLocalFile * const 0x0179ddd8, const 
nsACString & {...}) line 869 + 44 bytes
nsLocalFile::AppendNative(nsLocalFile * const 0x0179ddd8, const nsACString & 
{...}) line 855
nsLocalFile::CopyMove(nsIFile * 0x00000000, const nsACString & {...}, int 
0x00000000, int 0x00000001) line 1184
nsLocalFile::MoveToNative(nsLocalFile * const 0x0179ddd8, nsIFile * 0x00000000, 
const nsACString & {...}) line 1205
nsProfile::RemigrateProfile(nsProfile * const 0x01787230, const unsigned short 
* 0x0178aa88) line 2310 + 74 bytes
nsAppShellService::CheckAndRemigrateDefunctProfile() line 389 + 42 bytes
nsAppShellService::DoProfileStartup(nsAppShellService * const 0x01787e08, 
nsICmdLineService * 0x01229e58, int 0x00000001) line 256 + 8 bytes
InitializeProfileService(nsICmdLineService * 0x01229e58) line 1086 + 31 bytes
main1(int 0x00000004, char * * 0x00304b80, nsISupports * 0x00000000) line 1384 
+ 14 bytes
main(int 0x00000004, char * * 0x00304b80) line 1779 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

Comment 85

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

in my debug build, I loop forever with these assertions:

###!!! ASSERTION: |copy_string| will never terminate: 'count_copied > 0', file .
.\..\dist\include\string\nsAlgorithm.h, line 91
###!!! ASSERTION: You can't |write| into an |nsWritingIterator| with no space!:
'size_forward() > 0', file ..\..\dist\include\string\nsStringIterator.h, line 35
6
###!!! ASSERTION: |copy_string| will never terminate: 'count_copied > 0', file .
.\..\dist\include\string\nsAlgorithm.h, line 91
###!!! ASSERTION: You can't |write| into an |nsWritingIterator| with no space!:
'size_forward() > 0', file ..\..\dist\include\string\nsStringIterator.h, line 35
6

Comment 86

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

Attachment: string issue workaround patch

Comment 87

[Alec Flett]

Comment on attachment 81635 [details] [diff] [review]
string issue workaround patch

sr=alecf, or use NS_NAMED_LITERAL_CSTRING, sr= with that as well.

Comment 88

[Alec Flett]

Comment on attachment 81635 [details] [diff] [review]
string issue workaround patch

see http://www.mozilla.org/projects/xpcom/string-guide.html#Concatenation for
more details

Comment 89

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

using NAMED_LITERAL_CSTRING had the same assertions as NS_LITERAL_STRING, 
unless I'm not doing what you suggested.

going with the original patch, I'll discuss with this alecf more tomorrow.

I'm able to re-migrate again, marking fixed so that gbush will try with the 
trunk builds again.

Comment 90

[Grace Bush]

update: single profile case is working on windows :)))))
moving on to other tests/platforms

Comment 91

[jag (Peter Annema)]

Has a bug been filed on the underlying issue?

Comment 92

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The underlying issue in the string code is bug 70083.  The hang case is exactly
the same as the one reported in bug 74709.  In particular (based on the stack in
comment 84), you're passing an nsPromiseConcatenation to
nsLocalFile::MoveToNative, and then nsLocalFile::AppendRelativeNativePath makes
another nsPromiseConcatenation with the first promise as the second part.  The
current fragment API is insufficient to handle that case.  (See why I think bug
70083 should be fixed before we freeze nsAString?)

Comment 93

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

er, nsPromiseConcatenation is called nsDependentConcatenation now.

Comment 94

[Jay Patel [:jay]]

I'm seeing a lot of crashes with the following stack in recent MozillaTrunk builds:
nsLocalFile::Append
[nsLocalFileCommon.cpp  line 271] 
	 CSpellChecker::CSpellChecker
[spellchk.cpp  line 369] 
	 nsSpellCheckGlue::ReplaceAllOccurrences
[nsSpellCheckGlue.cpp  line 1137] 
	 nsSpellCheckGlue::NextMisspelledWord
[nsSpellCheckGlue.cpp  line 272] 
	 nsQueryElementAt::nsQueryElementAt  
	 nsComponentManager::CreateInstance
[nsComponentManagerObsolete.cpp  line 115] 
	 nsEditorShell::InitSpellChecker
[nsEditorShell.cpp  line 3843] 
	 XPTC_InvokeByIndex
[xptcinvoke.cpp  line 106] 
	 XPCWrappedNative::CallMethod
[xpcwrappednative.cpp  line 1995] 
	 XPC_WN_CallMethod
[xpcwrappednativejsops.cpp  line 1267] 
	 js_Invoke
[jsinterp.c  line 790] 
	 js_Interpret
[jsinterp.c  line 2744] 
	 js_Invoke
[jsinterp.c  line 806] 
	 js_InternalInvoke
[jsinterp.c  line 881] 
	 JS_CallFunctionValue
[jsapi.c  line 3414] 
	 nsJSContext::CallEventHandler
[nsJSEnvironment.cpp  line 1019] 
	 nsJSEventListener::HandleEvent
[nsJSEventListener.cpp  line 184] 
	 nsEventListenerManager::HandleEventSubType
[nsEventListenerManager.cpp  line 1220] 
	 nsEventListenerManager::HandleEvent
[nsEventListenerManager.cpp  line 1895] 
	 GlobalWindowImpl::HandleDOMEvent
[nsGlobalWindow.cpp  line 755] 
	 DocumentViewerImpl::LoadComplete
[nsDocumentViewer.cpp  line 1538] 
	 nsDocShell::EndPageLoad
[nsDocShell.cpp  line 3958] 
	 nsWebShell::EndPageLoad
[nsWebShell.cpp  line 731] 
	 nsDocShell::OnStateChange
[nsDocShell.cpp  line 3875] 
	 nsDocLoaderImpl::FireOnStateChange
[nsDocLoader.cpp  line 1105] 
	 nsDocLoaderImpl::doStopDocumentLoad
[nsDocLoader.cpp  line 744] 
	 nsDocLoaderImpl::DocLoaderIsEmpty
[nsDocLoader.cpp  line 642] 
	 nsDocLoaderImpl::OnStopRequest
[nsDocLoader.cpp  line 573] 
	 nsLoadGroup::RemoveRequest
[nsLoadGroup.cpp  line 531] 
	 nsJARChannel::OnStopRequest
[nsJARChannel.cpp  line 612] 
	 nsOnStopRequestEvent::HandleEvent
[nsRequestObserverProxy.cpp  line 213] 
	 PL_HandleEvent
[plevent.c  line 597] 
	 PL_ProcessPendingEvents
[plevent.c  line 530] 
	 _md_EventReceiverProc
[plevent.c  line 1078]  

I'm guessing this might be related to the tweaking mentioned by Seth in comment
#81.  If that is not true, let me know so I can log a new bug for those crashes.
If it is true, I can add the topcrash info here for tracking so I can verify any
fix that goes in for this bug (or log a new bug if that would be the best thing
to do).

Comment 95

[Grace Bush]

Mac and Linux still testing ok with single profile
Windows gave me a few succesful tests on NT with build 2002043003 on NT.  Tests 
with more than one migrated profile failed- no remigration, no old file

the later trunk build, 2002043010- on 2k and 98 are not showing a remigration 
with single profile.
:(

Comment 96

[Darin Fisher]

jpatel: what platform's are crashing with that stack trace?

Comment 97

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

>I'm guessing this might be related to the tweaking mentioned by Seth in comment
>#81.  If that is not true, let me know so I can log a new bug for those crashes.
>If it is true, I can add the topcrash info here for tracking so I can verify any
>fix that goes in for this bug (or log a new bug if that would be the best thing
>to do).

jpatel, that issue is seperate from this bug.  can you start a new bug (probably
on darin?) for that issue with that stack?

Comment 98

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

accepting.

to avoid getting hit by trunk regressions, we want to land this on the 1.0 and 
resume testing there.

I'm going to see drivers approval for that.

Comment 99

[chris hofmann]

Comment on attachment 81635 [details] [diff] [review]
string issue workaround patch

a=chofmann off by default on the 1.0 branch

Comment 100

[Jay Patel [:jay]]

I have logged bug 141560 for the crash mentioned in comment #94.  Sorry for any
confusion.

Comment 101

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

Attachment: supplimental patch, for safer approach that avoids the XUL.mfl in use problem.

grace had found a problem where in certain scenarios, we'd fail to remigrate.
I debugged and found that the problem was we were failing to move the existing
xxxxxxxx.slt directory to the side, because one of the files (XUL.mfl) was in
use.

Here's a new patch, that similifies how remigration works on disk.

currently we do this:

move xxxxxxxx.slt to xxxxxxxx.slt-old 
remigrate the 4.x profile to xxxxxxxx.slt

with this patch, we do this:

leave xxxxxxxx.slt alone
remigrate the 4.x profile to xxxxxxxx.slt-new

This is faster (no extra moving of files), safer (the existing .slt directory
is untouched, so if we have to restore, we can), and less error prone (no issue
if XUL.mfl is in use, or if moving the existing .slt folder fails.)

I'll get reviews for this on the trunk, land it, let it bake, and then prepare
something for the branch.

Comment 102

[Conrad Carlen (not reading bugmail)]

with this patch, we do this:

>leave xxxxxxxx.slt alone
>remigrate the 4.x profile to xxxxxxxx.slt-new

With the new profile dir having a new name, all of the absolute paths in the
old, backed-up profile are now invalid, right? The reason I did it the way it's
being done was so that the absolute paths in old vs. new would be the same so
"*-old" was useful for back-up purposes.

Little did I realize that moving a profile dir was going to be so fraught with
peril :-/ Also, the profile is shut down before it is moved so nothing should
still have open files. What component uses XUL.mfl?

Comment 103

[chris hofmann]

Comment on attachment 81635 [details] [diff] [review]
string issue workaround patch

withdraw a= until in-use issues are worked out.

Comment 104

[David :Bienvenu]

Comment on attachment 81930 [details] [diff] [review]
supplimental patch, for safer approach that avoids the XUL.mfl in use problem.

sr=bienvenu

Comment 105

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

>With the new profile dir having a new name, all of the absolute paths in the
>old, backed-up profile are now invalid, right? The reason I did it the way it's
>being done was so that the absolute paths in old vs. new would be the same so
>"*-old" was useful for back-up purposes.

you mean, a user could just rename .slt-old to .slt to get the profile we moved
aside.  But not, since the registry points to .slt-new, they can't easily
restore the old .slt directory.  (renaming .slt to .slt-new will not work.)

Doesn't our profile UI allow users to pick existing folders?  To restore an old
6.0 profile, can't they use the UI and pick the existing .slt directory?  I'll
investigate that.

I haven't determined what was using XUL.mfl, but I could determine that the file
was in use.  (the windows error code we got while doing the move was
ERROR_SHARING_VIOLATION)

Comment 106

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

taking to ccarlen, restore is harder in the "-new" world, but still doable.
the user has to rename a folder and then hand edit their prefs.js

in the "-old" world, they could just rename a folder.

but "-new" is less risky, restoring should be rare.  I'll prepare a relnote on
how to do it.

Comment 107

[Conrad Carlen (not reading bugmail)]

Comment on attachment 81930 [details] [diff] [review]
supplimental patch, for safer approach that avoids the XUL.mfl in use problem.

r=ccarlen

Comment 108

[Conrad Carlen (not reading bugmail)]

Missed something:

+    // XXX todo: what if <xxxxxxxx>.slt-new already exists?
+    nsCAutoString newDirLeafName(origDirLeafName + NS_LITERAL_CSTRING("-new"));
+    rv = newProfileDir->SetLeafName(newDirLeafName.get());
     NS_ENSURE_SUCCESS(rv,rv);
      
     // Create a new directory for the remigrated profile
     rv = newProfileDir->Create(nsIFile::DIRECTORY_TYPE, 0775);

Since now we're actually creating the dir, avoiding the dup (useful for QA) is
just a matter of calling CreateUnique() instead of Create()

Comment 109

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

Attachment: same patch, with CreateUnique()

Comment 110

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

Comment on attachment 81962 [details] [diff] [review]
same patch, with CreateUnique()

forwarding r=ccarlen, sr=bienvenu

Comment 111

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

fixed on trunk.  seeking driver a= now...

Comment 112

[scottputterman]

Adding adt1.0.0+.  After getting drivers approval, please check this into the
branch and add the fixed1.0.0 keyword.

Comment 113

[selmer (gone)]

Are the comments about having to edit the prefs.js caused by not being able to
choose a salted profile in the UI?  I seem to recall that we forcibly add the
profile name to the end of the selected directory name.  At some point, we
should detect the presence of a salted subdirectory use it.  I think doing that
would side-step the issues mentioned above.

Comment 114

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

Attachment: the patch for the branch.

Comment 115

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

windows, mozilla, branch bits:

http://warp.mcom.com/u/sspitzer/mozilla-opt-branch.zip

and soon:

ftp://ftp.mozilla.org/pub/mozilla/nightly/experimental/bug-137886/mozilla-win32-
opt-branch.zip

I'm working on linux, mozilla, branch bits.

Comment 116

[Dawn Endico]

linux bits are here:

ftp://ftp.mozilla.org/pub/mozilla/nightly/experimental/bug-137886/mozilla-linux-opt-branch.zip

Comment 117

[Grace Bush]

preliminary testing on all platforms for  commercial trunk builds is successful

Also tested mozilla builds on win and linux branch- small subset of test and 
these were successful also

Comment 118

[Randell Jesup [:jesup] (needinfo me)]

Looking at an appreg or two, I find some absolute directory names including the
salt directory, which would make it hard to rename a salted profile.

Comment 119

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

> Looking at an appreg or two, I find some absolute directory names including the
> salt directory, which would make it hard to rename a salted profile.

I explained this over the phone.

The appreg contains the path to <xxxxxxx>.slt-new, yes.

So, to restore you'd have to move .slt-new to the side, rename .slt to .slt-new,
and then edit prefs.js, globally replacign ".slt" with ".slt-new"

Comment 120

[Paul Wyskoczka]

Does Grace's testing suffice?  If not what more can she do, so we can get an a= thx

Comment 121

[chris hofmann]

Comment on attachment 81999 [details] [diff] [review]
the patch for the branch.

a=chofmann -pref turned off- for the 1.0 branch

Comment 122

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

fixed on branch.  marking fixed.

thanks to grace for all her testing this week and thanks to ccarlen for his
initial patch, which was the bulk of the fix.

Comment 123

[Jaime Rodriguez, Jr.]

Seth says its fixed on the branch, so fixed1.0.0. :-)

Comment 124

[Grace Bush]

tested all platforms on branch builds for 5/7 
marking verified1.0.0

finished testing on trunk builds all platforms 5/5