Left over from bug 23821. Need more informative alerts when file save fails due to permission violation. Right now it just says "Unknown error". Same thing for similar errors (eg, no space left on device).

Sorry, forgot to assign to law.

Definitely.

*** Bug 33320 has been marked as a duplicate of this bug. ***

Move to M16 for now ...

see it on windows too.

*** Bug 32461 has been marked as a duplicate of this bug. ***

Move to M21 target milestone.

The file `KingFred.html' could not be saved to the folder `Documents', because {reason}. {suggestion} ( Save As ... ) ( Cancel ) (( Retry )) [Mac, X] (( Retry )) ( Save As ...) ( Cancel ) [Windows] For lack of write permission: * reason = `you do not have permission to change the contents of that folder' * suggestion = `Change the folder properties and try again, or try saving in a different location.' (Do you have an XP way of telling whether it's a floppy which is write-protected?) Lack of space: * reason = `there is not enough room on the disk' (Ideally this would be followed by `(42 kb needed, 15 kb available)', but that's getting into RFE territory.) * suggestion = `Make more space on the disk and try again, or try saving in a different location.' Unknown error: * reason = `an unknown error occurred' * suggestion = nothing Any other causes?

*** Bug 60266 has been marked as a duplicate of this bug. ***

i'm bumping up the sev on this a bit, due to lorca's observations in bug 60266. bill/mscott, shouldn't we throw a dialog after the hits OK in the filepicker dialog, saying something to the effect of, "cannot save since you don't have the proper permissions for this directory"? since the download progress dialog does now appear, this gives the perception of dataloss.

Yes, we should inform the user about what happens. I had a fix for at least one aspect of this general problem but it got nixed for rtm. We need to merge the file download logic and make sure that all paths properly handle errors. There's lots of bugs relating to that; if there weren't so many, maybe I could sort them all out :-(.

Raising Severity due to data loss status. Adding myself to cc: list. From DUPLICATE 60266 (that I wrote up): Happens in: Linux 11-08-09 Does not happen in: Mac/Win->N/A Reason for Severity: Data loss/silent data loss/perceived data loss Reproducible: Always. Steps to Reproduce: 1. Open up a download box 2. Select directory to save in-make sure its not normally accessible to your particular account 3. Watch download/save progress 4. Confirm "Download/Save Complete" dialog box. 5. Check directory for contents Actual Results: Nothing there. Expected Results: File downloaded to be there.

Nominating for nsbeta1. This overlaps with other bug(s) but I want to flag it anyway.

Netscape Nav triage team: this is a Netscape beta stopper. Changed priority to P2.

Did a fix for this get checked in last night?

Almost. I added error handling code in the xfer code. That isn't used all the time, yet. I'm using bug 67216 to track that issue. So, I'm marking this one as dependent on that bug.

Adjusting keywords; dependent bug 67216 should be the one marked nsbeta1.

Marking nsbeta1+ since we should fix this

nav triage team: Yes, this sucks, but push has come to shove. Bumping from mozilla0.9 train, marking nsbeta1-, resetting target milestone

Marking nsbeta1+, mozilla0.9

I'm not getting to this dialog till next round...

nav triage team: Marking mozilla0.9.2

*** Bug 80242 has been marked as a duplicate of this bug. ***

<q src="mpt" date="2000-07-26 20:38"> Any other causes?</q> 3. Enter a pathname which doesn't exist (I had a c:\download directory, but accidently typed in the plural c:\downloads instead) Actual Results: "There was an error writing to the target location" Expected Results: "This directory does not exist. Should it be created?"

bill/bryner, are there even hooks for creating directories from the file pickers?

nav triage: frequency of this problem and severity are low enough to defer to m0.9.3.

nav triage team: Unfortunately, the work involved to fix this problem the right way is too much to take for mozilla0.9.3. Pushing out to mozilla1.0

Removing nsenterprise nomination.

spam: over to File Handling.

->098, actual text used is no longer the issue in this bug, we need to ensure that users are alerted to failed saves. updating summary to reflect bug morphage.

related: bug 65770 - not getting an alert when disk full for file downloads.

*** Bug 87438 has been marked as a duplicate of this bug. ***

*** Bug 108669 has been marked as a duplicate of this bug. ***

*** Bug 109069 has been marked as a duplicate of this bug. ***

*** Bug 105718 has been marked as a duplicate of this bug. ***

*** Bug 112175 has been marked as a duplicate of this bug. ***

*** Bug 113062 has been marked as a duplicate of this bug. ***

*** Bug 116753 has been marked as a duplicate of this bug. ***

I think my bug (116753) should be blocked by this, rather then duped, as it is a specific issue, on only one type of OS, which warrants seperate verification after this fix is in. Unless there is contention, I'll undupe it tomarrow.

*** Bug 115197 has been marked as a duplicate of this bug. ***

This document describes how I plan to fix this problem (once and for all). Anybody who cares about how we do this (or has an interest in the components that are modified) should have a look and offer feedback, ASAP.

All these download related items are moving out; delayed due to overhaul of downloading and many regressions in this area.

This patch makes the uriloader use the new nsIProgressDialog. It does not try to reuse nsIWebBrowserPersist as its stream listener. The main drawback is that there are still two places where we have network I/O that has to check for errors, etc. I will try to see if I can re-use the nsWebBrowserPersist as the uriloader's stream listener, but I'm worried that that might be hard. So, this is an interim fix to add error handling and use the new and improved progress dialog.

OK, I need some reviews, approvals, and super-reviews now. There are 4 separate items: 1. The new nsIProgressDialog interface and implementation in mozilla/embedding/components/ui/progressDlg. The files of interest are all named nsProgressDialog.* (the ones named nsProgressDlg.* are obsolescent). I need a volunteer to review it; I'd like either or both of ben and mscott to super-review. 2. The nsWebBrowserPersist changes, attached to this bug. Adam Lock needs to review/approve these changes. I need a volunteer to super-review (ben or mscott?). 3. The contentAreaUtils.js changes, attached to this bug. I need a volunteer to review this (Boris? I know you've been hacking code real near this); Ben should super-review/approve. 4. The uriloader/exthandler changes, attached to this bug. I need a volunteer to review (Boris?); mscott should super-review/approve. The only open issues are: a. Some problems related to handling errors returned on OnStopRequest. E.g., if I abort a download from my ftp server console, I get an alert saying "The operation was abortted by the system administrator" (or something like that) but the progress dialog then jumps to 100% and indicates the download is completed. Neither nsWebBrowserPersist nor the uriloader seem to look for errors and pass them to the dialog. I've marked where they should do that with comments but some code still needs to be written. b. There is some flakiness in the networking code (I think) when saving to a diskette. I've opened bug 124035 and bug 124036 for those problems. The one may be due to an error occuring when we create the file transport, which we're not handling. The timing one requires some investigation.

I forgot this file in the nsWebBrowserPersist patch. This needs to be added in mozilla/embedding/components/webbrowserpersist/locale/en-US as nsWebBrowserPersist.properties.

This patch doesn't build as-is on Linux because embedding/components/ui/progressDlg isn't hooked up to the build.

brian or bill, have you had a chance to test these patches on Mac [either 9.x or 10.1.x]? just curious. :)

Thanks, Brian; I had those unix makefile changes on my to-do list. Sarah, no Mac testing, yet. But it's all xp code so it should work fine :-). There does need to be comparable tweaks to the Mac build scripts, though. I'll work on that.

Comment on attachment 68248 [details] [diff] [review] Patch to nsWebBrowserPersist to work with new nsIProgressDialog r=adamlock A couple of nitpicks. First (and this is a holdover from the old code) we should use PRUint32 instead of unsigned int for bytesRead/written/remaining variables. And can you unindent the switch statements so the case line up underneath the switch?

Comment on attachment 68268 [details] [diff] [review] Patch to uriloader to make it work with the new nsIProgressDialog sr=mscott can we pull the string bundle #define properties line out and up at the top of the file instead of nested in the if clause just for clarity? Just for kicks can you make sure you can still open a large mail attachment after these changes?

Comment on attachment 68248 [details] [diff] [review] Patch to nsWebBrowserPersist to work with new nsIProgressDialog sr=mscott modulo Adam's comments about using PRUint32s instead of "unsinged int"

re: comment 54 I've made the changes suggested; thanks.

Minor grouch: Progress should never be shown in a dialog (not minimizable, not closable). Normally it will be in a progress window (minimizable, not closable); the exception is the download manager, where it will be in a document window (minimizable, closable). So calling anything `nsIProgressDialog' is just going to confuse people who use the API ... s/ProgressDialog/ProgressWindow/g would be nice. :-)

Comment on attachment 68251 [details] [diff] [review] Patch to contentAreaUtils.js to make it work with new nsIProgressDialog >+ var filesFolderLeafName = getStringBundle().GetStringFromName("filesFolder"); >+ filesFolderLeafName = filesFolderLeafName.replace(/\^BASE\^/, nameWithoutExtension); Any reason not to use FormatStringFromName here? >+ if (!filesFolder.exists()) >+ filesFolder.create(lfIID.DIRECTORY_TYPE, 0755); >+ } I'd suggest not doing this. This is done in any case by the WebBrowserPersist code (see nsWebBrowserPersist::SaveDocumentInternal) and that code actually has the smarts to not create this dir if there are no linked resources to persist. Let's not ruin that over here. :) >+dump("onStateChange exception=" + e + "\n" ); This is going away before checkin, right? :)

Comment on attachment 68268 [details] [diff] [review] Patch to uriloader to make it work with the new nsIProgressDialog > helperAppDlg \ >+ progressDlg \ > plugin \ Fix the indentation (makefile.win). >+NS_IMETHODIMP nsExternalAppHandler::Observe(nsISupports *aSubject, const char *aTopic, const PRUnichar *aData ) >+{ >+ // User pressed cancel button on dialog. >+ return this->Cancel(); >+} >+ Should we check aTopic? If it's always going to be "oncancel" (as the interface comments for nsIProgressDialog suggest), then the "observer" attr of nsIProgressDialog should perhaps be renamed "cancelObserver" for clarity.... As it stands people may be tempted to make things other than cancelling observable and then this code could fail. > mWebProgressListener = aWebProgressListener; >- webProgress->AddProgressListener(mWebProgressListener); >+ mTimeDownloadStarted = PR_Now(); >+ Why move this? It seems more correct to call that right at the end of OnStartRequest as before.... > while (count > 0) // while we still have bytes to copy... Shouldn't this be | while (NS_SUCCEEDED(rv) && count > 0) | ? Otherwise we'll go into an infinite loop on a read error.... >+ rv = mOutStream->Write(mDataBuffer, numBytesRead, &numBytesWritten); That should be bufPtr instead of mDataBuffer, no? >+ if (action!=nsIMIMEInfo::saveToDisk) add spaces around the "!=" >+ if(openWith.IsEmpty()) Space after the "if" as elsewhere in this file? Same for the "if"s inside this one.... >+ // Tell progress dialog what we're opening with. >+ progressDlg->SetOpeningWith(openWith); I'd say that we should either have a default value for "openWith" in case it's _still_ empty after the nsILocalFile dance or assert that it's not empty. I'm not sure how it can be empty at this point without something being deeply wrong... Also, you want openWith.get() to get a wstring, no? >+ PRInt32 mProgress; Could we add a brief comment explaining what this integer is?

Comment on attachment 68270 [details] [diff] [review] Additional nsWebBrowserPersist file >diskFull=There is not enough free space to store the file. Free up space and try again, or, choose a different target location. There should be no comma after the "or". I'm not sure we want to have the "try again" text in there. Also, I think we should have two different error messages: 1) Disk full when writing to our temp file in the uriloader. Since the user has no way to easily configure where we put this file (short of shutting down mozilla, changing some environment variables, and restarting) we should just say that the user should free up some space. Bonus points for adding the name of the directory or file we were saving to in this alert so the user knows _where_ to free up space (this would replace "the file" in the message). 2) Disk full when writing to the file/dir the user chose to save to when doing "Save As" (error in nsWebBrowserPersist). In this case the error should be similar to what we have now, again with "the file" replaced by the actual path.

Apparently the %1, %2, etc in that attachment of mine should be something else, but no-one in #mozilla seems to know precisely what. :-)

I didn't say that. bz said to use %s, he's right. I merely noted that if the order didn't match (eg if you localized to spanish or something) and needed to use the second param first, you could.

Re: Scott's comments (in comment 55) I've removed the #define for |properties| and just put the chrome url in the call as a literal. Likewise modified in nsWebBrowserPersist. Re: Boris's comments (in comment 60) I've made all those changes, except for: * the moving of the setting of mTimeDownloadStarted; this value is needed in ShowProgressDialog, which gets called before the previous setting of this field occurred. That could be worked around somehow, but it was easier to just move this up a few lines. The data is coming in, so it's not like we're lying. * SetOpenWith(openWith) not using |.get()|. It doesn't seem necessary and I prefer to let the compiler figure out the best way of passing the argument. Is that politically incorrect for some reason? Updated patches coming soon.

re: comment 59 >Any reason not to use FormatStringFromName here? No, but I just copied this code from nsProgressDlg.js as is. I suppose I can fix it now. >>+ if (!filesFolder.exists()) >>+ filesFolder.create(lfIID.DIRECTORY_TYPE, 0755); >>+ } > >I'd suggest not doing this Ditto for this. I'll remove these lines now, based on your say so. Ben/Adam, you guys seemed to want this before; please holler if removing this is a mistake. >+dump("onStateChange exception=" + e + "\n" ); Deleted (was temporary debug code).

re: comment 61...comment 64 I'm not sure what to make of the "suggestion" entries in mpt's new nsWebBrowserPersist.properties. We are a bit constrained here in that the code can only return a single string (from nsWebBrowserPersist.cpp and nsExternalHelperAppService.cpp) to the progress dialog code. mpt described his proposal as "ideal," but I'm stuck here in the real world. I can add code to nsProgressDialog.js to look for some specific return values (the NS_ERROR_FILE_* ones), at which point I could fetch additional "suggestion" strings for use in the alert. But, given that there seems to be only one suggestion for each error, I'd rather just combine the two sentences in the same property string (I'm ignoring the implied different suggestion in the case of the user running out of space on the same volume as the trash/recycle-bin because that's too hard to check for anyway and users will just have to figure that out, I say). I think the suggestions for putting the file names and amount of space still needed are useful. I'm going to work on mashing together mpt's error text and suggestion strings and getting the file name and size inserted, as he and Boris suggest. I may not get to that real soon, though. Can I get OK to check in what I have and deal with improving the messages via another bug?

> SetOpenWith(openWith) not using |.get()|. There was a push at some point to remove operator char* from all the string classes (bug 53057). I guess nsXPIDLC?String is safe from that for now, so do whichever you prefer here. :)

Blake commented (in another bug) that the title bar should read something like: 37% - Saving foobar.zip i.e., move the percentage to the beginning so it appears in the task bar. I like the idea of getting as much info as possible into the task bar, but I'm not sure this doesn't make the full title look a little awkward. Comments?

I think we should only show the percentage in the titlebar when the window is minimized, a la IE. This is very easy now that minimized/maximized/restored state flags have been implemented on nsIDOMChromeWindow. Someone commented that they want it in the titlebar all the time in case they have something obscuring the rest of that window, but that hardly seems like the common case to me. I think always displaying it in the titlebar as well as the window is overkill, and awkward as you point out.

I'd like to ditto Blake's comments.

A couple comments on the nsWebBrowserPersist changes: * could the block of new code (switch) in OnDataAvailable be moved to a separate method? (I worry this block may need to grow to accommodate new errors.) * I'm concerned about the string bundle dependency. Wouldn't it be better if the persist code returned a specific set of error codes which the client/caller could interpret/describe as it saw fit?

re: moving the switch statement Sounds reasonable. It wouldn't need to be a method (member function), though. Is a static utility function OK? That would make it easier for me to cut/paste it into the uriloader code. re: property file dependency This was suggested by Jud Valeski so that the code could be shared by clients that didn't care to embellish the alert text at all. Also, it kind of matches the strategy deployed by the networking code. A more pragmatic reason is that it is difficult, in general, to pass nsresult values to JavaScript code because they're only defined by C++ macros (although some, including all that we'd want to examine now, are also available to JS via Components.results). I wasn't thrilled with this scheme, either, but it's what we decided to do when the design was being debated.

As a note, the patches in this bug do not solve the problem of MoveTo() failing in the uriloader due to lack of space or some such. This is covered by bug 115201.

Your note is correct. Nor does it handle some cases where the error notifications come in via OnStopRequest. If I get to those things soon, then I'll incorporate them; otherwise, that will have to wait a bit longer.

I've updated this patch to refactor the error notification code into a separate routine as suggested by Kathy Brade. I've also slightly tweaked the code she added recently to call StartUpload. That should only happen if no error has occurred.

Updated patch to uriloader/exthandler stuff. This refactors the code that sends the error notifications as was done in nsWebBrowserPersist. I also added some error checking to handle errors when moving the temp file. This is less that ideal still, because the return value is always NS_ERROR_FAILURE so the message ends up as the generic "There was an error writing to the target location." nsILocalFile implementationsof MoveFile are going to have to be fixed to give better NS_ERROR_FILE_* messages. Likewise, I added error checking after calling LauchAppWithTempFile. That doesn't quite work though, because it seems the implementors of that function don't return error codes.

Updated patch; no material changes.

re: progress window title OK, I'm trying to wrap this part up and I see where IE (v5.5) uses "37% of big.zip Completed" and that's the title all the time. It would be easier if I didn't have two distinct titles so I wonder if I can just use the same technique? That would also permit me to ignore differences when saving versus opening-with. Thoughts? Also, noboby has come up with a solid proposal for the error alert text. I'm going to proceed with my best shot at merging the comments that have been received. I don't think we need to embellish the "out of space" message beyond inserting the target file path (the about downloaded so far is in the progress window and if users are counting their bytes that closely, they can do the math).

Comment on attachment 69774 [details] [diff] [review] Revised patch for uriloader/exthandler code r=bzbarsky. Make sure to make the mac build changes corresponding to those REQUIRES changes.

Comment on attachment 69776 [details] [diff] [review] Updated patch to contentAreaUtils >+ const flags = nsIWBP.PERSIST_FLAGS_NO_CONVERSION | nsIWBP.PERSIST_FLAGS_REPLACE_EXISTING_FILES; ... >+ var encodingFlags = flags; >+ if (persistArgs.contentType == "text/plain") { >+ encodingFlags |= nsIWBP.ENCODE_FLAGS_FORMATTED; >+ encodingFlags |= nsIWBP.ENCODE_FLAGS_ABSOLUTE_LINKS; >+ encodingFlags |= nsIWBP.ENCODE_FLAGS_NOFRAMES_CONTENT; >+ } Do we really want to have "PERSIST_FLAGS_NO_CONVERSION" and "PERSIST_FLAGS_REPLACE_EXISTING_FILES" in the encoding flags? That seems odd.... For one thing as encoding flags that would be ENCODE_FLAGS_PREFORMATTED and ENCODE_FLAGS_WRAP.... Is that purposeful? If so, just make it explicit. Also, could we always pass nsIWBP.ENCODE_FLAGS_ENCODE_ENTITIES? that will fix some bugs we have on entities being saved as characters in some charset instead... The rest looks good.

I suspect that encodingFlags stuff is wrong. I just cribbed that code from elsewhere and may have garbled it in the translation. The code I copied it from was messed up, too, as I recall. I'll straighten that out.

I've rolled into this patch the new .properties file, in which I've incorporated mpt's wording (for the most part). Each message incorporates the target file path as an insert.

I've enhanced the error handling to insert the file name (temporary vs. final) into the messages.

Fixes the encodingFlags snafu and changes the title text to put the percentage first.

Comment on attachment 70175 [details] [diff] [review] Final(?) contentAReaUtils.js patch r=bzbarsky

Comment on attachment 70173 [details] [diff] [review] Final(?) uriloader/exthandler patch r=bzbarsky

nsbeta1+ per nav triage team

fix checked in

The nsWebBrowserPersist changes created a new compiler warning: embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:674 `PRBool readError' might be used uninitialized in this function I am not absolutely sure if this can really happen, or it's just a compiler being stuping, but it seems that if function is called with 0 aLenght, something indeed might happen...

Another warning caused by this check-in: uriloader/exthandler/nsExternalHelperAppService.cpp:1159 `PRBool readError' might be used uninitialized in this function

I looked at that when I was reviewing.. I don't think onDataAvailable is ever called with a 0 data length (what would be the point?). We can add an initializer just for kicks, I suppose....

FYI, I've had bugs in the past where the length in an ODA call was 0 and I had code which wasn't coping with that properly. So it does happen.

OK, looking over the code again nsExternalHelperAppService will never hit the code that uses readError with readError unset. WebBrowserPersist could do it if mCancel is true.... Attaching simple patch to fix warnings.

Comment on attachment 70686 [details] [diff] [review] Patch to fix warnings r=hwaara

Mozilla lets you open new bugs, too. I suggest we do that in this case. And by the way, the reporting of errors while saving files has been totally fucked up from the git go. If we find some residual problem in that area, it would not surprise me. In fact, if we don't, that would shock me. But when it happens. let's please open new bugs specific to the remaining problems rather than keep re-opening this one.

Am I right assuming that marking thius bug as "fixed" would not stop attachment 70686 [details] [diff] [review] from going in? Or should I open a new bug for the warnings?

I'd open a separate bug. Just assign it to me.

tested using 2002.03.06 commercial verif builds on linux rh7.2, mac OS X v10.1.3 and win2k. seems that there are four basic test cases used to verify this bug: a. write permissions b. out of space in target directory c. check that target directory exists d. out of space in temp directory detailed results are below. since issues still remain here, i'm not sure if i should verify this particular bug. however, i have spun off or reopened bugs as needed, so maybe this should get closed...? in any case, there still are related bugs which might be part of the dependency tree. will verify those seperately (when fixed) since they haven't been dup'd here. i wish to verify this particular bug in a finite manner. :) a. write permissions. primarily Linux and prolly mac OS X. make sure to display an alert when the user lacks the priv to write to targetted directory. many dups. example: create a dir that's owned by root (or another user) and give it 700 mode. the try to save file to that dir. RESULTS: * linux: PASS. get an alert dlg saying "Cannot create file. Directory [path] is not writable." * mac: PASS. in the file picker, folders that don't have the proper privs are simply inaccessible, eg, greyed out or have a lock icon. * win2k: tested with a locked floppy. PASS, sort of. got the alert dlg "The disk cannot be written to because it is write protected. Please remove the write protection from the volume in drive A:." but the download progress remained and appeared completed --even though no saving occurred. b. out of space. All platforms. should get an alert where download cannot complete due to lack of space. many dups. cleanup issues remain: see bug 129332. linux example: try to save to full/limited harddisk --different partition than /tmp. mac example: try to save to full/limited Zip disk. win2k example: try to save to full/limited ramdisk. RESULTS: * linux: FAIL. see bug 129614. seemed to download successfully, but it was incomplete. no error dlg came up --and the salted file persisted in /tmp even after quitting. will try w/floppy later on... * mac: PASS. got an alert dlg "There is not enough room on the disk to save to Zip 100:[file]. Remove unnecessary files from that disk and try again, or try saving in a different location." * win2k: FAIL, also see bug 129614. one-time occurrence, though, on win2k: got an alert dlg "A:\[file] could not be saved, because an unknown error occurred. Sorry about that. Try saving to a different location." c. make sure target directory exists. All platforms. was bug 80242. example: in file picker for saving, enter a non-existent directory name. RESULTS: * linux: PASS. get an alert dlg saying " Path [path] doesn't exist, can't save [path/filename]" * mac: PASS. in the file picker for saving, i'm prevented from entering a bogus directory --unlike the file picker for opening, which has a "go to" field for such a purpose. * win2k: FAIL. reopened bug 80242. bogus dir is created anyway, and file is saved there. d. since we first download to a temp location, then move to user-determined target, if there's not enough room in the temp location, we should display an alert. All platforms. was bug 105718 and bug 112175. linux example: need to fill up /tmp a lot, then test saving. mac example: need to fill up Desktop folder, then test saving. win2k example: need to fill up c:\WINNT\Temp, then test saving. RESULTS: * linux: FAIL. see bug 129604. * mac: overall FAIL when the default temp location is the Desktop --same results as linux. this occurred with a new profile. also see bug 129604. one-time "success": got an alert dlg "There is not enough room on the disk to save to [non-Desktop-remembered location]:[salted filename]. Remove unnecessary file from the disk and try again, or try saving to a different location." this. however, was for an existing profile with that pref set. * win2k: FAIL, also see bug 129604.

Hmmm. Maybe 8 test cases, because clicking on a link and going through the helper app dialog (open-with vs. save-to-disk) is completely distinct from right clicking and choosing save-link-as. I'm guessing all your tests were using the former, which complicates things in a number of ways (especially prone to error when the downloaded copy in temp is moved/copied to the final destination). I think things work more reliably if you use the save-link-as technique. Not that the problems you cite aren't problems, too. They're just more likely problems in the nsExternalAppHandlerService code.

Sairuh--when you are testing "write permissions" be sure to test not only directory permissions but also file permissions. On a Mac, you can lock a file by selecting it and choosing "File | Get Info | General Information" and then checking the box near the bottom. I'm sure you know how to change the permissions on Linux files. I think you can lock Windows-files but I don't recall how at the moment.

Bug 131108 was just opened, which tells us that we are not dealing well with saves to files on which we do not have write-permissions.

that bug is a duplicate of a separate bug which was recently fixed

hokay, finally ran through the more common scenarios, but this time using "save link target as" from the link context menu. this will be a partial rubberstamp verification, since i didn't repeat the cases of a full temp location or full linux partition. if anyone would like to test those scenarios, pls do add your observations/methods here! anyhow, the detail tests and results (based on 2002.05.02.0x-1.0.0 branch comm bits) are at http://hopey.mcom.com/tests/bug27609-save-link.txt --will post as an attachment soon, since it's on an internal server.