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)
5.44 KB,
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
|
||
https://hg.mozilla.org/mozilla-central/rev/1e81f179adf2
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
•