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

RESOLVED FIXED in Thunderbird 11.0

Status

MailNews Core
Import
--
blocker
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: Philip Chee, Assigned: Callek)

Tracking

Trunk
Thunderbird 11.0

SeaMonkey Tracking Flags

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

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
+++ 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.
(Reporter)

Comment 1

6 years ago
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]

Comment 2

6 years ago
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. :-)
tracking-seamonkey2.8: --- → ?
Keywords: relnote
Target Milestone: --- → seamonkey2.8
Without this fixed, 2.8a1 nightlies don't build. Escalating to major and suggest blocking.
Severity: normal → critical
status-seamonkey2.8: --- → affected

Comment 5

6 years ago
The severity of Thunderbird bug 689120 has been elevated to "blocker" today.

Comment 6

5 years ago
Created attachment 580813 [details] [diff] [review]
WIP

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 :-)
(Assignee)

Comment 7

5 years ago
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)

Comment 8

5 years ago
Created attachment 580846 [details] [diff] [review]
patch2 (on top of WIP)
Assignee: nobody → bugspam.Callek
Status: NEW → ASSIGNED
(Assignee)

Comment 9

5 years ago
Created attachment 580850 [details] [diff] [review]
folded patch (do not land me)

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)
(Assignee)

Updated

5 years ago
tracking-seamonkey2.8: ? → +
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
Duplicate of this bug: 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+
(Assignee)

Comment 15

5 years ago
Landed

http://hg.mozilla.org/comm-central/rev/5d6b25eda12e
http://hg.mozilla.org/comm-central/rev/1afd843a67b1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-seamonkey2.7: --- → unaffected
status-seamonkey2.8: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 11.0
(Assignee)

Comment 16

5 years ago
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.