User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a5pre) Gecko/20070530 SeaMonkey/2.0a1pre Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a5pre) Gecko/2007052909 SeaMonkey/2.0a1pre When transitioning from xpfe to suiterunner, only the first tab of my multitab homepage was migrated. Reproducible: Didn't try Steps to Reproduce: 1. In SeaMonkey 1.5a, open 2 or more tabs 2. Edit => Preferences 3. Load on [ Navigator Startup |v], (*) Home Page 4. (Home Page), [ Use Current Group ] 5. [ OK ] 6. Close Sm 1.5a and install Sm 2.0a1. 7. Migrate "from Sm 1.1, Mozilla or Netscape 6" and go through the popups. Do not forget to migrate the home page. 8. A "Welcome" page will congratulate you. 9. Click "Home". Actual Results: Only the first homepage tab loads. Expected Results: All tabs of the homepage should appear just like they were in 1.5a. Additional info: In my Sm 1.5 profile, all tabs were listed in prefs.js as the values of "browser.startup.homepage", "browser.startup.homepage.1", etc., to "browser.startup.homepage.28"; and "browser.startup.homepage.count" was set at 29. They were set to a variety of file: http: and about: -protocol pages. Only the first one (a Bugzilla bug page) opened in suiterunner.

P.S. Not sure under which component to file this bug.

(In reply to comment #1) > P.S. Not sure under which component to file this bug. > Oops. Moved to "Profile Manager" component. If Bugzilla doesn't do it, then please, someone with higher privileges than mine should set "Assigned To" and/or "QA Contact" as they should be.

I'll take a look at this soon

Note to self: I probably need to implement bug 371309 at the same time as doing this.

This ports big 371309 (Migration Wizard explicitly sets a homepage which is wrong when using a distribution) to SeaMonkey, I'm doing it as the first part of fixing this bug. Possibly means there will be some extra work for the complete fix, but it hopefully will make things clearer as to what is actually happening.

Comment on attachment 276683 [details] [diff] [review] Port bug 371309 Discussion with Neil on IRC recommends that if possible we change Default to Current if migrating into an existing profile with the homepage set. I need to check a few things first. So cancelling review for the time being.

Moving title around so that I can find the bug easier when I look for it in my lists.

As comment 5, but additionally checks to see if the target profile already has a homepage or not and changes the displayed text appropriately. Note I've also changed how the pref copying routines work so that we don't overwrite the existing prefs.js (completely that is) if we're migrating into an existing profile.

Comment on attachment 278297 [details] [diff] [review] Port bug 371309 v2 This keeps crashing when I try to run it. I don't think layout likes you calling ResetPrefs (and I blame layout, not you). >- if (!this._autoMigrate) >+ if (!this._autoMigrate) { > this._wiz.advance(); >+ return; >+ } So after this point we can be sure that this._autoMigrate exists, right? >+ homePageStart.setAttribute("value", "DEFAULT"); Why bother? You check for an empty value anyway. Might as well leave it empty. >+ this._newHomePage = document.getElementById("homePageRadioGroup") >+ .selectedItem.value; Actually the radiogroup's value should equal that of its selected item. >+ if (!this._autoMigrate) >+ return false; ... which means we don't have to check it again here. >+ var targetPrefsFile = this._autoMigrate.directory; This is cloned every time, right?

Comment on attachment 278297 [details] [diff] [review] Port bug 371309 v2 Oh, and this doesn't actually fix the bug yet does it?

(In reply to comment #9) > (From update of attachment 278297 [details] [diff] [review]) > This keeps crashing when I try to run it. I don't think layout likes you > calling ResetPrefs (and I blame layout, not you). Weird, I don't see any problems with that on Linux. > >- if (!this._autoMigrate) > >+ if (!this._autoMigrate) { > > this._wiz.advance(); > >+ return; > >+ } > So after this point we can be sure that this._autoMigrate exists, right? Correct. > >+ homePageStart.setAttribute("value", "DEFAULT"); > Why bother? You check for an empty value anyway. Might as well leave it empty. I'd copied it from the FF patch, but you're right, so I've dropped it. > >+ this._newHomePage = document.getElementById("homePageRadioGroup") > >+ .selectedItem.value; > Actually the radiogroup's value should equal that of its selected item. Fixed. > >+ if (!this._autoMigrate) > >+ return false; > ... which means we don't have to check it again here. Removed. > >+ var targetPrefsFile = this._autoMigrate.directory; > This is cloned every time, right? http://mxr.mozilla.org/seamonkey/source/toolkit/xre/nsXREDirProvider.cpp#758 It implements nsIProfileStartup, so I believe that's the right function. (In reply to comment #10) > (From update of attachment 278297 [details] [diff] [review]) > Oh, and this doesn't actually fix the bug yet does it? Correct, but I think the fix will be a lot simpler now.

Revised patch based on comments.

(In reply to comment #11) >I'd copied it from the FF patch That's always a bad move ;-)

Comment on attachment 278397 [details] [diff] [review] Port bug 371309 v3 (checked in) >- if (this._newHomePage) { >+ // If _newHomePage is default, don't touch the current pref - >+ // then if a new profile it won't be set, if not a new profile >+ // it'll be left as what the user wanted. >+ if (this._newHomePage && this._newHomePage != "") { The extra test is unnecessary. r=me with it reverted.

This fixes the bug. I've dropped nsISuiteProfileMigrator::FORMDATA (as it was obsolete and left over as it was easier) and replaced it by HOMEPAGEDATA. This means we can reuse some of the existing functions to determine if we have home page data or not which makes it hang together better IMHO. It also means we don't have to have the extra "hack" for copying home page data in migration.js, and that home page will now be listed on the list of things that are migrated. I created CopyHomePageData in nsNetscapeProfileMigratorBase as I'd expect a Firefox migrator to want to use it, and also I didn't feel it right to put the extra pref copies in TransformPreferences - as that is really just copying the core preferences.

Comment on attachment 278405 [details] [diff] [review] Implement the fix (checked in) Layout's crashing on me again :-( >+bool PRBool >+nsNetscapeProfileMigratorBase::CopyHomePageData(PRBool aReplace) Presumably by this point we know we're always going to replace?

(In reply to comment #16) > (From update of attachment 278405 [details] [diff] [review]) > Layout's crashing on me again :-( > > >+bool > PRBool opps. > >+nsNetscapeProfileMigratorBase::CopyHomePageData(PRBool aReplace) > Presumably by this point we know we're always going to replace? Yes, migration.js: + if (this._autoMigrate) { this._itemsFlags = this._migrator.getMigrateData(this._selectedProfile, this._autoMigrate); + if (!this._newHomePage) + this._itemsFlags &= ~nsISuiteProfileMigrator.HOMEPAGEDATA; + } this._itemsFlags is then passed to nsSeamonkeyProfileMigrator::Migrate(...): + if (aItems & nsISuiteProfileMigrator::HOMEPAGEDATA) + COPY_DATA(CopyHomePageData, aReplace, + nsISuiteProfileMigrator::HOMEPAGEDATA);

Both patches checked in -> fixed.