Closed Bug 689437 Opened 13 years ago Closed 12 years ago

Drop support for migrating from old profiles (drop usages of libreg)

Categories

(MailNews Core :: Import, defect)

defect
Not set
blocker

Tracking

(seamonkey2.7 unaffected, seamonkey2.8+ fixed)

RESOLVED FIXED
Thunderbird 11.0
Tracking Status
seamonkey2.7 --- unaffected
seamonkey2.8 + fixed

People

(Reporter: philip.chee, Assigned: Callek)

References

Details

Attachments

(3 files)

+++ This bug was initially created as a clone of Bug #689120 +++

Bug 679352 wishes to drop libreg support.

The easy way to do this for s/Thunderbird/SeaMonkey/ is to drop support for migrating from older Mozilla profiles - SeaMonkey 1.1.x and earlier (and hence the suite), and early versions of Thunderbird, from bug 679352 comment 4 I believe this is equivalent to:

> This code exists to import the profile list from Firefox <0.9.2 to
> Firefox >=0.9.3. I think it can go now.
Jens, I think this should be relnoted or otherwise documented.

http://forums.mozillazine.org/viewtopic.php?f=6&t=2316533

[quote]
So, Mozilla Core wants to get rid of some functionality which was necessary to import/migrate profiles prior to Firefox 0.9.x stored in the Mozilla Suite style (bug 679352). There is a corresponding Thunderbird version (bug 689120) to remove that functionality from comm-central as well. My guess is that it somehow (i.e., equally) would affect SeaMonkey as well.

Consequences? Starting with, let's say, SM 2.6, profiles from SeaMonkey 1.x, Netscape 6.x/7.x, and early (pre-toolkit) Firefox/Thunderbird version would no longer be recognized and migrated to the new format used since 2.0 (which was introduced much earlier by Firefox/Thunderbird).

Workaround: Users on 1.x or earlier would have to migrate to 2.[0-5] first, then update to 2.6 or later to be current.

I'm wondering how many users are actually still on SM 1.x or Netscape/Suite versions, thus would be affected by dropping that migration code... :-k
Last edited by rsx11m on Tue 27 Sep 2011 22:22, edited 1 time in total.
[/quote]
Yes, this should definitely have a release note with the workaround once implemented.
So, bug 679352 landed, libreg is gone. We need to get this done until 2.8 ships.

I guess a future relnote should recommend first upgrading to 2.0.14 and then to the latest 2.x.

While we're at it, we should probably go through the Startup & Profiles component and invalidate any bugs that still (exclusively) refer to the old migrator. And file bugs about porting new migrators, like for Opera and Chrome. :-)
Keywords: relnote
Target Milestone: --- → seamonkey2.8
Without this fixed, 2.8a1 nightlies don't build. Escalating to major and suggest blocking.
Severity: normal → critical
The severity of Thunderbird bug 689120 has been elevated to "blocker" today.
Attached patch WIPSplinter Review
WIP, SeaMonkey and Thunderbird now build, but a bit rough around the edges (I don't have any experience with libreg/profile migration) & haven't tested it / looked at what tests need removing/changing. 

Sorry lumped together, but wasn't really sure what belonged here vs bug 689120, since today was the first time I've built c-c / really looked at the tree properly.

Anyway, enough excuses, hope it's ok enough to get you started :-)
Comment on attachment 580813 [details] [diff] [review]
WIP

Review of attachment 580813 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, looks good to overall. There is lots and lots of cruft in this code as it is though :( But either way, a wonderful first step.

Going to attach my patch 2 (I want to give you credit when we push this) and then qfold it together for Mark and Neil to both review (either can say the others review is not necessary) and then land asap.

::: mail/components/migration/src/nsSeamonkeyProfileMigrator.cpp
@@ -309,5 @@
> -  seamonkeyData->Append(NS_LITERAL_STRING(OLD_FOLDER));
> -  seamonkeyData->Append(NS_LITERAL_STRING(REGISTRY_FILE));
> -
> -  return GetProfileDataFromRegistry(seamonkeyData, mProfileNames,
> -                                    mProfileLocations);

This whole section needs cleanup, doing bare minimum (so I don't break stuff) of that myself in patch 2

::: suite/locales/jar.mn
@@ -239,5 @@
>    locale/@AB_CD@/messenger/am-server-advanced.dtd                           (%chrome/mailnews/pref/am-server-advanced.dtd)
>    locale/@AB_CD@/messenger/am-server-top.dtd                                (%chrome/mailnews/pref/am-server-top.dtd)
>    locale/@AB_CD@/messenger/am-archiveoptions.dtd                            (%chrome/mailnews/pref/am-archiveoptions.dtd)
>    locale/@AB_CD@/messenger/appleMailImportMsgs.properties                   (%chrome/mailnews/appleMailImportMsgs.properties)
> -  locale/@AB_CD@/messenger/comm4xMailImportMsgs.properties                  (%chrome/mailnews/comm4xMailImportMsgs.properties)

NOTE: Seems suite was packaging this in the wrong exposed location anyway.

::: suite/profile/migration/src/nsProfileMigrator.h
@@ -42,5 @@
>  #include "nsIProfileMigrator.h"
>  #include "nsCOMPtr.h"
>  
>  #define NS_SUITEPROFILEMIGRATOR_CID \
> -{ 0x4ca3c946, 0x5408, 0x49f0, { 0x9e, 0xca, 0x3a, 0x97, 0xd5, 0xc6, 0x77, 0x50 } }

Technically I doubt we needed the CID change here, but I'd rather keep it in that risk breakage.

::: suite/profile/migration/src/nsSeamonkeyProfileMigrator.cpp
@@ +226,5 @@
>  
>  nsresult
>  nsSeamonkeyProfileMigrator::FillProfileDataFromRegistry()
>  {
> +  return NS_ERROR_FILE_NOT_FOUND;

We should probably just remove this provider.... (doing so in patch 2)
Attachment #580813 - Flags: feedback+
Assignee: nobody → bugspam.Callek
Status: NEW → ASSIGNED
This is the folded version of the above 2 patches, for easier overall review (I'll be tackling any review comments anyway).

Feel free to cancel each others review if you like, I also already reviewed the code, but this is not my expertise of an area, so even if I didn't make changes would want one of you to review.
Attachment #580850 - Flags: review?(neil)
Attachment #580850 - Flags: review?(mbanner)
Lets start by shifting this to mailnews core as it is effectively a core bug.
Component: Startup & Profiles → Import
Product: SeaMonkey → MailNews Core
QA Contact: profile-manager → import
Target Milestone: seamonkey2.8 → ---
Severity: critical → blocker
Summary: Drop support for migrating from old profiles (drop usages of libreg) from SeaMonkey → Drop support for migrating from old profiles (drop usages of libreg)
No longer depends on: 689120
Comment on attachment 580850 [details] [diff] [review]
folded patch (do not land me)

Review of attachment 580850 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/migration/src/nsNetscapeProfileMigratorBase.cpp
@@ -165,5 @@
>    return NS_OK;
>  }
>  
> -nsresult
> -nsNetscapeProfileMigratorBase::GetProfileDataFromRegistry(nsILocalFile* aRegistryFile,

As we're stopping import from old versions, we need to change the importFromSeamonkey2.label (migration.dtd) string from

"Netscape 6, 7, Mozilla 1.x or SeaMonkey"

to

"SeaMonkey 2 or later".

Please bump the id of the string and the accesskey as well.

Whilst you're here, please also just remove the three sourceName* strings from Thunderbird's migration.properties as they aren't used and would be wrong after this patch anyway:

http://mxr.mozilla.org/comm-central/search?string=sourceName&find=%2Fmail%2F&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

(Suite uses them for setting the homepage in migration.js)

::: mail/components/migration/src/nsProfileMigrator.h
@@ +48,4 @@
>  #include "nsStringGlue.h"
>  
>  #define NS_THUNDERBIRD_PROFILEIMPORT_CID \
> +{ 0xea05dd97, 0xb66b, 0x4305, { 0xa6, 0x58, 0xd5, 0xde, 0x7f, 0x54, 0x0c, 0xc9 } }

Unless I'm very much mistaken, nsProfileMigrator is not an interface, so we don't need to change its CID. This is only a component id used to look up the component and not something binary extensions would hook into.

::: mailnews/import/content/importDialog.js
@@ -679,5 @@
> -              if (promptService) {
> -                var clickedOk = false;
> -                clickedOk = promptService.select(window, 
> -                                               gImportMsgsBundle.getString('profileTitle'),
> -                                               gImportMsgsBundle.getString('profileText'),

profileTitle & profileText need removing from importMsgs.properties as well.
Comment on attachment 580850 [details] [diff] [review]
folded patch (do not land me)

This looks fine with the changes mentioned addressed. I've not looked in detail at the changes in suite though, but they seem approximately correct.

This should also get sr due to the amount we're changing/removing.
Attachment #580850 - Flags: superreview?(neil)
Attachment #580850 - Flags: review?(neil)
Attachment #580850 - Flags: review?(mbanner)
Attachment #580850 - Flags: review+
Comment on attachment 580850 [details] [diff] [review]
folded patch (do not land me)

>   // Try profiles.ini first
>   nsresult rv = GetProfileDataFromProfilesIni(thunderbirdData,
>                                               mProfileNames,
>                                               mProfileLocations);
> 
>-  if (rv != NS_ERROR_FILE_NOT_FOUND)
>-    return rv;
>-
>-  thunderbirdData->Append(NS_LITERAL_STRING(REGISTRY_FILE));
>-
>-  // Then try the old registry format
>-  return GetProfileDataFromRegistry(thunderbirdData, mProfileNames,
>-                                    mProfileLocations);
>+  return rv;
> }
This entire block (including the unchanged lines) should just read

return GetProfileDataFromProfilesIni(thunderbirdData,
                                     mProfileNames,
                                     mProfileLocations);
Attachment #580850 - Flags: superreview?(neil) → superreview+
Landed

http://hg.mozilla.org/comm-central/rev/5d6b25eda12e
http://hg.mozilla.org/comm-central/rev/1afd843a67b1
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 11.0
Landed followup (but pointed at wrong bug) http://hg.mozilla.org/comm-central/rev/7814c0f2732d
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.