769466 - Remove messy homepage-set code from migration.js

Status: RESOLVED INVALID

(Firefox :: Migration, defect)

Description

[Asaf Romano (gone)]

Attachment: patch

The code for setting homepage in migration.js is insanely complex. I'm not sure if it was ever needed, but given that all migrators call doStartup very early (excluding the migrator from which you cannot import homepage), and given that my testing shows everything works as expected, I think we can remove it and move on.

Comment 1

[Asaf Romano (gone)]

Comment on attachment 637691 [details] [diff] [review]
patch

Matt will review this one.

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 637691 [details] [diff] [review]
patch

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

::: browser/components/migration/content/migration.js
@@ +309,5 @@
>    onHomePageMigrationPageAdvanced: function ()
>    {
> +    let selectedItem =
> +      document.getElementById("homePageRadiogroup").selectedItem;
> +    if (selectedItem.id != "homePageSingleStart")

The old code was worried about not having a selected item.  Do we know that we are always guaranteed to have one?  If not, also check that selectedItem is truthy here: 
if (selectedItem && selectedItem.id != "homePageSingleStart")

@@ +310,5 @@
>    {
> +    let selectedItem =
> +      document.getElementById("homePageRadiogroup").selectedItem;
> +    if (selectedItem.id != "homePageSingleStart")
> +      this._newHomePage = selectedItem.value;

Nit: I think selectedHomePage would be more clear.

@@ -402,5 @@
> -            var dirSvc = Components.classes["@mozilla.org/file/directory_service;1"]
> -                                   .getService(Components.interfaces.nsIProperties);
> -            var prefFile = dirSvc.get("ProfDS", Components.interfaces.nsIFile);
> -            prefFile.append("prefs.js");
> -            prefSvc.savePrefFile(prefFile);

Why did you get rid of the savePrefFile call?  Are we guaranteed that it will be called after this (in case of an unclean shutdown)?

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Bug 1434167 made this invalid