Last Comment Bug 762639 - "Import Settings and Data" dialog show up on first run
: "Import Settings and Data" dialog show up on first run
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Migration (show other bugs)
: 14 Branch
: x86_64 Linux
: -- normal (vote)
: Firefox 16
Assigned To: Mano (::mano, needinfo? for any questions; not reading general bugmail)
:
: Matthew N. [:MattN] (PM me if requests are blocking you)
Mentors:
Depends on:
Blocks: 748569
  Show dependency treegraph
 
Reported: 2012-06-07 13:04 PDT by Mike Hommey [:glandium]
Modified: 2012-07-19 06:59 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified


Attachments
"Bring back" the migrators generator (2.84 KB, patch)
2012-06-13 16:00 PDT, Mano (::mano, needinfo? for any questions; not reading general bugmail)
mak77: review+
gavin.sharp: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2012-06-07 13:04:08 PDT
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] 2012-06-07 13:05:21 PDT
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: gavin@gavinsharp.com] 2012-06-07 13:06:34 PDT
That range corresponds to bug 748569's landing:
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=adc7d8b31160
Comment 3 Marco Bonardo [::mak] 2012-06-13 13:22:56 PDT
taking to have it on the radar, Mano if you have a fix at hand feel free to steal
Comment 4 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-06-13 16:00:00 PDT
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.
Comment 5 Marco Bonardo [::mak] 2012-06-14 02:42:09 PDT
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 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-06-14 03:20:19 PDT
(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 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-06-14 03:43:05 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/9bd4a9bf1db3
Comment 8 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-06-14 04:03:15 PDT
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] 2012-06-15 06:00:40 PDT
https://hg.mozilla.org/mozilla-central/rev/9bd4a9bf1db3
Comment 10 Alex Keybl [:akeybl] 2012-06-15 15:35:52 PDT
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 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-06-18 02:58:51 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/695003d45d7f
Comment 12 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-06-18 03:08:20 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/f0875ba9c08d
Comment 13 Virgil Dicu [:virgil] [QA] 2012-07-06 00:46:07 PDT
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] 2012-07-19 06:59:29 PDT
Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0

Verified in Firefox 15b1 on Ubuntu 12.04.

Note You need to log in before you can comment on or make changes to this bug.