Workaround for: Bug 1242176 - All passwords deleted upon startup if privacy.clearOnShutdown.passwords = True but privacy.sanitize.sanitizeOnShutdown = false

This is one approach if we know that something isn't locking privacy.clearOnShutdown.passwords or going to set privacy.clearOnShutdown.passwords to true again.

I'm going to install Norton Identity Safe now in a VM to see what exactly they're doing to make sure this hotfix will work.

(Patch looks fine, but the issue we're looking into is to understand when/how Norton is flipping these prefs, if that's what is actually happening, to make sure it's not just going to undo the hotfix's pref changes.)

See bug 1242176 comment 48 for the explanation of what's going on. * I added setting privacy.sanitize.migrateClearSavedPwdsOnExit to true when the master pref privacy.sanitize.sanitizeOnShutdown is false so I no longer see any need for an additional pref to keep track of who was hotfixed. This makes it so that we don't re-introduce the regression after the hotfix gets installed as we proactively indicate that the migration isn't needed for these users. * I also no longer offer the hotfix to Fx44 users because it doesn't add any value since privacy.sanitize.migrateClearSavedPwdsOnExit should already be true (meaning this code won't run anymore) for all of them and their passwords are already gone. Ritu, Fx38 was chosen somewhat arbitrarily (partly because it's ESR) as the lower bound. We need people to get this hotfix before upgrading to Fx44 to avoid the data loss. Do you prefer a different lower bound?

> > Ritu, Fx38 was chosen somewhat arbitrarily (partly because it's ESR) as the > lower bound. We need people to get this hotfix before upgrading to Fx44 to > avoid the data loss. Do you prefer a different lower bound? I think Fx38 seems reasonable. Is this something that we will also need to hotfix for ESR38.6.0 release that went live on Tuesday 1/26 as well?

Matt, Justin: I am worried about this reply Yuric had to my comment https://bugzilla.mozilla.org/show_bug.cgi?id=1242176#c54. He suspects that we might experience data loss not just in passwords but other history entities as well. SoftVision team tested "Clear history when Firefox closes" feature but I am not sure if they exactly followed MattN's suggested repro steps here: https://bugzilla.mozilla.org/show_bug.cgi?id=1089695#c121. I am going to test this myself on 44 now. In the mean time, can you guys test this as well? And what would a hotfix that address all history entities look like. Or are better off doing a dot release in that case? Thanks!

(In reply to Ritu Kothari (:ritu) from comment #5) > > > > Ritu, Fx38 was chosen somewhat arbitrarily (partly because it's ESR) as the > > lower bound. We need people to get this hotfix before upgrading to Fx44 to > > avoid the data loss. Do you prefer a different lower bound? > > I think Fx38 seems reasonable. Is this something that we will also need to > hotfix for ESR38.6.0 release that went live on Tuesday 1/26 as well? ESR38.6.0 doesn't have the bug but the hotfix isn't for people who already lost their passwords, it's to go to users before they get Fx44. If you look at the patch, it's not even going to get delivered to Fx44. The hotfix should get delivered to ESR so that if an ESR user switches to Fx44 then they won't lose their passwords.

I didn't include any previous hotfixes in this one (yet). Can people confirm we don't need to? The previous hotfix v20160106.01 included hotfixes from the following bugs: * Bug 1234802 - hotfix to turn of the pref media.fragmented-mp4.gmp.enabled * Bug 1236042 - hotfix to disable the youtube override introduced in 43.0.3 * Bug 1237103 - hotfix to fix the Application Reputation remote lookups. This new hotfix is planning to target 38.0 - 43.* so we would add complexity if we have to do various version checks in the hotfix code and thus I would like to avoid that if possible.

(In reply to Matthew N. [:MattN] from comment #8) > I didn't include any previous hotfixes in this one (yet). Can people confirm > we don't need to? The previous hotfix v20160106.01 included hotfixes from > the following bugs: > * Bug 1234802 - hotfix to turn of the pref media.fragmented-mp4.gmp.enabled > * Bug 1236042 - hotfix to disable the youtube override introduced in 43.0.3 Bug 1234802 and bug 1236042 have been fixed in 44, so dropping their hotfixes should be OK if we feel "enough" release channel users have updated from 43 to 44.

(In reply to Chris Peterson [:cpeterson] from comment #9) > Bug 1234802 and bug 1236042 have been fixed in 44, so dropping their > hotfixes should be OK if we feel "enough" release channel users have updated > from 43 to 44. Note that I think we're going to keep updates to Fx44 disabled until enough people get this new hotfix to avoid dataloss of passwords upon installing Fx44.

(In reply to Ritu Kothari (:ritu) from comment #6) > In the mean time, can you guys test this as well? I tested with all data types checked but with the master pref set to false and didn't lose important data. > And what would a hotfix that address all history entities look like. It depends on what the cause was. It would be a different problem as the code with the error only affected logins.

(In reply to Matthew N. [:MattN] from comment #10) > (In reply to Chris Peterson [:cpeterson] from comment #9) > > Bug 1234802 and bug 1236042 have been fixed in 44, so dropping their > > hotfixes should be OK if we feel "enough" release channel users have updated > > from 43 to 44. > > Note that I think we're going to keep updates to Fx44 disabled until enough > people get this new hotfix to avoid dataloss of passwords upon installing > Fx44. I guess that is OK. If a 43 user has not received the latest 43 hotfix yet, then they're still having a bad time watching YouTube on Linux or XP. So they won't be in a worse place if your hotfix doesn't include those fixes.

(In reply to Matthew N. [:MattN] from comment #11) > (In reply to Ritu Kothari (:ritu) from comment #6) > > In the mean time, can you guys test this as well? > > I tested with all data types checked but with the master pref set to false > and didn't lose important data. > That's great! Thanks for the due diligence.

Deployed to production yesterday in bug 1244321.

Back from pto: just like Chris said! Matt, many thanks for taking care of this hotfix!