"Import Settings and Data" dialog show up on first run

RESOLVED FIXED in Firefox 14

Status

()

Firefox
Migration
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: mano)

Tracking

({regression})

14 Branch
Firefox 16
x86_64
Linux
regression
Points:
---

Firefox Tracking Flags

(firefox14+ verified, firefox15+ verified)

Details

Attachments

(1 attachment)

The "Import Settings and Data" dialog shows up on first run when there is no profile, with the following content:
"o Don't import anything
No programs that contain bookmarks, history or password data could be found."

I noticed this on 14.0b6, but traced it with nightlies:
This is the last one that does *not* do it:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-05-31-04-20-08-mozilla-aurora/firefox-14.0a2.en-US.linux-x86_64.tar.bz2

This is the first one that does it:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-06-01-04-20-08-mozilla-aurora/firefox-14.0a2.en-US.linux-x86_64.tar.bz2

I presume this happens on all platforms, but I only witnessed it on Linux x86-64.
Corresponding changesets:
89176b6d643c for the last one that does *not* do it.
540694f5df42 for the first one that does it.
That range corresponds to bug 748569's landing:
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=adc7d8b31160
tracking-firefox14: --- → +
tracking-firefox15: --- → +
taking to have it on the radar, Mano if you have a fix at hand feel free to steal
Assignee: nobody → mak77
Created attachment 632930 [details] [diff] [review]
"Bring back" the migrators generator

In the Original Patch I introduced this migrators-generator in order to both handle the issue raised here and generate the wizard radio buttons dynamically. Later on, as part of the diet, I fall back to building the wizard contents "statically", so I removed the generator I've introduced, not considering that it had this other purpose...

This bring back the generator, and, for now, it's used only for determining if there's any migrator available. Of course, it's platform-specific behavior isn't necessary *yet*, but it cannot hurt anyone. I'll use this new functionality on trunk sometime soon.
Assignee: mak77 → mano
Attachment #632930 - Flags: review?(mak77)
Comment on attachment 632930 [details] [diff] [review]
"Bring back" the migrators generator

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

So basically this happens only on Linux when chrome is not installed, or another platform where we don't support any browser source. OSX and Win always have a default browser we import from.

We will need some QA on this.

::: browser/components/migration/src/MigrationUtils.jsm
@@ +491,5 @@
> +    // If a supported default browser is found check it first
> +    // so that the wizard defaults to import from that browser.
> +    let defaultBrowserKey = getMigratorKeyForDefaultBrowser();
> +    if (defaultBrowserKey)
> +      migratorsKeysOrdered.sort(function(a, b) b == defaultBrowserKey ? 1 : 0);

nit: space after function for anonymous

@@ +597,5 @@
> +    if (!migrator) {
> +      // If there's no migrator set so far, ensure that there is at least one
> +      // migrator available before opening the wizard.
> +      try {
> +        this.migrators.next();

Btw, if I read this correctly, if there is no default browser set, but there's a supported browser installed, we proceed passing null migrator and empty string migratorKey, so that the dialog just selects the first source available.

Probably on trunk, once we can get migratorKey from the migrator itself, we may remove the previous else and just use this to set migrator and migratorKey, since migrators has default browser as the first one.  Though for aurora/beta this is fine.
Attachment #632930 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #5)
> Comment on attachment 632930 [details] [diff] [review]
> "Bring back" the migrators generator
> 
> Review of attachment 632930 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So basically this happens only on Linux when chrome is not installed, or
> another platform where we don't support any browser source. OSX and Win
> always have a default browser we import from.
> 

Linux is the only expected case, right, but, in theory, this could also apply to OS X (If you never open Safari and copy Fx from a disk image). Indeed it cannot happen on Windows, because the IE migrator doesn't know saying no.

> @@ +597,5 @@
> > +    if (!migrator) {
> > +      // If there's no migrator set so far, ensure that there is at least one
> > +      // migrator available before opening the wizard.
> > +      try {
> > +        this.migrators.next();
> 
> Btw, if I read this correctly, if there is no default browser set, but
> there's a supported browser installed, we proceed passing null migrator and
> empty string migratorKey, so that the dialog just selects the first source
> available.
> 
> Probably on trunk, once we can get migratorKey from the migrator itself, we
> may remove the previous else and just use this to set migrator and
> migratorKey, since migrators has default browser as the first one.  Though
> for aurora/beta this is fine.

Indeed, that's the plan for trunk.
http://hg.mozilla.org/integration/mozilla-inbound/rev/9bd4a9bf1db3
Comment on attachment 632930 [details] [diff] [review]
"Bring back" the migrators generator

[Approval Request Comment]
Bug caused by (feature/regressing bug #):  bug 748569
User impact if declined:
1) Linux: If chrome is not installed, a useless (though functional) wizard will show up in the first startup.
2) Mac: If Firefox is started in a user account on which Safari was never opened, the same issue would raise.
3) Windows: not an issue.
Testing completed (on m-c, etc.): there are no automated tests for migration.
Risk to taking this patch (and alternatives if risky): In theory, If I did something *very* wrong, the migration wizard wouldn't show up when it should be.
Possible backout is very easy as the patch only touches the MigrationUtils module.
String or UUID changes made by this patch: none
Attachment #632930 - Flags: approval-mozilla-beta?
Attachment #632930 - Flags: approval-mozilla-aurora?
Blocks: 748569
Keywords: regression

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/9bd4a9bf1db3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Attachment #632930 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 632930 [details] [diff] [review]
"Bring back" the migrators generator

[Triage Comment]
Sounds like a low risk fix for the most part. Let's land before our next beta on Tuesday.
Attachment #632930 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
http://hg.mozilla.org/releases/mozilla-aurora/rev/695003d45d7f
status-firefox15: --- → fixed
http://hg.mozilla.org/releases/mozilla-beta/rev/f0875ba9c08d
status-firefox14: --- → fixed
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0

Verified in Ubuntu 12.04, Firefox 14b10 with Chrome profile deleted. 

When no Firefox profile present on the System:
-no import dialog displayed when chrome profile is deleted.
-import dialog displayed as expected and functional when a Chrome profile was previously used (both import from chrome and don't import working as expected).
status-firefox14: fixed → verified
Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0

Verified in Firefox 15b1 on Ubuntu 12.04.
status-firefox15: fixed → verified
You need to log in before you can comment on or make changes to this bug.