When I click to download a PDF file and pick "save to disk" (it's the default for me, FWIW), I don't get it downloaded. I sometimes *do* get a pair of exceptions in the console, though, to read instead: Error: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIDownloadManager.userDownloadsDirectory]" nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)" location: "JS frame :: file:///Applications/Minefield.app/Contents/MacOS/components/nsHelperAppDlg.js :: anonymous :: line 96" data: no] Source File: file:///Applications/Minefield.app/Contents/MacOS/components/nsHelperAppDlg.js Line: 96 Error: uncaught exception: unknown (can't convert to string) Started recently, possibly with today's nightly (2007082204; OS X on x86). My download dir is "/Users/shaver/Downloads" in the prefs window, and for both browser.download.dir and browser.download.downloadDir the pref value is (ahem): browser.download.downloadDir;AAAAAAFAAAIAAQhMNCBDYWNoZQAAAAAAAAAAAAAAAAAAAAAAAADBcRzJSCsAAAAH2LoJRG93bmxvYWRzAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAkRMcGpWicAAAAAAAAAAP////8AAAkgAAAAAAAAAAAAAAAAAAAABnNoYXZlcgAQAAgAAMFxjUkAAAARAAgAAMGpyqcAAAABAAgAB9i6AABvGQACAB9MNCBDYWNoZTpVc2VyczpzaGF2ZXI6RG93bmxvYWRzAAAOABQACQBEAG8AdwBuAGwAbwBhAGQAcwAPABIACABMADQAIABDAGEAYwBoAGUAEgAWVXNlcnMvc2hhdmVyL0Rvd25sb2FkcwATAAEvAAAVAAIADf//AAA= HTH!

I'm not on OSX, so I can't test this. I'm wondering if this is a failure in the directory services code for osx for retreiving the user's download folder. It was not used much prior to bug 308073. http://lxr.mozilla.org/mozilla/source/toolkit/components/downloads/src/nsDownloadManager.cpp#741 Can you post the result of the two new attributes in the download manager - userDownloadsDirectory and defaultDownloadsDirectory? var dm = Components.classes["@mozilla.org/download-manager;1"] .getService(Components.interfaces.nsIDownloadManager); dm.userDownloadsDirectory dm.defaultDownloadsDirectory dm.userDownloadsDirectory.target dm.defaultDownloadsDirectory.target (both should nsILocalFile) One of these might result in the same error. Note, browser.download.downloadDir is obsolete in Firefox, and is not used, so I'm not sure how it got set to that garbage. browser.download.dir is the pref we standardized on for custom download folder post 308073.

userDownloadsDirectory: NS_ERROR_FILE_UNRECOGNIZED_PATH (same error) defaultDownloadsDirectory: [xpconnect wrapped (nsISupports, nsILocalFileMac, nsILocalFile, nsIFile, nsIHashable)] user.target: NS_ERROR_FILE_UNRECOGNIZED_PATH (same error) default.target: NS_ERROR_NOT_IMPLEMENTED from nsILocalFileMac.target (That's not garbage, btw, it's how we encode Mac filehandles, because of pre-OS X legacy about names not being unique identifiers. Welcome to the Mac!)

Hey Mike, what's your browser.download.folderList pref?

It's set to 2.

Ok, so you're saying the funny data in your browser.download.dir is correct on the Mac? I was looking at the patch, and afaict, the code that sets that didn't really change. However, in the new userDownloadsDirectory code I've made the assumption it's a path. I think that might be the problem, I should be getting the complex value as an nsilocalfile instead.

Yes, I think that's it. Mike, if I roll a patch, would you have the time to test it out to be sure it addresses the problem? I'm not on osx so i'm unable to test this.

Tested on windows, works great. It would be nice if we could find someone to try it out on osx as well.

Slightly different error now, after rebuilding with the patch, probably just line-number fuzz: Error: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIDownloadManager.userDownloadsDirectory]" nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)" location: "JS frame :: file:///Users/shaver/src/trunk/obj-out/dist/Minefield.app/Contents/MacOS/components/nsHelperAppDlg.js :: anonymous :: line 107" data: no] Source File: file:///Users/shaver/src/trunk/obj-out/dist/Minefield.app/Contents/MacOS/components/nsHelperAppDlg.js Line: 107

Mike, would you mind posting your browser.download.dir pref in here again? The first example you gave had "browser.download.downloadDir;" on the front of what appears to be valid base64 encoded data. Was that correct?

No, that was an artifact of choosing "Copy" instead of "Copy Value" in the about:config window. The pref value starts after the ;, and is AAAAAAFAAAIAAQhMNCBDYWNoZQAAAAAAAAAAAAAAAAAAAAAAAADBcRzJSCsAAAAH2LoJRG93bmxvYWRzAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAkRMcGpWicAAAAAAAAAAP////8AAAkgAAAAAAAAAAAAAAAAAAAABnNoYXZlcgAQAAgAAMFxjUkAAAARAAgAAMGpyqcAAAABAAgAB9i6AABvGQACAB9MNCBDYWNoZTpVc2VyczpzaGF2ZXI6RG93bmxvYWRzAAAOABQACQBEAG8AdwBuAGwAbwBhAGQAcwAPABIACABMADQAIABDAGEAYwBoAGUAEgAWVXNlcnMvc2hhdmVyL0Rvd25sb2FkcwATAAEvAAAVAAIADf//AAA=

ahh, ok, just confirming - is Users/shaver/Downloads a valid directory on your system?

With a leading / it is, yes, and it happens to be my Downloads directory of choice. :) ("/Users/shaver/Downloads")

Decoding that base64 chunk shows "Users/shaver/Downloads" within it, and I'm not sure if the starting '/' is implicit in these things. But if not, InitWithNativePath, which I believe is being called, has this check - 1349 else if (filePath.IsEmpty() || filePath.First() != '/') 1350 return NS_ERROR_FILE_UNRECOGNIZED_PATH; and it's one of the few places in OSX file code that returns NS_ERROR_FILE_UNRECOGNIZED_PATH. I'm wondering if there is a missing '/' in your custom dir pref.

There's a leading slash in my pref as shown in the prefs window. Where is InitWithNativePath being called? Inside the complex-value deserialization?

Yes, and that error code really isn't returned very often, which is why I was looking at that. I hate to be a pain but do you have any time to place a break point in GetUserDwonloadsFolder and step through it to see where the failure is actually occuring? If not I'll try to find somebody on irc that can help us out.

Comment on attachment 278426 [details] [diff] [review] custom download folder patch v.1 > I'm wondering if there is a missing '/' in your custom dir pref. Isn't the call to InitWithNativePath returning that error because you're passing it the base64 encoded string? Anyhow, some review comments: >Index: toolkit/components/downloads/src/nsDownloadManager.cpp > if (customDirectory) { >- nsCOMPtr<nsILocalFile> aFile = >- do_CreateInstance("@mozilla.org/file/local;1", &rv); Glad you're removing this, using an "a" prefix for local variables is just confusing :) >+ customDirectory->Exists(&bRes); > if (bRes) { >- NS_ADDREF(*aResult = aFile); >+ NS_ADDREF(*aResult = customDirectory); > return NS_OK; > } >- rv = aFile->Create(nsIFile::DIRECTORY_TYPE, 755); >+ rv = customDirectory->Create(nsIFile::DIRECTORY_TYPE, 755); > NS_ENSURE_SUCCESS(rv, rv); > if (bRes) { >- NS_ADDREF(*aResult = aFile); >+ NS_ADDREF(*aResult = customDirectory); > return NS_OK; > } This second block of code you're changing looks dead - bRes can't be true here, otherwise you would have returned in the previous block. It looks like perhaps you meant to include another Exists() call before checking it, but even then Exists() shouldn't return false after a succesful Create() call. Seems like you should just remove the second "if (bRes)" (or rewrite it so that the Create() call is in a (!bRes) block rather than returning early, for clarity). This patch does fix the problem for me, though, so r+, but please do either attach a new patch here that includes the dead code removal, or file a bug to remove it separately.

And it'd sure be nice to have some tests for for existent and non-existent "download dirs". The latter would have caught the dead code bug, I believe, by causing that method to return the default download dir when it creates the directory.

(removed an uneeded rv =) Gavin, on the tests, I've not had the chance to write one of those. The purpose would be to check to make sure the directory returned was valid?

(In reply to comment #20) > Created an attachment (id=278606) [details] > custom download folder patch v.2 I don't think this is correct either - removing the rv checking for both calls makes the problem worse (one or both could fail while bRes remains true). I do think you want to go with the first patch, but just additionally remove the second "if (bRes)" check. > Gavin, on the tests, I've not had the chance to write one of those. The > purpose would be to check to make sure the directory returned was valid? Right, just verifying that the nsIFile you get back from the call to GetUserDownloadsDirectory() is correct for various values of the pref. Perhaps Shawn can help with this, he probably knows the DM tests best.

Comment on attachment 278606 [details] [diff] [review] custom download folder patch v.2 Beltzner doesn't do code reviews, and this doesn't need any additional (ui-)review once my comments are addressed.

Landed on the trunk. mozilla/browser/components/preferences/main.js 1.11 mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp 1.110

I can confirm that in the 2007-08-22-04 builds of Minefield on OS X 10.4, with a 'Users/stephend/Downloads/' directory, and the pref to prompt me where to save, I get the following exception thrown: Error: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIDownloadManager.userDownloadsDirectory]" nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)" location: "JS frame :: file:///Applications/Minefield.app/Contents/MacOS/components/nsHelperAppDlg.js :: anonymous :: line 96" data: no] Source File: file:///Applications/Minefield.app/Contents/MacOS/components/nsHelperAppDlg.js Line: 96 With the latest Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007090204 Minefield/3.0a8pre build, I can successfully save to 'Users/stephend/Downloads,' with no exceptions thrown. shaver: thanks for logging this bug! Verified FIXED