Closed
Bug 1244064
Opened 9 years ago
Closed 9 years ago
Fix indentation of the passwords migration step in sanitize.js
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
DUPLICATE
of bug 1244908
People
(Reporter: mak, Unassigned)
References
Details
it's currently badly indented in the startup path, but it should have been moved to the onShutdown path.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1242176 for more details.
Reporter | ||
Comment 1•9 years ago
|
||
Matt, fine with this?
We're not going to touch the code, just move it. If that's fine we can make this a mentored bug.
Flags: needinfo?(MattN+bmo)
IIRC, my understanding when I touched that code is that it was put on startup to make sure that cleanup takes place even in case of crash.
Reporter | ||
Comment 3•9 years ago
|
||
It was happening on shutdown before the refactoring, so I'm not sure why we care. It would run again at the next shutdown, and if we crash at every shutdown, there are worse problems than a passwords clear migration.
Btw, if we want to retain it on startup we should at least fix the indentation.
Ok, then my memory is just foggy.
But yes, I think that cleanup for privacy reasons should (also) take place on startup, not just passwords.
Comment 5•9 years ago
|
||
It's supposed to take place on startup (upon upgrading to Fx42 originally) as that's when the feature is removed. I think it makes sense to cleanup the old pref and do the migration on startup like we do in migrateUI. Note that it also does: Services.prefs.setBoolPref("signon.rememberSignons", false);
I've never seen other migrations done on shutdown so I think it's fine on startup.
Flags: needinfo?(MattN+bmo)
Reporter | ||
Comment 6•9 years ago
|
||
yeah, it's mostly strange cause even if it's on startup it depends on a shutdown pref... Btw, let's just fix indentation then.
Summary: Move the passwords migration step to Sanitize::onShutdown → Fix indentation of the passwords migration step in sanitize.js
Comment 7•9 years ago
|
||
Sorry, I forgot about this bug when I filed bug 1244908. Can we dupe or do we care about fixing this on m-a and m-b?
See Also: → 1244908
Reporter | ||
Comment 8•9 years ago
|
||
Sure, fine by me, I didn't know you were planning to remove the migration already.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•