Closed
Bug 702748
Opened 13 years ago
Closed 13 years ago
Use a pref for disabling per-site remembering of download directory
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: darktrojan, Assigned: darktrojan)
References
Details
(Keywords: user-doc-needed)
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
Lots of users are complaining about this feature, so we should have a pref to disable it.
Attachment #574665 -
Flags: review?(gavin.sharp)
|
||
Comment 1•13 years ago
|
||
Comment on attachment 574665 [details] [diff] [review]
patch
Looks like you moved too much code into the if (isContentPrefEnabled()) block in getFile - doesn't this break the private browsing mode behavior when the pref is flipped?
Attachment #574665 -
Flags: review?(gavin.sharp) → review-
|
Assignee | |
Comment 2•13 years ago
|
||
Attachment #574665 -
Attachment is obsolete: true
Attachment #575127 -
Flags: review?(gavin.sharp)
|
||
Comment 3•13 years ago
|
||
If I'm reading that correctly, currently you can clear the normal last dir pref, but not the per-site pref.
gDownloadLastDir.setFile(null, null) - ok
gDownloadLastDir.setFile(uri, null) - throws
With the above patch applied:
gDownloadLastDir.setFile(null, null) - ok
gDownloadLastDir.setFile(uri, null) - ok or throws depending on savePerSite value
Why not make it possible to clear the per-site pref like just the normal one -> more consistent api.
Something like:
if (aURI && isContentPrefEnabled()) {
if (aFile instanceof Components.interfaces.nsIFile)
Services.contentPrefs.setPref(aURI, LAST_DIR_PREF, aFile.path);
else
Services.contentPrefs.removePref(aURI, LAST_DIR_PREF);
}
Also, should the comment at the beginning of file updated to include all this per-site stuff.
|
|
Assignee | |
Comment 4•13 years ago
|
||
Thanks Atte, that's a good point. Throwing when passed null was unintentional. I'll fix it.
|
|
Assignee | |
Comment 5•13 years ago
|
||
Attachment #575127 -
Attachment is obsolete: true
Attachment #575127 -
Flags: review?(gavin.sharp)
Attachment #578814 -
Flags: review?(gavin.sharp)
|
|
||
Comment 6•13 years ago
|
||
Comment on attachment 578814 [details] [diff] [review]
patch v3
Sorry for the delay, Geoff. I'll be better next time! And if I'm not, yell at me loudly!
Attachment #578814 -
Flags: review?(gavin.sharp) → review+
|
|
Assignee | |
Comment 7•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
|
||
Comment 8•13 years ago
|
||
In which end-user release will this be fixed?
|
||
Comment 9•13 years ago
|
||
(In reply to David E. Ross from comment #8)
> In which end-user release will this be fixed?
As the target milestone says: mozilla11 (FF/TB 11 / SM 2.8).
|
||
Comment 10•13 years ago
|
||
And the most important information is missing in the comments:
The pref is called "browser.download.lastDir.savePerSite" and is accessible in about:config (if not, create it as a bool).
|
|
Assignee | |
Comment 12•13 years ago
|
||
(In reply to :aceman from comment #10)
> And the most important information is missing in the comments:
> The pref is called "browser.download.lastDir.savePerSite"
Good point.
> and is accessible in about:config (if not, create it as a bool).
Not by default, users will need to add it.
Keywords: user-doc-needed
|
||
Comment 13•4 years ago
|
||
Hi Geoff,
Disabling Save by Site disabling is currently (v83.0) not available, either in the main Preferences, and also about:config.
Would you be happy to add that into the preferences (preferably the main prefs)?
Flags: needinfo?(geoff)
|
|
Assignee | |
Comment 14•4 years ago
|
||
No. You need to add it yourself as a preference in about:config, as mentioned in the most recent comments.
Flags: needinfo?(geoff)
You need to log in
before you can comment on or make changes to this bug.
Description
•