748569 - Fix (most) remaining migration bugs for Firefox 14

Status: VERIFIED FIXED

(Firefox :: Migration, defect)

Description

[Asaf Romano (gone)]

Combo-patch for remaining migration issues is in the pipe, not too big, not to small. Bugs which are already filed will be blocked by this bug.

Here's a list of user-facing bugs I'm going to fix here:
 * [Major] Bug 738263 - Default bookmarks.html import on migration is temporarily broken ("planned" regression)
 * [Major] Bug 748047 - Reset profile feature is broken (regression)
 * Migration wizard fails to recognize default browser on Windows vista and window 7. This is a long standing issue, but I don't think it's filed.
 * On Mac and Linux we don't even bother detecting the default browser.
 * Detection of default browser on Windows doesn't check for safari.
 * The wizard lies (and doesn't tell all the truth) about the data that is about to be migrated:
   "Import Options, Bookmarks, History, Passwords and other data from:"
    ...Chrome, Safari
 * Internet Explorer becomes _Microsoft_ Internet Explorer in the homepage wizard-page.

Implementation details to be fixed here:
  * Simplify migration wizard modes.
  * Remove the puzzling code for saving the homepage pref.
  * Build the list of migrators in the wizard dynamically. As a result, quite a few duplicated strings are noew removed from migration.dtd.
  * nsIBrowserProfileMigrator: do away with the startup arguments.
  * slow decom: getMigrator now returns the wrappedJSObject of each migrator.
  * Remove some leftovers of "From File" mode.
  * When the default browser is detected on windows, the reg-key was never closed (my bad). The new code does not rely on registry keys.

The following issues will _not_ be covered by this bug:
  * Chrome migrator reports that migration is done too early (not yet filed)
  * Safari migrator does not import cookies and top sites (to be done in other bugs. We may or may land these for Firefox 14)
  * about:home is still featured as "Firefox Start, a fast home page with built-in search"
  * The migration wizard interaction is worse than Linux.
  * It's rocket science to test the homepage page on builds with non-official branding.

Comment 1

[Asaf Romano (gone)]

* I'm also not going to fix the issues regarding import from chrome not really working if chrome is open during migration.

Comment 2

[Asaf Romano (gone)]

For the API I'm going to use for retrieving the default browser ( nsIExternalProtocolService.getApplicationDescription), here are the results on the various platforms this needs to support:

Windows 7 (I haven't checked Vista, but it should be the same):
Internet Explorer
Google Chrome
Safari

Windows XP: same same, despite the completely different implementation. Hooray! 

OS X:
Chrome
Safari

Linux is not mentioned here, not because of me or my opinions, but because there's only one browser from which we can import there. However, I did notice that, unlike all other platforms, this method returns "Firefox Web Browser" if Firefox is set as the default browser. Strange.

Comment 3

[Asaf Romano (gone)]

Btw, on windows, all of this is somewhat pointless because the *installation* wizard defaults to setting Firefox to be the default browser, meaning that the data is already lost when the migration wizard shows up.

Comment 4

[Asaf Romano (gone)]

Attachment: trunk patch

For Aurora I'll avoid the changes to migration.properties.

Comment 5

[Asaf Romano (gone)]

For the record, it's "google-chrome" on Linux. I'll add that to the patch, but it doesn't really change anything as Chrome is the only other browser.

Comment 6

[Marco Bonardo [:mak]]

(In reply to Mano from comment #5)
> For the record, it's "google-chrome" on Linux. I'll add that to the patch,
> but it doesn't really change anything as Chrome is the only other browser.

I wonder if this stays valid for the different distributions... maybe we may ask paolo and tim to check, since they use different distro (Ubuntu and Mint I think).

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 618453 [details] [diff] [review]
trunk patch

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

As previously discussed in the office, needs some diet, but globally looks fine

::: browser/components/migration/content/migration.js
@@ +33,5 @@
>  
> +    if ("arguments" in window &&
> +        MigratorPrototype.isPrototypeOf(window.arguments[0])) {
> +      this._migrator = window.arguments[0];
> +      this._migratorInternalName = this._migrator.internalName;

no need for this further caching, directly use this._migrator.internalName

@@ +66,4 @@
>      // Figure out what source apps are are available to import from:
> +    let group = document.getElementById("importSourceGroup");
> +    let nothingItem = document.getElementById("nothing");
> +    for (let migrator in MigrationUtils.migrators) {

"in" looks a bit confusing here, can we use "of" or "for..each..in" on this kind of generator?

@@ +75,3 @@
>      }
>  
> +    group.selectedItem = group.firstChild;

Add a comment about the .migrators ordering and why this just works

::: browser/components/migration/public/nsIBrowserProfileMigrator.idl
@@ +70,5 @@
>     * @param   aDoingStartup "true" if the profile is not currently being used.
>     * @return  bit field containing profile items (see above)
>     * @note    a return value of 0 represents no items rather than ALL.
>     */
> +  unsigned short getMigrateData(in wstring aProfile);

these changes have to go away for the Aurora version of the patch

::: browser/components/migration/src/ChromeProfileMigrator.js
@@ +340,5 @@
> +  "internalName": { value: "chrome" },
> +  "classDescription": { value: "Chrome Profile Migrator" },
> +  "contractID": { value: "@mozilla.org/profile/migrator;1?app=browser&type=chrome" },
> +  "classID": { value: Components.ID("{4cec1de4-1671-4fc3-a53e-6c539dc77a26}") }
> +});

While these changes make technically sense, they are not needed and inflate the patch quite a bit. I don't think we need to care about external users overwriting these values, that would be just dumb to do :) Just add .internalName to the prototype, as discussed

::: browser/components/migration/src/MigrationUtils.jsm
@@ +41,5 @@
> +      "Internet Explorer": "ie",
> +      "Safari": "safari",
> +      "Google Chrome": "chrome",
> +      "Chrome": "chrome",
> +    };

Add a comment about chrome being the only thing we import on Linux and thus not caring remapping it.

@@ +182,5 @@
>  
>      if (aItems != Ci.nsIBrowserProfileMigrator.ALL)
>        resources = [r for each (r in resources) if (aItems & r.type)];
>  
> +    // Called either directly or through the import-bookmarks-callback.

s/import-bookmarks-callback/import-default-bookmarks callback/

@@ +185,5 @@
>  
> +    // Called either directly or through the import-bookmarks-callback.
> +    function doMigrate() {
> +      // TODO: use Map (for the items) and Set (for the resources)
> +      // once they are iterable.

bug# plz

@@ +186,5 @@
> +    // Called either directly or through the import-bookmarks-callback.
> +    function doMigrate() {
> +      // TODO: use Map (for the items) and Set (for the resources)
> +      // once they are iterable.
> +      let resourcesGroupedByItems = new Dict();

hm, these look grouped by ResourceType, so I'd prefer resourcesGroupedByType, items is kinda too generic (we use it for a bunch of stuff)

@@ +187,5 @@
> +    function doMigrate() {
> +      // TODO: use Map (for the items) and Set (for the resources)
> +      // once they are iterable.
> +      let resourcesGroupedByItems = new Dict();
> +      resources.forEach(function(resource) {

hm, for..of? unless there is a specific need for a callback, I guess it may be more gc efficient and readable (no, I don't have numbers, so whatever you think is nicer). This happens later too.

@@ +204,5 @@
> +
> +      notify("Migration:Started");
> +      resourcesGroupedByItems.listkeys().forEach(function(migrationType) {
> +        let migrationTypeA = migrationType;
> +        let itemResources = resourcesGroupedByItems.get(migrationType);

nit: just resources, imo

@@ +247,5 @@
> +      if (resources.some(function(r)
> +                         r.type == MigrationUtils.resourceTypes.BOOKMARKS)) {
> +        // Import default-bookmarks, unless it's the Firefox migrator, which copies
> +        // an existent places file.
> +        if (!this.startupOnlyMigrator) {

the logic here looks confusing, I'd be fine with having some minor code duplication (or have a notifyGlue helper) if we could better split the startupOnlyMigrator case from the default bookmarks import one.

@@ +262,5 @@
>  
> +        // Either way notify glue that it does not need to import bookmarks on its
> +        // own.
> +        Cc["@mozilla.org/browser/browserglue;1"].
> +        getService(Ci.nsIObserver).observe(null, "initial-migration", null);

nit: align .getService to the opening square brace (so that . is below [)

@@ +453,5 @@
>      else {
>        try {
>          migrator = Cc["@mozilla.org/profile/migrator;1?app=browser&type=" +
> +                      aKey].createInstance(Ci.nsIBrowserProfileMigrator)
> +                           .wrappedJSObject;

nit: move the prefix to a const and get getter indentation?

@@ +464,5 @@
>    },
>  
> +  // Iterates the available migrators.
> +  // The migration wizard relies on the order of migrators yielded.
> +  get migrators() {

Use a javadoc, please.
add a note on why this is not a plain array (the perf stuff and such)

@@ +557,5 @@
> +        this.migrators.next();
> +        gProfileStartup = aStartup;
> +        this.showMigrationWizard();
> +      }
> +      catch(ex) { }

you should only catch the end of iterator exception I think

::: browser/locales/en-US/chrome/browser/migration/migration.dtd
@@ +6,2 @@
>  <!ENTITY importFromNothing.label        "Don't import anything">
>  <!ENTITY importFromNothing.accesskey    "D">

May the accesskey removal hurt accessibility? Having only Nothing with an accesskey sounds a bit strange and we usually provide accesskeys on radio. Not that I care that much a user can quickly jump to an option in this rarely used dialog, but sounds sort of a regression, so we must be sure we figure the downsides.

::: browser/locales/en-US/chrome/browser/migration/migration.properties
@@ +3,5 @@
>  # Browser Specific
> +ie=Internet Explorer
> +safari=Safari
> +chrome=Google Chrome
> +firefox=Mozilla Firefox

String changes should not be part of the Aurora patch. Removals can be.
Btw, out of curiosity, why half of the browsers have the brand in front of it and other half doesn't?

Comment 8

[Matthew N. [:MattN]]

Requesting tracking-firefox14 because it will fix bug 748047 which is tracking-firefox14+.

Comment 9

[Marco Bonardo [:mak]]

Mano, could you please prioritize this fix? Next week I'll be partially busy at the JS Day, so would be better to have this ready for review sooner.

Comment 10

[Justin Dolske [:Dolske]]

This is blocking bug 748047, which _needs_ to have an Aurora resolution.

This is a rather sizable patch -- is there a a simpler workaround we can take to fix profile-reset? Landing a big patch like this on Aurora will raise some eyebrows (though, in fairness, it's all migration code which isn't likely to get much testing on the non-release channels anyway).

Comment 11

[Asaf Romano (gone)]

Attachment: minimal patch...

Going to test this quite hard in the next few hours...

Comment 12

[Asaf Romano (gone)]

Attachment: diff -w

Comment 13

[Asaf Romano (gone)]

This fixes bug 738263, bug 748047 and the broken default-browser detection. It does not address anything else from comment 0. I'll take care of that in a follow up.

Comment 14

[Marco Bonardo [:mak]]

Comment on attachment 624342 [details] [diff] [review]
diff -w

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

some typos to fix yet and tests are broken, but the approach looks good, and the patch is much smaller

::: browser/components/migration/content/migration.xul
@@ +63,5 @@
>  #endif
>      <description id="importBookmarks" control="importSourceGroup" hidden="true">&importFromBookmarks.label;</description>
>  
>      <radiogroup id="importSourceGroup" align="start">
> +# Caution  See https://developer.mozilla.org/en/Build/Text_Preprocessor#Conditionals

May you be more verbose about what we should care about in this specific case? Not having multiple elifdef?

@@ +64,5 @@
>      <description id="importBookmarks" control="importSourceGroup" hidden="true">&importFromBookmarks.label;</description>
>  
>      <radiogroup id="importSourceGroup" align="start">
> +# Caution  See https://developer.mozilla.org/en/Build/Text_Preprocessor#Conditionals
> +#ifdef XP_WIM

hm, doubt WIM works... typo

::: browser/components/migration/src/MigrationUtils.jsm
@@ +35,5 @@
>  }
>  
> +function getMigratorKeyForDefaultBrowser() {
> +  const APP_DESC_TO_KEY = {
> +    "Internet Explorer": "ie",

please add a comment on where to find the right mapping strings in the codebase.

@@ +45,5 @@
> +  try {
> +    return APP_DESC_TO_KEY[
> +      Cc["@mozilla.org/uriloader/external-protocol-service;1"].
> +      getService(Ci.nsIExternalProtocolService).
> +      getApplicationDescription("http")] || "";

make a temp var for the key please, should be more readable and less error prone than this oneline

@@ +177,5 @@
>  
>      if (aItems != Ci.nsIBrowserProfileMigrator.ALL)
>        resources = [r for each (r in resources) if (aItems & r.type)];
>  
> +    // Called either directly or through the import-bookmarks-callback.

nit: remove dashes

@@ +247,5 @@
> +                         r.type == MigrationUtils.resourceTypes.BOOKMARKS)) {
> +        let browserGlue = Cc["@mozilla.org/browser/browserglue;1"].
> +                          getService(Ci.nsIObserver);
> +
> +        // XXXmano: very-temporary hack, we want to pass the migrator internal name

you know, XXX... bug please :)

@@ +262,5 @@
> +
> +            // Note doMigrate doesn't care about the success value of the
> +            // callback.
> +            BookmarkHTMLUtils.importFromURL(
> +              NetUtil.newURI(bookmarksHTMLFile).spec, true, function() {

looks like you have 2 calls to newURI, maybe you may directly have an url var and oneline this

@@ +274,5 @@
> +        }
> +      }
> +      else if (!this.startupOnlyMigrator) {
> +        MigrationUtils.profileStartup.doStartup();
> +      }

This indented ifs/elseifs thing is quite unreadable.
May we move this above as an if (!this._startupOnlyMigrator) (before the "if (resources"...) instead of handling it later? I'm not quite sure, I think I'm going to file a Splinter bug to have indentation guides in reviews.

@@ +491,5 @@
>     * @param [optional] aOpener
> +   *        the window that asks to open the wizard.
> +   * @param [optioanl] aParams
> +   *        arguments for the migration wizard, in the form of an nsIArray.
> +   *        @see nsIWindowWatcher.

align @see with @param

@@ +503,5 @@
>        if (win) {
>          win.focus();
>          return;
>        }
> +      // On mac, the migration wiazrd should only be modal in the case of

typo: wiazrd

@@ +520,5 @@
> +  /**
> +   * Show the migration wizard for startup-migration.
> +   *
> +   * @param aProfileStartup
> +   *        @see nsIProfileMigrator.

nit: well, @see should be elsewhere, but whatever

@@ +525,5 @@
> +   * @param [optional] aMigratorKey
> +   *        If set, the migration wizard will import from the corresponding
> +   *        migrator, bypassing the source-selection page.
> +   * @throws if aMigratorKey is invalid or if it points to a non-existent
> +   *         source.

so, it's sort of unclear what happens if aMigratorKey is not passed-into

@@ +550,4 @@
>      }
>      else {
> +      let defaultBrowserKey = getMigratorKeyForDefaultBrowser();
> +      if (defaultBrowserKey) {

May this be empty ever? how do we proceed from an empty result, doesn't look like we manage it in any special way?

::: browser/components/nsBrowserGlue.js
@@ +127,5 @@
>    _isPlacesInitObserver: false,
>    _isPlacesLockedObserver: false,
>    _isPlacesShutdownObserver: false,
>    _isPlacesDatabaseLocked: false,
> +  _migrationImportsDefulatBookmarks: false,

typo: importDefulat

@@ +225,5 @@
>          subject.QueryInterface(Ci.nsISupportsPRBool);
>          subject.data = true;
>          break;
>        case "places-init-complete":
> +        if (!this._migrationImportsDefulatBookmarks)

and here, as well

@@ +274,5 @@
>          else if (data == "force-places-init") {
>            this._initPlaces();
>          }
>          break;
> +      case "initial-migration-will-import-default-bookmarks":

you cannot change the "initial-migration" topic without changing all the code using it, there are some tests

http://mxr.mozilla.org/mozilla-central/search?string=initial-migration

@@ +275,5 @@
>            this._initPlaces();
>          }
>          break;
> +      case "initial-migration-will-import-default-bookmarks":
> +        this._migrationImportsDefulatBookmarks = true;

and so on

@@ +282,5 @@
> +        // |data| is set to the migrator-key.  If it's "firefox", it means
> +        // this import was a profile-reset, in which case we don't need to
> +        // re-initialize anything.
> +        if (data != "firefox")
> +          this._initPlaces();

just in case, since this slightly changes the order of things, I suggest doing a tryrun to ensure import tests pass still and don't need additional changes.

Comment 15

[Asaf Romano (gone)]

Attachment: ready for another pass

The tests aren't fixed yet, and so is the #ifdef comment. I'll address these next. Anyway, this is ready for another review, I think.

Comment 16

[Asaf Romano (gone)]

Attachment: diff -w

Comment 17

[Asaf Romano (gone)]

I forgot to explain why/how I could simplify this code as much in this revision: While reading your review, I realized that we don't need to do any extra handling for the Firefox-migrator case, because places-init isn't expected to be fired if a working places db was copied. Thus, we only need to fire initial-migration-* for "non-startup" migrators.

Comment 18

[Marco Bonardo [:mak]]

Comment on attachment 625052 [details] [diff] [review]
diff -w

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

so, looks ok, remember to address comment 15.
We will need QA attention on this.

::: browser/components/migration/content/migration.js
@@ +32,5 @@
>      this._wiz = document.documentElement;
>  
>      if ("arguments" in window && window.arguments.length > 1) {
>        this._source = window.arguments[0];
> +      this._migrator = window.arguments[1] ? window.arguments[1].QueryInterface(kIMig) : null;

I wonder if wouldn't be better to use "instanceof kIMig" as a test

::: browser/components/migration/src/MigrationUtils.jsm
@@ +63,5 @@
> +  catch(ex) {
> +    Cu.reportError("Could not detect default browser: " + ex);
> +  }
> +  return browserDesc in APP_DESC_TO_KEY ?
> +         APP_DESC_TO_KEY[browserDesc] : "";

well in my previous comment I actually meant just:
try {
  let browserDesc = ...
  return APP_DESC_TO_KEY[browserDesc]
} catch (ex) {
  Cu.re
}
return ""

@@ +257,5 @@
> +      // Either way, we notify browser-glue that it does not need to import the
> +      // default bookmarks on its own.
> +      let migratingBookmarks =
> +        resources.some(function(r)
> +                       r.type == MigrationUtils.resourceTypes.BOOKMARKS);

const BOOKMARKS = MigrationUtils.resourceTypes.BOOKMARKS;
let migratingBookmarks = resources.some(function (r) r.type == BOOKMARKS);

@@ +262,5 @@
> +      if (migratingBookmarks && !this.startupOnlyMigrator) {
> +        let browserGlue = Cc["@mozilla.org/browser/browserglue;1"].
> +                          getService(Ci.nsIObserver);
> +        browserGlue.observe(null,
> +          "initial-migration-will-import-default-bookmarks", "");

may move these to TOPIC_WILL_IMPORT_DEFAULT_BOOKMARKS and TOPIC_DID_IMPORT_DEFAULT_BOOKMARKS consts and oneline this.

@@ +273,5 @@
> +          // callback.
> +          BookmarkHTMLUtils.importFromURL(
> +            NetUtil.newURI(bookmarksHTMLFile).spec, true, function() {
> +              browserGlue.observe(null,
> +                "initial-migration-did-import-default-bookmarks", "");

may oneline with const, as said

@@ +281,5 @@
> +        }
> +      }
> +      else if (!this.startupOnlyMigrator) {
> +        MigrationUtils.profileStartup.doStartup();
> +      }

so you do

if (migratingBookmarks && !this.startupOnlyMigrator) {
 ...
 MigrationUtils.profileStartup.doStartup()
}
else if (!this.startupOnlyMigrator) {
  MigrationUtils.profileStartup.doStartup();
}

why can't we do

if (!this.startupOnlyMigrator) {
  MigrationUtils.profileStartup.doStartup();
}
if (migratingBookmarks) {
  ...
}

I don't think the time we notify glue matters compared to doStartup()

Comment 19

[Marco Bonardo [:mak]]

please file a bug covering what has been removed from this patch, since the status atm is a bit unclear.

Comment 20

[Asaf Romano (gone)]

Attachment: patch as landed

http://hg.mozilla.org/integration/mozilla-inbound/rev/852aa11f1cbf

Comment 21

[Asaf Romano (gone)]

Attachment: diff -w

Comment 22

[Asaf Romano (gone)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/852aa11f1cbf

Comment 23

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/852aa11f1cbf

Comment 24

[Justin Dolske [:Dolske]]

Woo. Thanks for final push on getting this done, Mano! :)

Comment 25

[Asaf Romano (gone)]

Comment on attachment 627644 [details] [diff] [review]
patch as landed

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new migration system
User impact if declined: default bookmarks (Getting Started, Mozilla Firefox folder), along with smart queries (Recently Bookmarked, Recent Tags), won't be imported for new users, default profile feature is broken, the default browser won't be detected in the migration wizard (the last being a long standing regression though).
Risk to taking this patch (and alternatives if risky): this is as small as it can get, so there are not any alternatives. The patch is pretty much self-contained to the migration component, thus possible back-out could be easy.
String or UUID changes made by this patch: none
Testing completed (on m-c, etc.): There are no automated tests for migration. I tested all the use cases I know of (but only on OS X). The migration component in general needs a lot of attention for Fx 14 given that the it was almost completely rewritten during this development cycle.

Comment 26

[Alex Keybl [:akeybl]]

Comment on attachment 627644 [details] [diff] [review]
patch as landed

[Triage Comment]
We discussed with QA over mail. We don't expect to get significant user testing externally, so we'll uplift this and have QA do targeted internal testing prior to release.

Approved for Aurora 14.

Comment 27

[Marco Bonardo [:mak]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/adc7d8b31160

Comment 28

[Paul Silaghi, QA [:pauly]]

(In reply to Mano from comment #0)
> Combo-patch for remaining migration issues is in the pipe, not too big, not
> to small. Bugs which are already filed will be blocked by this bug.
> 
> Here's a list of user-facing bugs I'm going to fix here:
>  * [Major] Bug 738263 - Default bookmarks.html import on migration is
> temporarily broken ("planned" regression) 
NOT FIXED

>  * [Major] Bug 748047 - Reset profile feature is broken (regression)
VERIFIED

>  * Migration wizard fails to recognize default browser on Windows vista and
> window 7. This is a long standing issue, but I don't think it's filed.
>  * On Mac and Linux we don't even bother detecting the default browser.
>  * Detection of default browser on Windows doesn't check for safari.
VERIFIED - BUG 262326, BUG 420938

>  * The wizard lies (and doesn't tell all the truth) about the data that is
> about to be migrated:
>    "Import Options, Bookmarks, History, Passwords and other data from:"
>     ...Chrome, Safari
HOW TO TEST THIS ?

>  * Internet Explorer becomes _Microsoft_ Internet Explorer in the homepage
> wizard-page.
NOT FIXED

Comment 29

[Asaf Romano (gone)]

(In reply to Paul Silaghi [QA] from comment #28)
> (In reply to Mano from comment #0)
> > Combo-patch for remaining migration issues is in the pipe, not too big, not
> > to small. Bugs which are already filed will be blocked by this bug.
> > 
> > Here's a list of user-facing bugs I'm going to fix here:
> >  * [Major] Bug 738263 - Default bookmarks.html import on migration is
> > temporarily broken ("planned" regression) 
> NOT FIXED
> 

Thanks a ton for reporting that. Discussion + patch are on that bug.

> >  * [Major] Bug 748047 - Reset profile feature is broken (regression)
> VERIFIED
> 
> >  * Migration wizard fails to recognize default browser on Windows vista and
> > window 7. This is a long standing issue, but I don't think it's filed.
> >  * On Mac and Linux we don't even bother detecting the default browser.
> >  * Detection of default browser on Windows doesn't check for safari.
> VERIFIED - BUG 262326, BUG 420938
>

Great :)
 
> >  * The wizard lies (and doesn't tell all the truth) about the data that is
> > about to be migrated:
> >    "Import Options, Bookmarks, History, Passwords and other data from:"
> >     ...Chrome, Safari
> HOW TO TEST THIS ?
> 

That's postponed for now (it was fixed in the original patch, but we wanted to keep this bug for major issues).

> >  * Internet Explorer becomes _Microsoft_ Internet Explorer in the homepage
> > wizard-page.
> NOT FIXED

Ditto.

Comment 30

[Paul Silaghi, QA [:pauly]]

Bug 738263 - Default bookmarks.html import on migration is temporarily broken - is now VERIFIED

So there are 2 more issues to fix:
* The wizard lies (and doesn't tell all the truth) about the data that is about to be migrated:
   "Import Options, Bookmarks, History, Passwords and other data from:"
    ...Chrome, Safari
 * Internet Explorer becomes _Microsoft_ Internet Explorer in the homepage wizard-page.

which are postponed for now based on comment 29.

Given that, can this bug be set on VERIFIED?

Comment 31

[Marco Bonardo [:mak]]

yes, sounds good.