1276705 - Consider removing (all?) settings migration code from IE and Safari migrators

Status: RESOLVED FIXED

(Firefox :: Migration, defect)

Description

[:Gijs (he/him)]

When we last added a migrator (Edge) we decided not to import any settings. Likewise, for the automigration, we're likely not going to import any settings either per both my & Verdi's opinions on their usefulness.

Looking at the Safari and IE migrators, it is unclear that the settings they import are particularly worth keeping (e.g. cookie policy, whether we display images, whether we block popups...). Almost all of them seem like ones for which we should just use the (Firefox) default. Simplifying the code by removing these things would make our migration story a bit simpler and remove a source of errors (of the imports that report errors for IE on 46, over half are related to settings: https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2016-05-19&keys=ie&max_channel_version=release%252F46&measure=FX_MIGRATION_ERRORS&min_channel_version=null&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2016-02-15&table=0&trim=0&use_submission_date=0 ).

Dolske/Verdi, sound good or am I missing something?

Comment 1

[Justin Dolske [:Dolske]]

Nuke them with fire. I know I've commented in other bugs along these lines, but looking (again) at the migrated settings I agree they're all pretty fringe (and in some cases things we removed from our own UI due to footgun potential) -- thus not worth retaining. I further think it's reasonable to expect that someone who has spent the time hyper-customizing their previous browser is likely going to spent time going through Firefox settings anyway.

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1276705 - rm IE/safari settings importers,

Review commit: https://reviewboard.mozilla.org/r/92906/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92906/

Comment 3

[Justin Dolske [:Dolske]]

Comment on attachment 8810627 [details]
Bug 1276705 - rm IE/safari settings importers,

https://reviewboard.mozilla.org/r/92906/#review93306

r+ assuming the additional code removal is trivial.

::: browser/components/migration/IEProfileMigrator.js
(Diff revision 1)
>      }
>      return results;
>    },
>  };
>  
> -function Settings() {

Out of curiousity, (and just FTR), I did some poking to see what originally added this stuff, and it basically dates back to CVS days 10+ years ago. My curiousity doesn't extend further back than that, as I don't think it's really relevant now. :)

::: browser/components/migration/SafariProfileMigrator.js
(Diff revision 1)
>  
> -function Preferences(aMainPreferencesPropertyListInstance) {
> -  this._mainPreferencesPropertyList = aMainPreferencesPropertyListInstance;
> -}
> -Preferences.prototype = {
> -  type: MigrationUtils.resourceTypes.SETTINGS,

There's a bit more to remove from the Safari migrator -- there's a separate WebFoundationCookieBehavior thingie that's importing a cookie setting. Seems like that should go too.

Comment 4

[:Gijs (he/him)]

Comment on attachment 8810627 [details]
Bug 1276705 - rm IE/safari settings importers,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/92906/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0027b2bfafaf
rm IE/safari settings importers, r=Dolske

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0027b2bfafaf

Comment 7

[Guilherme Lima]

Should bug 745853, bug 260274 and bug 1233694 get marked as wontfix?

Comment 8

[:Gijs (he/him)]

(In reply to Guilherme Lima from comment #7)
> Should bug 745853, bug 260274 and bug 1233694 get marked as wontfix?

I missed this comment somehow, sorry. I marked these bugs as WONTFIX. Thanks!