Closed Bug 476174 Opened 12 years ago Closed 12 years ago

Default Download Directory (DfltDwnld) uses old Safari 2's value instead of Leopard's ~/Downloads

Categories

(Core :: XPCOM, defect, P2)

All
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: Mardak, Assigned: sdwilsh)

References

Details

(Keywords: verified1.9.1)

Attachments

(1 file)

After bug 384068 landed, I noticed my downloads ended up in ~/Desktop/Downloads even though I was running OS X 10.5.6

Some reason DfltDwnld resulted in ~/Desktop instead of ~/Downloads

Components.classes["@mozilla.org/file/directory_service;1"].getService(Components.interfaces.nsIProperties).get("DfltDwnld", Components.interfaces.nsILocalFile).reveal()

After some debugging, whimboo suggested changing Safari 2's download directory preference. Even though I'm using the new unibody MacBook5,1; I migrated my account from an old iBook that had Safari 2's download directory set to ~/Desktop.

whimboo provided a url to get Safari 2 because Leopard doesn't ship with it: http://michelf.com/projects/multi-safari/

After launching Safari 2, changing the download directory to ~/Downloads, then quitting the app only did DfltDwnld fix itself.
Seems like this has been causing problems for other people:

http://support.mozilla.com/en-US/kb/unable+to+download+or+save+files#Delete_corrupt_plist_file
http://discussions.apple.com/thread.jspa?messageID=5666281&#5784012

"corrupt plist" might not be corrupt, but if you migrated from a previous system where the download directory pointed to a now non-existant directory, strange stuff would happen. I suppose it's not too bad now because we check if the directory is writable, etc.
I figured out how this could have happened over breakfast, and I'm glad that you folks came to the same conclusion.

What we are going to have to do is update http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/src/nsDownloadManager.cpp#1269 to check the OS version number and assume the download stack on 10.5 and later instead of doing this crazy comparison.

Also need to update http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsDirectoryService.cpp#917 somehow - looking into that now.
Hardware: x86 → All
Attached patch v1.0Splinter Review
This makes our life a whole lot more sane on OS X.  We probably even want to take this on 1.9.1.

This makes us use the user download folder if it exists, and goes to the desktop otherwise.  The download folder will exist on 10.5 or later, and for 10.4 we go to the desktop as we always have.  Note - that means we don't use the internet config pref anymore, but only Camino uses that anyway.  (They may want to change as well)

This ends up fixing bug 301647, as well as addressing bz's past comments about making the directory service manage this stuff in a more sane manner.

Just pushed this to the try server, so we'll have test builds up in a bit.
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Attachment #359815 - Flags: ui-review?(beltzner)
Attachment #359815 - Flags: superreview?(benjamin)
Attachment #359815 - Flags: review?(joshmoz)
Attachment #359815 - Flags: review?(gavin.sharp)
Blocks: 301647
Requesting blocking-1.9.1 on this so we can finally get the download location correct for all OS X users.  Bug 454242 blocked, and this fixes another issue that manifests as the same problem.
Flags: blocking1.9.1?
We kind of need to get this right. It makes us stick out like a sore thumb on OSX.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Whiteboard: [needs review gavin][needs review josh][needs sr bsmedberg][needs ui-r beltzner]
Tryserver build works for me. I switched the download directory back to ~/Desktop in Safari 2 and downloaded something on trunk and it ended up in ~/Desktop/Downloads while the tryserver build end sup in ~/Downloads
Attachment #359815 - Flags: review?(joshmoz) → review+
Whiteboard: [needs review gavin][needs review josh][needs sr bsmedberg][needs ui-r beltzner] → [needs review gavin][needs sr bsmedberg][needs ui-r beltzner]
Whiteboard: [needs review gavin][needs sr bsmedberg][needs ui-r beltzner] → [needs review gavin][needs sr bsmedberg][needs ui-r beltzner][SWAG: 1hr post reviews]
Attachment #359815 - Flags: review?(gavin.sharp) → review+
Whiteboard: [needs review gavin][needs sr bsmedberg][needs ui-r beltzner][SWAG: 1hr post reviews] → [needs sr bsmedberg][needs ui-r beltzner][SWAG: 1hr post reviews]
Attachment #359815 - Flags: superreview?(benjamin) → superreview+
Whiteboard: [needs sr bsmedberg][needs ui-r beltzner][SWAG: 1hr post reviews] → [needs ui-r beltzner]
Attachment #359815 - Flags: ui-review?(beltzner) → ui-review+
Comment on attachment 359815 [details] [diff] [review]
v1.0

uir=beltzner; better is better, thanks
Whiteboard: [needs ui-r beltzner]
Whiteboard: [can land]
http://hg.mozilla.org/mozilla-central/rev/4d7f404d3aa3
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → mozilla1.9.2a1
Verified fixed on the 1.9.1 branch using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090220 Shiretoko/3.1b3pre.

Verified fixed on the trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090220 Minefield/3.2a1pre.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 485201
You need to log in before you can comment on or make changes to this bug.