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)

defect
Not set
normal

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.
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.
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.
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)
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
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
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.