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

Status: RESOLVED FIXED

(Firefox :: Migration, defect)

Description

[Mike Hommey [:glandium]]

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.

Comment 1

[Mike Hommey [:glandium]]

Corresponding changesets:
89176b6d643c for the last one that does *not* do it.
540694f5df42 for the first one that does it.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

That range corresponds to bug 748569's landing:
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=adc7d8b31160

Comment 3

[Marco Bonardo [:mak]]

taking to have it on the radar, Mano if you have a fix at hand feel free to steal

Comment 4

[Asaf Romano (gone)]

Attachment: "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.

Comment 5

[Marco Bonardo [:mak]]

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.

Comment 6

[Asaf Romano (gone)]

(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.

Comment 7

[Asaf Romano (gone)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/9bd4a9bf1db3

Comment 8

[Asaf Romano (gone)]

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

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9bd4a9bf1db3

Comment 10

[Alex Keybl [:akeybl]]

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.

Comment 11

[Asaf Romano (gone)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/695003d45d7f

Comment 12

[Asaf Romano (gone)]

http://hg.mozilla.org/releases/mozilla-beta/rev/f0875ba9c08d

Comment 13

[Virgil Dicu [:virgil] [QA]]

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).

Comment 14

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0

Verified in Firefox 15b1 on Ubuntu 12.04.