432427 - Download history can still be cleared unexpectedly when unchecking "Download history" from preferences before using the clearing private data dialog

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

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

From bug 431729 comment 15:
1. In "Privacy", "Private Data", "Settings", uncheck "Download History"
2. Download something
3. Tools | Clear Private Data -- notice "Download History" is _unchecked_ but
/bolded/
4. Click "Clear Private Data Now"

Expected Results:

No download history is deleted

Actual Results:

All completed/failed/cancelled download history is deleted

In step #3, if download history data *is* to be deleted, it should be checked;
being bolded just means the pref is toggle-able.

Comment 1

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

Attachment: patch

This fixes two bugs:
1) The "Downloads" checkbox checked and disabled states aren't initialized properly if "Clear history" _pref_ is true when the "Clear downloads" pref isn't (comment 0).
2) Selecting "Clear history" from the dialog when "Clear downloads" isn't selected disables the downloads checkbox, but doesn't check it, which is a bit misleading (some download history will be cleared in this case).

This whole situation sucks, and there are other bugs that we should probably fix (the "Settings" dialog not showing the same inter-dependency as the "Clear" dialog, what happens when you select "Download History" but not "History" is not clear, etc), but ideally we can fix this properly as soon as we're able to change strings again.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Gavin, can you find someone else to review this? Connor is out today.

Comment 3

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

Comment on attachment 319576 [details] [diff] [review]
patch

Not sure who else can review this, sdwilsh apparently has no internet connection. Trying Asaf...

Comment 4

[Dietrich Ayala (:dietrich)]

Comment on attachment 319576 [details] [diff] [review]
patch

drive-by review, per beltzner's request: r=me

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 319576 [details] [diff] [review]
patch

a1.9=beltzner

Comment 6

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

mozilla/browser/base/content/sanitize.xul 	1.18 

Comment 7

[Stefan Breunig]

The "Download History" checkbox isn't disabled initially when the dialog opens, although Browsing History is checked.
To reproduce on a new profile, first download something so you have Download History and then open the sanitize dialog.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050606 Minefield/3.0pre

Should I open a new bug for this? 

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #7)
> The "Download History" checkbox isn't disabled initially when the dialog opens,
> although Browsing History is checked.
> To reproduce on a new profile, first download something so you have Download
> History and then open the sanitize dialog.
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050606
> Minefield/3.0pre
> 
> Should I open a new bug for this? 

No, as the build you're using doesn't have the fix; please try tomorrow's build, or grab a tinderbox build: ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/FX-WIN32-TBOX-trunk/firefox-3.0pre.en-US.win32.installer.exe

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED as filed using:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050707 Minefield/3.0pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre

-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050706 Minefield/3.0pre

The first part of the second paragraph of comment 1 has been spun out as bug 432724.