User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090531 SeaMonkey/2.0b1pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090531 SeaMonkey/2.0b1pre New bug as described in https://bugzilla.mozilla.org/show_bug.cgi?id=381157#c89 Reproducible: Always Steps to Reproduce: 1. if already exists delete downloads.sqlite 2. start a download 3. in download manager dialog choose "save file" Actual Results: After selecting "save file" it takes quite a long time until windows file selector opens (Probably in this time downloads.rdf is converted to downloads.sqlite - downloads.rdf file size is about 340K). For every entry in downloads.rdf the error message "x:\...\file.zip could not be saved, because an unknown error occurred. Try saving to a different location." is displayed. In error console there are several "Error: too much recursion" shown. After clicking through all error messages the download manager window opens showing the file progress as finished/failed/canceled. But no file is really downloaded. The partition with the download directory would have enough space to save the files. Expected Results: Import of downloads.rdf happens without error messages. I tried to strip down downloads.rdf to reproduce the error with a smaller file. But unfortunately the error occurs only with the original file. Because a lot of private urls in the file I can't attach it to this bug. I will try to create a downloads.rdf that triggers this bug the next days.

This downloads.rdf is a stripped version of my own file that triggers the described error.

Could you copy one of the "Error: too much recursion" errors in here (Using the "Copy context menu item in the console)? The location where this happens might give us some info what is failing here. I suspect it's toolkit code but I'm not sure.

So if you first open the Download Manager once and then start a download the problem does not happen?

(In reply to comment #5) > Could you copy one of the "Error: too much recursion" errors in here (Using the > "Copy context menu item in the console)? The location where this happens might > give us some info what is failing here. There is no more data available, "Error: too much recursion" is already the copied entry. (In reply to comment #6) > So if you first open the Download Manager once and then start a download the > problem does not happen? Opening the download manager without starting any download is enough to trigger the errors.

Shawn, any idea what could be happening there? This sound like it would be toolkit code, but we have nothing to confirm it...

well, the download state in the downloads.rdf is set to -1, which means it's in the NOT_STARTED state. So, we'll go ahead and try to start it. We check for downloads in this state at startup, and try to start them (startup of the service).

I got a similar error after switching from pre-toolkit-download-manager build to Mozilla/5.0 (X11; U; Linux i686; rv:1.9.1pre) Gecko/20090610 SeaMonkey/2.0b1pre One of the files has started downloading again after the conversion. Looking in the downloads.rdf, the file entry seems to be corrupted (a couple of duplicate lines): <RDF:Description RDF:about="file:///home/mike/NEW/proofforchobos.wmv" NC:Name="proofforchobos.wmv"> <NC:ProgressMode>normal</NC:ProgressMode> <NC:ProgressMode>none</NC:ProgressMode> <NC:StatusText>Downloading</NC:StatusText> <NC:StatusText>Finished</NC:StatusText> <NC:Transferred>0KB of 0KB</NC:Transferred> <NC:Transferred>55872KB of 55872KB</NC:Transferred> <NC:URL RDF:resource="http://rs425l33.rapidshare.com/files/181009094/3193060/proofforchobos.wmv"/> <NC:URL RDF:resource="http://rs425tl2.rapidshare.com/files/181009094/3196738 /proofforchobos.wmv"/> <NC:File RDF:resource="file:///home/mike/NEW/proofforchobos.wmv"/> <NC:DownloadState NC:parseType="Integer">0</NC:DownloadState> <NC:DownloadState NC:parseType="Integer">1</NC:DownloadState> <NC:ProgressPercent NC:parseType="Integer">0</NC:ProgressPercent> <NC:ProgressPercent NC:parseType="Integer">100</NC:ProgressPercent> </RDF:Description> Also, all converted entries in the new d/l manager contain "0 bytes" in the "transferred" column. Is it a known bug?

kmike: Yes, your file seems to have some kind of strange corruption, having two things in there. The "0 bytes" thing is probably from us setting a number of defaults when importing the old SeaMonkey downloadmanager lists, as there's a lot of things that are not completely compatible in inporting. Shawn, should we do something at import time so that we don't import erroneaos NOT_STARTED cases? All in all, I think we should probably ship SeaMonkey 2.0 Beta 1 as it is wrt import, maybe relnote this and gather more feedback from that.

I guess we could just not import NOT_STARTED downloads. That's kinda odd that it's in that state in the rdf file though...

Confirming this is a problem, I get this as well, but the 12 entries I have with problems are those with the following in: <NC:DownloadState NC:parseType="Integer">-1</NC:DownloadState> No duplicate lines for me. I am presuming -1 has a meaning that means that download manager tries to process it in some way which it shouldn't.

Just opening the download manager is sufficient, you do not need to start a download to trigger the error messages. At the moment there are no checks on state in nsDownloadManager::ImportDownloadHistory() http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp#628 Should be fairly easy to fix, but is in toolkit, so cannot really block this beta release on the issue. Saying that we will take a reviewed fix. Nominate as a blocker for another release if you want.

I think we probably should have a one-liner there switching the state of NOT_STARTED download to CANCELED, then those that might have rightfully had that state can always be retried ;-)

(In reply to comment #15) > I think we probably should have a one-liner there switching the state of > NOT_STARTED download to CANCELED, then those that might have rightfully had > that state can always be retried ;-) I concur

Comment on attachment 385834 [details] [diff] [review] Patch r=sdwilsh You'll have to add a unit test (not hard) for this to land on branch though.

now with unit test

Comment on attachment 388116 [details] [diff] [review] Patch >+ if (state == -1) >+ state = 3; Could we use the named constants here please so it's clearer in the code as to what is going on? r=sdwilsh

Frank, can we move forward on this please? This is definitely wanted for SeaMonkey 2.0b2, even if there's no way of marking that.

Comment on attachment 388116 [details] [diff] [review] Patch Wanted for the next SeaMonkey Beta, this could (in theory) affect users switching from Firefox 2 to Firefox 3.5.x, but as far as I know the FF 2 download manager has never set that state in the downloads.rdf file.

Important info for approval and any later readers of this bug (just told it on IRC but should state it here as well): The relevant code this is patching is only on 1.9.1, the whole downloads.rdf import code has been removed from mozilla-central for the 1.9.2 cycle. Because of that, this patch is only for landing on 1.9.1 and not mozilla-central.

Comment on attachment 388116 [details] [diff] [review] Patch Approved for 1.9.1.3. a=ss

Pushed to 1.9.1 branch, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/56b99735ac85

Verified for 1.9.1.3 using unit test.