Closed Bug 769466 Opened 10 years ago Closed 5 years ago

Remove messy homepage-set code from migration.js

Categories

(Firefox :: Migration, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: asaf, Assigned: asaf)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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)
Blocks: 768814
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
Closed: 5 years ago
Depends on: 1434167
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.