Support migrating home pages and multiple versions of Firefox Start

VERIFIED FIXED

Status

()

Firefox
Migration
VERIFIED FIXED
13 years ago
7 years ago

People

(Reporter: mconnor, Assigned: mconnor)

Tracking

({late-l10n, relnote, verified1.8})

unspecified
late-l10n, relnote, verified1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8rc1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

13 years ago
In order to both facilitate more user choice and more flexibility in how we maintain Firefox Start around the world, we are taking a late change to offer users a choice between Firefox Start and the homepage from their previous browser. We are also increasing our flexibility so that Firefox Start is not necessarily locked to a single search partner.

Note to localizers: the strings that will be added should not break localizations automatically, as needed fallback code kicks in if key strings are missing.
(Assignee)

Comment 1

13 years ago
Created attachment 200558 [details] [diff] [review]
patch
Attachment #200558 - Flags: review?(vladimir)
Comment on attachment 200558 [details] [diff] [review]
patch


Please fix tab/space issues throught the patch.. there's a bunch of stray tabs throughout the files in the patch.


>Index: browser/components/migration/content/migration.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/migration/content/migration.js,v
>retrieving revision 1.22.2.2
>diff -u -p -8 -w -r1.22.2.2 migration.js
>--- browser/components/migration/content/migration.js	26 Sep 2005 02:49:21 -0000	1.22.2.2
>+++ browser/components/migration/content/migration.js	23 Oct 2005 22:32:26 -0000
>@@ -152,27 +152,33 @@ var MigrationWizard = {

>       this._wiz.currentPage.next = "selectProfile";
>     else {
>-      this._wiz.currentPage.next = (this._autoMigrate || this._bookmarks) ? "migrating" : "importItems";
>+    	if (this._autoMigrate)
>+    	  this._wiz.currentPage.next = "homePageImport";
>+    	else if (this._bookmarks)
>+        this._wiz.currentPage.next = "migrating"
>+      else
>+        this._wiz.currentPage.next = "importItems";
>+    		
>+      this._wiz.currentPage.next = (this._autoMigrate || this._bookmarks) ? "homePageImport" : "importItems";
>       var sourceProfiles = this._migrator.sourceProfiles;

Er, the new code sets this._wiz.currentPage.next, and then sets it again?  Am I missing something?  It looks like the "migrating" value of .next never happens.  Not sure what's intended here.

With those two issues fixed, r=vladimir
Attachment #200558 - Flags: review?(vladimir) → review+
(Assignee)

Updated

13 years ago
Attachment #200558 - Flags: approval1.8rc1?

Updated

13 years ago
Attachment #200558 - Flags: approval1.8rc1? → approval1.8rc1+

Updated

13 years ago
Attachment #200558 - Flags: approval1.8rc1+ → approval1.8rc1?

Updated

13 years ago
Flags: blocking1.8rc1+

Comment 3

13 years ago
Any updates on this? I can't remember from the meeting if we were waiting on another patch for this bug. 
(Assignee)

Comment 4

13 years ago
Created attachment 200705 [details] [diff] [review]
better patch

fixed a few flaws I found in testing this last night and today (multiple profiles in Opera, etc)

Also makes home page migration a part of official builds (it could be a part of unofficial builds, but those would be different strings) by adding the strings to browserconfig.properties.

This has some obtuse bulletproofing in place for setting the home page in the migration wizard, since the seamonkey migrator does weird things with the prefservice.  Very safe, well-tested, ready to land.
Attachment #200558 - Attachment is obsolete: true
Attachment #200705 - Flags: approval1.8rc1?
Attachment #200558 - Flags: approval1.8rc1?

Comment 5

13 years ago
would be great if we could get vlad to re-review the updated patch which has some changes to it after the fact tomorrow, but we should still get this in tonight. 
(Assignee)

Updated

13 years ago
Attachment #200705 - Flags: review?(vladimir)
(Assignee)

Comment 6

13 years ago
Test plan for QA:

This patch only has an effect during first-run migration, so you need to clear out the firefox profiles directory entirely to test.

Need to test that migration still works, and that both selecting Firefox Start and Import your home page from X will give the correct home page (noting, of course, that we have a first-run homepage in this case).

I will roll a test Windows build tomorrow for QA to test the multiple start options case, and I'll try to get Linux and Mac builds done by someone.

Updated

13 years ago
Attachment #200705 - Flags: approval1.8rc1? → approval1.8rc1+

Comment 7

13 years ago
You broke mac tbox.
(Assignee)

Updated

13 years ago
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
(Assignee)

Comment 8

13 years ago
known, bustage fix already in, I shouldn't have been assuming that we didn't build the useless stubs for these.

Comment 9

13 years ago
Please announce this in the l10n-NG and tell people where to put the translated file.
This works on Windows Mac and Linux for a single page homepage. But we only migrate the first page of a multi-tab group set as a home page. (from Mozilla, Safari)
reopening...  can we get group of tabbed pages migrated as well?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

13 years ago
Keywords: fixed1.8
Created attachment 200791 [details] [diff] [review]
bump the uuid of nsIBrowserProfileMigrator

The uuid of nsIBrowserProfileMigrator wasn't bumped. this has r=mconnor over irc.
Attachment #200791 - Flags: review+

Updated

13 years ago
Attachment #200791 - Flags: approval1.8rc1? → approval1.8rc1+

Comment 13

13 years ago
I bumped the IID on the branch so Chase and I can kick off another respin. Please land this on the trunk.
Keywords: fixed1.8
(Assignee)

Comment 14

13 years ago
multiple homepages is a more advanced feature, and our current method of storing these is pretty bogus, so I passed on implementing it.

Please file a followup enhancement to support this (for post 1.5).
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago13 years ago
Resolution: --- → FIXED
Probably should add a release note to indicate that multiple pages won't be migrated. Adding keyword.
Keywords: relnote
filed 313879 for the multipage issue
Status: RESOLVED → VERIFIED
Keywords: fixed1.8 → verified1.8
This bug may have caused bug 316995, bug 317033, and bug 316634. I'm basing that solely on the fact that this was one of the only patches to touch migration code on the branch, I haven't been able to test.

Comment 18

13 years ago
It'd have been nice if a new API (nsIWindowsRegKey) had been used when making a patch for the trunk. I'll upload a patch to bug 250255
Now that this has been resolved, is there any way to check to see if the user has actually deliberately set their page in their previous browser, vs. the default? If they have, it's an indication that they want to keep it. 
(Assignee)

Comment 20

13 years ago
It may be an indication they want to keep it, or it may have been overridden by a malicious install of some kind, or unwittingly via a JS confirmation.  I'm remembering how many people disliked the "always migrate" behaviour we had pre-1.0, and that was a big argument against the migration.
(In reply to comment #12)
> Created an attachment (id=200791) [edit]
> bump the uuid of nsIBrowserProfileMigrator

This was never landed on the trunk. I just landed it now.
mozilla/browser/components/migration/public/nsIBrowserProfileMigrator.idl; 1.8;
You need to log in before you can comment on or make changes to this bug.