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

Status: REOPENED

(Toolkit :: Data Sanitization, defect, P2)

Description

[Harshit Sohaney]

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]

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]

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

[Paul Zühlcke [:pbz]]

[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]

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

[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]

@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

[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]

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

[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]

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

[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]

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

[Paul Zühlcke [:pbz]]

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