Camino/2004-04-20-08 fails to save web pages or images. It works fine using Mozilla-Mac/2004-04-20-08. 1. Access URL 2. File/Save As... Camino saves nothing. Additionally, files such as .sit or .gz files *are* downloaded invisibly in the background, but are not appearing in the Downloads window.

cc'ing darin, he changed the d/l code most recently.

greg, on what date did this start happening in camino?

Presumably this is fallout from bug 240835. Hopefully, it will have fixed itself in the 4-21 build, since it looks like the 4-20 Camino build was before the final checkin, but the 4-20 build was after.

The problem is new in the 20040420 build. It is not evident in the 20040416 build. There were no Camino builds between those two.

bug 240835's patches did not affect camino... http://lxr.mozilla.org/mozilla/source/camino/src/embedding/CHDownloadFactories.mm#86 looks like it should also check for nsITransfer now, not only nsIDownload... and it should also support nsISupports... but I don't think that'd fix this...

the patch wasn't in the bug, darin had to land it after camino caught fire from his checkins.

Still evident using 2004042108.

Disregard comment 7. What I thought was the 0421 build is still just 0420.

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

This was definitely already broken by 2004-04-17 (12:00). I looked again more closely, and it seems I was mistaken about the checkin times. I don't see anything in bonsai after the 4/20 NB, so this probably *won't* fix itself in the next build.

(In reply to comment #4) > The problem is new in the 20040420 build. It is not evident in the 20040416 > build. There were no Camino builds between those two. yes, i am having this trouble with 2004042108 nightly. this was not there in 16th April nightly.

2004042308 still broken. I guess the bustage fixes checked in over the last day didn't target this bug.

I'm unable to investigate this problem right now since I don't have immediate access to a Mac. I will be at the Mozilla Foundation sometime this week, and I'll try to use one of the Macs there to debug this, but in the meantime if someone can help investigate this, I would greatly appreciate it. Thanks in advance!

I'm playing around with the debuger, but I'm not familiar enough with this code to know what I'm looking for. Here's what I found while watching an attempt to save a web page: The code fails in the init method of nsDownloadListener http://lxr.mozilla.org/mozilla/source/camino/src/download/nsDownloadListener.mm#68 on the line: NS_ENSURE_TRUE(targetFile, NS_ERROR_INVALID_ARG); because this if statement above it fails, and thus targetFile is never set: nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(mDestination); if (fileURL) The line that sets fileURL is new from the breakage fixing: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/camino/src/download&command=DIFF_FRAMESET&file=nsDownloadListener.mm&rev1=1.11&rev2=1.12&root=/cvsroot I'm not 100% clear on what do_QueryInterface does, exactly, but I assume that it expects mDestination to be initialized, and I'm pretty sure it isn't yet--the only initialization for it that I see is later in this same function, where it's set to the method parameter aTarget. Should this be using aTarget instead? I hope that's somewhat helpful. I'll keep trying to figure it out, but if someone wants to point me in the right direction that would be great.

Sorry, I should have simply tested before posting. The answer is yes: using aTarget in place of mDestination makes saving pages and images work correctly. I'm still seeing problems with the download window not opening itself, and the transfer status being listed as "interrupted", but that may be bug 240367. I'm updating my build to incorporate that patch, then I'll test various saving and downloading and post any remaining issues.

This bug is probably related to this one: http://bugzilla.mozilla.org/show_bug.cgi?id=240748 It does not crash Camino, but maybe it is related? I remember around the 20040417 build it was crashing when I tried to save files also.

(In reply to comment #14) > because this if statement above it fails, and thus targetFile is never set: > > nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(mDestination); > if (fileURL) Heh. The reason is, of course, that mDestination is null at this point - it's only set to mTarget later in this function. > I'm not 100% clear on what do_QueryInterface does, exactly, but I assume that it > expects mDestination to be initialized, and I'm pretty sure it isn't yet--the > only initialization for it that I see is later in this same function, where it's > set to the method parameter aTarget. Should this be using aTarget instead? That does seem like the correct fix. (That, or move the mDestination setting to earlier in this function) > I hope that's somewhat helpful. I'll keep trying to figure it out, but if > someone wants to point me in the right direction that would be great. That does explain this breakage. Thanks for investigating this! (I don't think bug 240748 is related...)

I think this is preferable to moving |mDestination = aTarget|, since that would split up the member initializations. The patch for bug 240367 does indeed fix the status/progress issues, so I think this is all we need.

Comment on attachment 146910 [details] [diff] [review] Use aTarget instead of mDestination Requesting sr; too simple to need an r

you can have an r=biesi anyway :)

landed on trunk only (branch doesn't have this problem). thanks, everyone, for following up and nailing this!

Comment on attachment 146910 [details] [diff] [review] Use aTarget instead of mDestination sr=pink

mike, with bug 24867 landing on the branch, this will need to be landed there as well (according to bug 24867 comment 211). If so, then a=asa if needed.

aieeeeeeee. this is going to suck. reopening for branch landing, keep a close watch on bug 24867

Darin was good to us, and checked in the correct version of the Camino patch, so we should be all set here.