Only first tab of multitab homepage is migrated (xpfe -> suiterunner migration)

RESOLVED FIXED

Status

SeaMonkey
Startup & Profiles
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: tonymec, Assigned: standard8)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

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.
Version: unspecified → Trunk
(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.
Component: General → Profile: Manager
(Assignee)

Comment 3

11 years ago
I'll take a look at this soon
Assignee: general → bugzilla
QA Contact: general → profile-manager
(Assignee)

Comment 4

11 years ago
Note to self: I probably need to implement bug 371309 at the same time as doing this.
(Assignee)

Updated

11 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

11 years ago
Depends on: 329744
(Assignee)

Comment 5

11 years ago
Created attachment 276683 [details] [diff] [review]
Port bug 371309

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.
Attachment #276683 - Flags: review?(neil)
(Assignee)

Comment 6

11 years ago
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.
Attachment #276683 - Flags: review?(neil)
(Assignee)

Comment 7

11 years ago
Moving title around so that I can find the bug easier when I look for it in my lists.
Summary: (xpfe -> suiterunner migration) Only first tab of multitab homepage is migrated → Only first tab of multitab homepage is migrated (xpfe -> suiterunner migration)
(Assignee)

Comment 8

11 years ago
Created attachment 278297 [details] [diff] [review]
Port bug 371309 v2

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.
Attachment #278297 - Flags: review?(neil)

Comment 9

11 years ago
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 10

11 years ago
Comment on attachment 278297 [details] [diff] [review]
Port bug 371309 v2

Oh, and this doesn't actually fix the bug yet does it?

Updated

11 years ago
Depends on: 393783
(Assignee)

Comment 11

11 years ago
(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.
(Assignee)

Comment 12

11 years ago
Created attachment 278397 [details] [diff] [review]
Port bug 371309 v3 (checked in)

Revised patch based on comments.
Attachment #276683 - Attachment is obsolete: true
Attachment #278297 - Attachment is obsolete: true
Attachment #278397 - Flags: review?(neil)
Attachment #278297 - Flags: review?(neil)

Comment 13

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

Comment 14

11 years ago
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.
Attachment #278397 - Flags: review?(neil) → review+
(Assignee)

Updated

11 years ago
Attachment #278397 - Attachment description: Port bug 371309 v3 → Port bug 371309 v3 (checked in)
(Assignee)

Comment 15

11 years ago
Created attachment 278405 [details] [diff] [review]
Implement the fix (checked in)

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.
Attachment #278405 - Flags: review?(neil)

Comment 16

11 years ago
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?
(Assignee)

Comment 17

11 years ago
(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);

Updated

11 years ago
Attachment #278405 - Flags: review?(neil) → review+
(Assignee)

Updated

11 years ago
Attachment #278405 - Attachment description: Implement the fix. → Implement the fix (checked in)
(Assignee)

Comment 18

11 years ago
Both patches checked in -> fixed.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.