1892947 - Align clear cookies and site data on shutdown checkbox with new sanitize on shutdown prefs

Status: VERIFIED FIXED

(Toolkit :: Data Sanitization, defect, P1)

Description

[Harshit Sohaney [:hsohaney]]

The clear cookies and site data when Firefox shuts down checkbox is not aligned with the new sanitizeOnShutdown prefs https://searchfox.org/mozilla-central/source/browser/components/preferences/privacy.js#2095. We should support the new prefs based on if the user is using the new clear history dialog.

Comment 1

[Harshit Sohaney [:hsohaney]]

Attachment: Bug 1892947 - Align clear on shutdown for cookies and site data section prefs to the new clear on shutdown prefs. r?pbz

Comment 2

[Harshit Sohaney [:hsohaney]]

Attachment: Bug 1892947 - (part 2) tests for clear cookies and site data on shutdown pref alignment with new clear history dialog. r?pbz

Depends on D208410

Comment 3

[Emma Zühlcke [:emz]]

[Tracking Requested - why for this release]:
Affects the new dialog we're shipping in Fx126 that comes with new prefs for clearing on shutdown.
This breaks syncing the "Delete cookies and site data when Firefox is closed" checkbox in about:preferences with the clear on shutdown settings. The checkbox still updates the old prefs which means checking it will have no effect.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eeabe25b2ac3
Align clear on shutdown for cookies and site data section prefs to the new clear on shutdown prefs. r=pbz,settings-reviewers
https://hg.mozilla.org/integration/autoland/rev/5c4c36caa85d
(part 2) tests for clear cookies and site data on shutdown pref alignment with new clear history dialog. r=pbz,settings-reviewers

Comment 5

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/eeabe25b2ac3
https://hg.mozilla.org/mozilla-central/rev/5c4c36caa85d

Comment 6

[Harshit Sohaney [:hsohaney]]

Comment on attachment 9398247 [details]
Bug 1892947 - Align clear on shutdown for cookies and site data section prefs to the new clear on shutdown prefs. r?pbz

Beta/Release Uplift Approval Request

• User impact if declined: This breaks syncing the "Delete cookies and site data when Firefox is closed" checkbox in about:preferences with the clear on shutdown settings. The checkbox still updates the old prefs which means checking it will have no effect. Users who expect their data to be cleared on shutdown will not see their data cleared.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Start out with a fresh profile for both test scenarios.

Test 1:

1. Check the checkbox "Delete cookies and site data when Firefox is closed" from about:preferences -> privacy & security -> Cookies and Site Data
2. Verify if the clear on shutdown settings have site data selected to be cleared (about:preferences -> privacy & security -> history -> custom history settings)

Test 2:

1. Select both "cookies and site data" and "cache" to clear on shutdown through about:preferences -> privacy & security -> history -> custom history settings
2. The checkbox "Delete cookies and site data when Firefox is closed" should be checked now

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is fairly small and there are automated tests coverage for this pref syncing.
• String changes made/needed: No
• Is Android affected?: No

Comment 7

[Donal Meehan [:dmeehan]]

Comment on attachment 9398247 [details]
Bug 1892947 - Align clear on shutdown for cookies and site data section prefs to the new clear on shutdown prefs. r?pbz

Approved for 126.0b8

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/90770625d9e3
https://hg.mozilla.org/releases/mozilla-beta/rev/b3259c59eabe

Comment 9

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

I have verified this on Firefox Nightly 127.0a1, build ID 20240429210502 and on Firefox Beta 126.0b8, build ID 20240429171348 (Treeherder build) using macOS 12.6.6, Windows 11 and Ubuntu 22.04, following the steps from Comment 6.

• @hsohaney, after performing test 1 on a fresh profile, I have noticed that the only difference between an affected and a fixed build is the fact that the "History" option from the "Clear browsing data and cookies" dialog is checked on the fixed build (besides the "Cookies and site data" and "Temporary cached files and pages" checkboxes) - is this what needs to be verified in this scenario?
• After performing test 2 on a fresh profile, "Delete cookies and site data when Firefox is closed" checkbox is checked as soon as the "Clear history when Firefox closes" checkbox is selected from "Use custom settings for history" setting

If there is anything else that should be checked here regarding the new prefs, please let us know. Thanks!

Comment 10

[Harshit Sohaney [:hsohaney]]

@bhidecuti, only the Cookies and site data and Temporary cached files and pages checkboxes should be checked in the clear on shutdown settings once you check "Delete cookies and site data when Firefox is closed" from about:preferences -> privacy & security -> Cookies and Site Data. Was clear on shutdown selected on the profile by any chance before checking the checkbox?

Comment 11

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

Attachment: Clear history checked after checking the "Delete cookies and site data" checkbox

@hsohaney, the "Clear history when Firefox closes" checkbox from History section becomes automatically checked after checking the "Delete cookies and site data when Firefox is closed" from about:preferences -> privacy & security -> Cookies and Site Data on a fresh profile (on both latest Nightly and Beta 8 builds) - please see the attached video.
No prior modifications were performed in about:preferences before checking the "Delete cookies.." checkbox.
Are there any other details that I am missing? Thanks in advance!

Comment 12

[Harshit Sohaney [:hsohaney]]

Oh yeah that's expected! I thought you meant that the history checkbox gets checked to clear browsing history on shutdown. Thanks, this is expected behaviour!

Comment 13

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

Attachment: History checkbox checked after checking the "Delete cookies..." checkbox

@hsohaney, just to make sure that we are on the same page, I am attaching a screenshot with the behavior I was mentioning in Comment 9, for the first test, where it can be observed that the "History" option from the "Clear browsing data and cookies" dialog is checked on the fixed build (while on an affected build it is not, only the "Cookies and site data" and "Temporary cached files.." checkboxes are) after selecting the "Delete cookies and site data when Firefox is closed" checkbox.
Is this what needs to be verified in this scenario, as it seems to be the only difference between an affected and a fixed build for test 1?
Thank you in advance.

Comment 14

[Harshit Sohaney [:hsohaney]]

That seems off, only the cookies and site data and temporary cached files should be selected when you check "Delete cookies and site data when Firefox is closed". Could you layout the exact steps you performed to have "History" show up as checked? That is not intended behaviour

Comment 15

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

@hsohaney, of course. Here are the steps fore reproducing the behavior mentioned in Comment 9 (please see the video here: https://drive.google.com/file/d/1GJI318sfNdMuo_vCiBkoCwUzkb97thlo/view?usp=sharing)

1. Launch Firefox (latest Nightly / Latest Beta (b8) with a fresh profile
2. Go to about:preferences#privacy
3. Check the "Delete cookies and site data when Firefox is closed" checkbox from "Cookies and Site Data" section
4. Scroll down until you reach the "History" section and press the "Settings..." button (next to the "Clear history when Firefox closes" checkbox)
5. Observe the checked checkboxes from the "Clear browsing data and cookies" dialog

AR: "History" checkbox is checked (besides the "Cookies and site data" and "Temporary cached files and pages" checkboxes)

Also, I have noticed that if I perform the above steps right after updating to the latest Nightly build 127.0a1 (2024-05-01) / latest Beta build 126.0b8 the "History" checkbox is indeed not checked (only the "Cookies and site data" and "Temporary cached files and pages" checkboxes are checked). After that, if I create a new profile again and repeat the steps above, the "History" checkbox is checked again.

Please let me know how we should proceed further.

(In reply to Harshit Sohaney from comment #6)

Test 1:

1. Check the checkbox "Delete cookies and site data when Firefox is closed" from about:preferences -> privacy & security -> Cookies and Site Data
2. Verify if the clear on shutdown settings have site data selected to be cleared (about:preferences -> privacy & security -> history -> custom history settings)

Could you please clarify one more time what needs to be verified for test 1? Thank you!

Comment 16

[Sandor Molnar[:smolnar]]

Backed out due to critical failure with clear on shutdown

Backout link: https://hg.mozilla.org/integration/autoland/rev/b034eb8252983ca8aa3bbe76f995573f761bd900

Comment 17

[Harshit Sohaney [:hsohaney]]

Thanks for finding the error! That was unexpected and we have disabled the dialog for this cycle. The new dialog will now ship in 127. Thanks for catching that :bhidecuti

Comment 18

[Neha Kochar [:neha]]

Can someone describe the critical failure that led to the backout? What is the mitigation plan for Fx126 where the patch was uplifted to beta?

Comment 19

[Emma Zühlcke [:emz]]

See Bug 1894933 for details. We’re reverting to the old dialog for 126. Not sure who coined the term critical failure. It’s a data loss bug so we want to play it safe for 126 rather than uplifting a fix right before RC week.

Comment 20

[Serban Stanca [:SerbanS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/b034eb8252983ca8aa3bbe76f995573f761bd900

Comment 21

[Donal Meehan [:dmeehan]]

Backout merged to beta: https://hg.mozilla.org/releases/mozilla-beta/rev/69e776e53917940760e6e8a2b9e52ae2fade6b88

Comment 22

[Harshit Sohaney [:hsohaney]]

Can someone describe the critical failure that led to the backout? What is the mitigation plan for Fx126 where the patch was uplifted to beta?

We found breakage in clear on shutdown where, if a new user (or a user who had just flipped their pref to the new dialog), set the “Delete cookies and site data on shutdown” checkbox, and did not have clear history on shutdown checked before, suddenly had all of their history and browsing data deleted unexpectedly.
This was a specific edge case that was introduced with a beta uplift we had done earlier in the cycle here Bug 1892947 - Align clear cookies and site data on shutdown checkbox with new sanitize on shutdown prefs(In reply to Neha Kochar [:neha] from comment #18)

Comment 23

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

Thanks for looking into this, hsohaney.
I can confirm that the old "Settings for Clearing History" dialog is displayed while using Firefox 126.0 RC, build ID 20240506203248 on macOS 12.6.6, Windows 11 and Ubuntu 22.04.

Comment 24

[Emma Zühlcke [:emz]]

Lowering severity since the new dialog is currently disabled eveyrwhere.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2cf2eb879245
Align clear on shutdown for cookies and site data section prefs to the new clear on shutdown prefs. r=pbz,settings-reviewers,Gijs
https://hg.mozilla.org/integration/autoland/rev/861781a8286a
(part 2) tests for clear cookies and site data on shutdown pref alignment with new clear history dialog. r=pbz,settings-reviewers

Comment 26

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2cf2eb879245
https://hg.mozilla.org/mozilla-central/rev/861781a8286a

Comment 27

[Oana Botisan, Desktop QA]

I am a bit concerned about the results we had after testing the scenarios we had to run for this bug

Test 1:

1. Check the checkbox "Delete cookies and site data when Firefox is closed" from about:preferences -> privacy & security -> Cookies and Site Data
2. Verify if the clear on shutdown settings have site data selected to be cleared (about:preferences -> privacy & security -> history -> custom history settings)

When the "Delete cookies and site data when Nightly is closed" box is checked, then the option for "Clear history when Nightly closed" is automatically checked in the "Nightly will: Use custom settings for history" section.
This is confusing considering the fact that the history is not actually deleted.

My understanding of the reason that this feature didn't ship in Release 126 is because in this senario the history was deleted. So why is the option for "Clear history when Nightly closed" is selected by default?
It's confusing either way, because it either shouldn't it be selected by default or the history should be deleted.

Test 2:

1. Select both "cookies and site data" and "cache" to clear on shutdown through about:preferences -> privacy & security -> history -> custom history settings
2. The checkbox "Delete cookies and site data when Firefox is closed" should be checked now

We had issues on Ubuntu with this scenario, because sometimes the "Delete cookies and site data when Nightly is closed" box is NOT checked automatically after the the option for "Clear history when Nightly closed" is selected. And also, nothing is selected inside the Settings modal from the "History" section.

Comment 28

[Harshit Sohaney [:hsohaney]]

The reason the "Clear history when Nightly closed" option is automatically checked is because "Delete cookies and site data when Nightly is closed" checkbox controls clearing data on shutdown. We clear cookies and site data and cache on shutdown upon checking this checkbox, and should sync this pref change with the clear on shutdown dialog found through the "Nightly will: Use custom settings for history" section. This is also the old behaviour.

For the second comment, this is expected behaviour. The "Delete cookies and site data when Nightly is closed" should only be checked if both "clear cookies and site data" and "cache" are selected for clearing on shutdown through the clear on shutdown dialog. Was this the concerned behaviour, or was there something I'm missing?

Comment 29

[Oana Botisan, Desktop QA]

That is still confusing because of the message “Clear history when Nightly closes” even if the History option is not selected inside the modal. If somebody sees "clear history", they would expect that the history is deleted. Maybe if there was another message then things will be a little bit clearer.

On the second comment the only difference is that on Windows and macOS you don't have to select any options from the modal in order for the "Delete cookies and site data when Nightly is closed" to be checked, but on Ubuntu sometimes you have to select something, sometimes you don't. The behaviour on Ubuntu is not consistent either on its own or compared to the other OSs

Comment 30

[Harshit Sohaney [:hsohaney]]

That is still confusing because of the message “Clear history when Nightly closes” even if the History option is not selected inside the modal. If somebody sees "clear history", they would expect that the history is deleted. Maybe if there was another message then things will be a little bit clearer.

I think that problem can be a separate bug of its own since it is the old behaviour. I can create a bug for this, but I do not think this should block release since nothing has changed there.

On the second comment the only difference is that on Windows and macOS you don't have to select any options from the modal in order for the "Delete cookies and site data when Nightly is closed" to be checked, but on Ubuntu sometimes you have to select something, sometimes you don't. The behaviour on Ubuntu is not consistent either on its own or compared to the other OSs

The expected behaviour is if sanitize.clearonshutdown_v2.cookiesAndStorage and sanitize.clearOnShutdown_v2.cache are true and sanitize.privacy.sanitizeOnShutdown is true, then "Delete cookies and site data when Nightly is closed" is checked. It works the other way around aswel, as in if the checkbox is checked, then all of the mentioned prefs are enabled.

By default, when a user checks "clear history on shutdown", sanitize.clearonshutdown_v2.cookiesAndStorage sanitize.clearonshutdown_v2.cache and sanitize.clearonshutdown_v2.historyFormDataAndDownloads is enabled (for a fresh profile). Since cookiesAndStorage and cache are checked, we expect "Delete cookies and site data when Nightly is closed" to be checked. This is the behaviour you are noticing on macos and windows. I tested this on Ubuntu, and it seems to be following the same behaviour. Could you lay out the steps you took to see it behave differently on Ubuntu?

Comment 31

[Oana Botisan, Desktop QA]

Steps to reproduce:

1. Open Firefox with a new profile.
2. Go to about:preferences#privacy
3. Select "Use custom settings for history" in the History section and check the box fir "Clear history when Nightly closes"
4. Refresh page.
5. Change bak to "Remember history"
6. Repeat steps 1 - 4.

Sometimes you have to repeat the steps three times for the issue to reproduce.
Or deselect "Delete cookies and site data when Nightly is closed" manually before changing to "Remember history" if you can't reproduce the issue.

This are all the things we did in order to reproduce the issue. The interesting part is that with this steps you can't reproduce the issue on macOS and Windows. This only happens on Ubuntu.

Comment 32

[Oana Botisan, Desktop QA]

Attachment: Screencast from 03.06.2024 16_52_09.webm

Comment 33

[Harshit Sohaney [:hsohaney]]

Attachment: Screen Recording 2024-06-03 at 10.31.36 AM.mp4

This is expected behaviour because:

1. When you enable clear on shutdown for a new profile, history, cookies and cache get selected by default.
2. Since cache and cookies are selected, the "clear history when firefox closes" checkbox gets selected.
3. Upon deselecting the checkbox, you disable cache and cookies clearing on shutdown. But history is still enabled, that is why the clear history on shutdown option is still selected.
4. Now, when you disable clear history on shutdown, and then reenable it, the previous selected options are remembered (just history).
5. Since cookies and cache are not selected, the "clear history when firefox closes" checkbox is not selected.

I also unable to see any different behaviour on macos, I have attached a screen recording of me going through those steps on macos seeing the same behaviour.

Comment 34

[Oana Botisan, Desktop QA]

Thank you for the answer.
I will mark this issue as VERIFIED FIXED after we had the chance to verify on beta as well.

Comment 35

[Oana Botisan, Desktop QA]

We managed to verify the fix using Firefox 128.0b1 on macOS 12, Windows 11 and Ubuntu 22.04. Everything is as expected.