"Clear Recent History" dialog has "Form & Search History" grayed-out & force-checked, when there's nothing to clear

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Preferences
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: dholbert, Assigned: felix)

Tracking

Trunk
Firefox 10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 1 obsolete attachment)

STR:
 1. Start Firefox with a fresh profile
 2. Ctrl-Shift-Del, to bring up Clear Private History dialog
 3. Click "Details"

ACTUAL RESULTS:
 "Form & Search History" is grayed out & force-checked. (I can't uncheck it.)

Presumably this is because I don't *have* any saved form & search history. However, this behavior is still quite counter-intuitive, because...
  1. There's nothing to clear, so if anything, it seems like the box should be force-UNchecked.
  2. I don't have any saved data in any of the other categories, either (Cookies, Cache, Active Logins, etc), but *they're* not grayed out -- I can still check & uncheck them all I want.
Summary: "Clear Recent History" dialog has "Form & Search History" checked & grayed-out, when there's nothing to clear → "Clear Recent History" dialog has "Form & Search History" grayed-out & force-checked, when there's nothing to clear
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090611 Minefield/3.6a1pre
Version: 3.5 Branch → Trunk
(Assignee)

Updated

6 years ago
Assignee: nobody → ffung
OS: Linux → All
Hardware: x86 → All
(Assignee)

Comment 2

6 years ago
Created attachment 561310 [details] [diff] [review]
Clear History: Disabled Preferences Should Not Be Checked

Summary of Changes:
On initialization, iterate through checkboxes with preferences rather than the preference elements themselves. Then any preference that cannot be cleared is untied from the checkbox so we can show a cleared disabled checkbox regardless of the set preference.
Attachment #561310 - Flags: review?(ehsan)
Attachment #561310 - Flags: review?(dholbert)
Comment on attachment 561310 [details] [diff] [review]
Clear History: Disabled Preferences Should Not Be Checked

I don't know the prefs code at all (and I'm out this week on a camping trip), so I'm not an appropriate reviewer.  Clearing r?dholbert.
Attachment #561310 - Flags: review?(dholbert)
Comment on attachment 561310 [details] [diff] [review]
Clear History: Disabled Preferences Should Not Be Checked

This looks good to me.  I only have two comments:

1. Please keep using this.sanitizePreferences, as the old code does.
2. Can you please add a test as well?  You can modify this existing test to start: <http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_sanitizeDialog.js>
Attachment #561310 - Flags: review?(ehsan)
(Assignee)

Comment 5

6 years ago
Created attachment 561651 [details] [diff] [review]
Clear History: Disabled Preferences Should Not Be Checked

Additional Changes:
- Added Tests

I did not understand why we would keep using this.sanitizePreference as per your suggestion ehsan. Since we are detaching the preference from the checkbox, (i.e. removing the preference attribute), if we iterate through this.sanitizePreference then we would need to run a query for each pref to find the related checkbox. It'd also be slightly more confusing because the mapping isn't one pref to one checkbox, there being less checkboxes than prefs.
Attachment #561310 - Attachment is obsolete: true
Attachment #561651 - Flags: review?(ehsan)
Comment on attachment 561651 [details] [diff] [review]
Clear History: Disabled Preferences Should Not Be Checked

Yes, you're right.  I was confused.  :-)

If you have tested this on the try server, I can land it for you.

Thanks a lot for your patch!
Attachment #561651 - Flags: review?(ehsan) → review+

Comment 7

6 years ago
Try run for d02e6e4995c3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d02e6e4995c3
Results (out of 96 total builds):
    success: 91
    warnings: 5
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ffung@mozilla.com-d02e6e4995c3
(Assignee)

Updated

6 years ago
Whiteboard: checkin-needed
Keywords: checkin-needed
Whiteboard: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a3850b04dc0
Keywords: checkin-needed
Whiteboard: [inbound]
Target Milestone: --- → Firefox 10
https://hg.mozilla.org/mozilla-central/rev/1a3850b04dc0
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.