351490 - "clear now" button doesn't behave according to checkbox "ask me before clearing private data" in tools - options - privacy

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[u130342]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060821 Firefox/2.0b2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060821 Firefox/2.0b2

In options - privacy - checkbox "ask me before clearing private data" there's a button "clear now".
When the checkbox is checked the button gets ellipses "Clear Now..." indicating a dialog will appear after clicking, if unchecked ellipses disappear.

But, when clicking "Clear Now" or "Clear Now..." the state of the checkbox is ignored! Instead the setting gets used which was active when opening the dialog (privacy.sanitize.promptOnSanitize).

Thus:
privacy.sanitize.promptOnSanitize=true, checkbox=unchecked, "Clear Now", shows dialog
privacy.sanitize.promptOnSanitize=false, checkbox=checked, "Clear Now", doesn't show dialog

Reproducible: Always

Steps to Reproduce:
1. open - tools - options, select privacy
2. change checkbox "ask me before clearing private data" !important!
3. click "clear now/..."
4. click ok (saves preferences)
5. repeat step 1-4


Actual Results:  
if "clear now...": no dialog is shown
if "clear now": dialog is shown

Expected Results:  
"Clear Now..." should always show dialog
"Clear Now" should never show dialog
Clear Now button should show dialog according to checkbox


Maybe it's a critical bug, because one might think to be able click on "clear now..." then deselect something (e.g. browsing history). Browsing history is the lost immediately, without further notice!

Comment 1

[Jackie Liu]

I can reproduce "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060904 BonEcho/2.0b2".  Requesting "blocking-firefox2"

Comment 2

[Adam Guthrie]

CCing Jeff since he wrote this code.

Comment 3

[Mike Connor [:mconnor]]

Gavin, short version is, from looking at how we call this, the pref needs to be synced when the Clear Now button is clicked.

This is going to be whenever instantApply is set to false

Comment 4

[:Gavin Sharp [email: [e-mail]]]

It's not just a matter of syncing the pref before calling sanitize, because we don't want the pref to be changed if the user then clicks cancel. Option 1 would be to set and the restore the pref around the sanitize() call, but that's pretty hacky. Option 2 would be to not update the button unless the pref actually changes, but I'm not sure that that is optimal behavior.

Comment 5

[Mike Connor [:mconnor]]

Maybe we should just update the ellipsis on windows open, and onsynctopreference, then the behaviour doesn't change until the pref takes effect.  This is correct, if slightly confusing.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: update the button based on the actual pref value

This makes it so that the button label doesn't change when instantApply is false, and the checkbox is toggled. The instantApply=true case isn't affected, since valueFromPreferences changes before the onchange handler is called.

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 238008 [details] [diff] [review]
update the button based on the actual pref value

please get this in ASAP and request approval.

this shouldn't have any risk, its just calling another method on the same object.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/components/preferences/privacy.js 	1.18

Comment 9

[Mike Schroepfer]

Comment on attachment 238008 [details] [diff] [review]
update the button based on the actual pref value

a=schrep for drivers.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/components/preferences/privacy.js 	1.9.2.6