Last Comment Bug 566484 - Replace "Download retention" with "Download history" in Browser preferences
: Replace "Download retention" with "Download history" in Browser preferences
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Download & File Handling (show other bugs)
: Trunk
: All All
: -- minor (vote)
: seamonkey2.1a2
Assigned To: rsx11m
:
:
Mentors:
Depends on: 487675
Blocks: 423281
  Show dependency treegraph
 
Reported: 2010-05-17 16:10 PDT by rsx11m
Modified: 2010-05-19 16:01 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (4.23 KB, patch)
2010-05-17 16:14 PDT, rsx11m
iann_bugzilla: review+
Details | Diff | Splinter Review
Proposed patch (v2) (4.23 KB, patch)
2010-05-18 05:37 PDT, rsx11m
rsx11m.pub: review+
neil: superreview+
Details | Diff | Splinter Review

Description rsx11m 2010-05-17 16:10:25 PDT
This is a follow-up to bug 487675 which I had on my list for a while. The
label introduced there for the browser.download.manager.retention pref reads "Download retention", thus is accurate with regard to the preference, but somewhat inconsistent with "Download history" defined in sanitize.dtd for the Privacy & Security preferences. The latter follows less the name for the pref but appears to be more intuitive, thus avoiding ambiguities, therefore the proposal to change it in the Browser preferences as well.
Comment 1 rsx11m 2010-05-17 16:14:27 PDT
Created attachment 445845 [details] [diff] [review]
Proposed patch

This changes the label to "Download history" as proposed, and also changes "Remove downloads" to "Remove list entries" to be more specific what is does
(the term "download" is used just above, thus no need to repeat it here).
It also matches the identifier with the modified entity names.
Comment 2 Robert Kaiser 2010-05-17 18:36:34 PDT
BTW, I still wonder if the pref UI implemented in bug 487675 is actually mirrored by real download manager UI behavior, as I don't remember implementing functionality for that pref in our UI.
Comment 3 rsx11m 2010-05-17 18:57:43 PDT
Good point, but it seems to work. I've just changed the setting from "Never"
to "When quitting SeaMonkey", then quit and restarted, and the list was gone.
Comment 4 neil@parkwaycc.co.uk 2010-05-18 03:07:28 PDT
(In reply to comment #2)
> BTW, I still wonder if the pref UI implemented in bug 487675 is actually
> mirrored by real download manager UI behavior, as I don't remember implementing
> functionality for that pref in our UI.
It's handled in nsDownloadManager.cpp [GetRetentionBehavior()].
Comment 5 Robert Kaiser 2010-05-18 04:36:51 PDT
Right, I just verified that it doesn't need implementation elsewhere. Clearly my bug 487675 comment #0 was somewhat misguided and I did let myself misguide me once again when I re-read it.
Comment 6 neil@parkwaycc.co.uk 2010-05-18 05:02:02 PDT
> This changes the label to "Download history" as proposed, and also changes
> "Remove downloads" to "Remove list entries" to be more specific what is does
> (the term "download" is used just above, thus no need to repeat it here).
Well, I don't like "Remove list entries:"... if you don't like "Remove download entries:" I could go with "Remove entries:". Either way the entities could be named removeEntries.label/accesskey.
Comment 7 rsx11m 2010-05-18 05:37:31 PDT
Created attachment 445943 [details] [diff] [review]
Proposed patch (v2)

Label and entity titles changed per Neil's comment #6, it's a bit longer but still fits well into the dialog window (at least on WinXP). 

> (comment #4) It's handled in nsDownloadManager.cpp [GetRetentionBehavior()].
I figured that toolkit is taking care of it somewhere.

Carrying forward r=IanN from previous patch.
Comment 8 neil@parkwaycc.co.uk 2010-05-18 05:44:54 PDT
Comment on attachment 445943 [details] [diff] [review]
Proposed patch (v2)

>-<!ENTITY removeDownloads.label          "Remove downloads">
[Odd; I was expecting this to end in a :]
Comment 9 rsx11m 2010-05-18 05:51:08 PDT
Me too, but neither does any of the other labels followed by some box end
in a colon, thus I kept it this way. Making label styles consistent across preference dialogs may be a bug on its own...

Thanks for the reviews, push on trunk please.
Comment 10 Justin Wood (:Callek) 2010-05-19 14:18:58 PDT
Pushed as: http://hg.mozilla.org/comm-central/rev/b750da3cd889

Note You need to log in before you can comment on or make changes to this bug.