Remove messy homepage-set code from migration.js

RESOLVED INVALID

Status

()

RESOLVED INVALID
7 years ago
a year ago

People

(Reporter: mano, Assigned: mano)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 637691 [details] [diff] [review]
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.
Attachment #637691 - Flags: review?(mak77)
Comment on attachment 637691 [details] [diff] [review]
patch

Matt will review this one.
Attachment #637691 - Flags: review?(mak77) → review?(mnoorenberghe+bmo)
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)?
Attachment #637691 - Flags: review?(mnoorenberghe+bmo) → feedback+
Bug 1434167 made this invalid
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Depends on: 1434167
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.