Scheduled feature work for MachV: Suggested requirements: * Extend download functionality to include: * Tracking of progress on current downloads simultaneously in one window * Extended information such as file size, time remaining, location, etc. * Pause and Resume functionality * Historical view of downloads * Should be usable as a standalone download manager (similar to SmartDownload) * Should include ability to serve content/ads * Possibly only for downloads longer than a specified length * Possibly contextually sensitive by file type Current engineering Plan: Feature proposal at http://www.silverstone.net.nz/work/DownloadManager.htm Task breakdown and estimates at http://mozilla.org/xpapps/MachVPlan/Download_Mgr_estimate.html Currently assigned to ben for planning/design; implementation estimate: 14.5 days to parity + 4 days optional enhancements we will have to postpone, and likely have to cut.

See also bug 25995, "Download manager (multiple downloads per window)".

spam: over to File Handling. i have not changed the assigned developer [or the other fields for that matter], so if anyone realizes that a bug should have a more appropriate owner, go ahead and change it. :)

Would it possible to use the download manager for downloading mail attachments, as suggested by yours truly in bug 103875?

alex, that sounds nifty. mscott and ben, what do you think?

is the silentdownload projekt part of the build ? this is a must feature for the download manager :)

hmmmmm it would be interesting if everything went through it. For example a large page is being displayed by browser, page itself and every image, script and stylesheet file progress being displayed in Download Manager. That way you can easily see what files a page consists of, and quickly click and stop downloads of images you don't want, or select priority in which images download - could be quite handy on slow connections. Could even make a special folder in DM for tracking browser downloads. Another folder for mail downloads. Another folder for normal file downloads. And everything else like components or plugin downloads. In other words giving user *total* control over any downloads application makes? Could also display statistical info like total bandwidth application, or certain parts of it take. Would such functionality require any significant rewrite in Networking component? Or does it use sufficient Visitor design patterns and provides API for managing all downloads in progress?

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

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

One addition to the original list: persistent download queues. That's the best thing about external download managers :) Oh, and it should also be able to cap the number of simultaneous downloads. As for using it to monitor and control browser content downloads, that's not a bad idea. It would need to be separated from the files being saved to disk in order to avoid confusing the display with lots of files the user doesn't necessarily care about popping in and out of the queue.

Another addition to the original list: Only download files when the network/Dialup connection is less than (insert % here) of maximum capacity. The download manager should check the network connection maybe every 30 seconds or so to make sure that it isn't being used before using up all the bandwidth. It should first check while downloading (if downloading from a slow site it may not be using much bandwidth) and then (if necessary) pause downloading and check again, to make sure some other process isn't trying to use the connection.

original proposal including UI is at http://mozilla.org/xpapps/MachVPlan/DownloadMgr.html

Comments on the UI posted at http://mozilla.org/xpapps/MachVPlan/DownloadMgr.html Open should be grouped with Show. Delete is way too ambiguous. For xfers in progress, is should be 'Cancel', for completed downloads, rename it to 'Clear' or 'Remove', and group with 'Clear all completed'. Perhaps make a seperate frame for completed Transfers? Napster/gnutella style. Need a way to pause all downloads, to regain your bandwidth for another more important task. This would be a manual way to address the background downloading Karthik Sheka requested, and much easier (XP!) to implement by 1.0. Will this be an optional component, or replace the current download dialogs? I like having a choice, but in this case, I wouldn't mind seeing the individual dialogs dead and buried. What's the plan for UNIX? Many of the features would require depending on GNOME and/or KDE. (Show Location, icons, maybe more) > The image above shows the use of text labels although icons would be more > appropriate. Do you mean text & icons, or JUST icons? I seriously doubt you can come up with images for all of those things that are self explainatory. (Yes, tooltips, blah... what an awful way to learn an interface. The only freaking toolbar buttons in MS Word I use are bold/italic/underline, because I can't be bothered to memorize the rest)

doesn't the "pause" function make this bug dependant on bug 107552 ?

->blake

what are the chances this will make 098?

This is going to land in 0.9.9. It was temporarily put aside when I got word that law was rewriting how we handle download errors, and other related code, as that impacts download manager code, and because I discovered one other rdf bug that waterson fixed a few days ago.

Is there a screenshot of your implementation that you could post?

Does this still have a chance of making it into 0.9.9?

law, can you please take a look at the files in xpfe/components/download-manager/ src/? (I have updates to the FE to checkin and nsDownloadProgressListener.js, but I need to leave right now.)

Sorry, Blake. I just didn't have many spare cycles to devote to this today. A quick glance at the files raises these issues: (*) I'd prefer to see some sort of open/close methods in nsIDownloadManager that would be used to open/close the download manager UI. That makes it easier to plug in a new implementation of of the overall component (e.g., one of those infamous "3rd party download managers"). As it stands, client code is hard-coding dependencies on the implementation of the UI. (*) nsIDownloadItem seems like it needs more attributes. E.g., an nsIMIMEInfo or something so a helper app can be opened? Also, there isn't enough info in that object to initiate a download (I don't know where to look to find out exactly what operations can be performed on these things). (*) The interfaces need some documentation describing how they are to be used and how the pieces fit together. At a minimum there should be a block comment for each interface that says what the heck it is used for, what typical client code does, and how the other interfaces fit with it. In this case, I'd be interested in knowing how the current download code (in the uriloader and webbrowserpersist) fit with this. (*) Another example: What the heck is an nsIDownloadProgressListener and how does it fit with the other pieces? (*) Using the destination file path as a key is shaky, due to the fact that these aren't necessarily unique on Mac. Maybe we could use the "internal" name like what is stored in prefs? (*) The remote xml data source loading is done asynchronously. From experience, this means trouble! As I recall, there are two problem areas: a. You can sometimes get two instances of what you want to be a single data source. See how the code in the nsExternalHelperAppService and the helper app pref panel evolved over time to battle this issue. That code now relies on rdfService->GetDataSource to ensure that the same data source is acquired all the time. b. If you load asynchronously, then subsequent requests to find info won't find it. This can confuse things greatly. If we do an asynch load, then we need to register some sort of callback and/or listener and somehow fend off requests that come in prior to the data loading (e.g., reject addItem calls if the data source hasn't loaded yet). It looks like the UI stuff is real old and not complete? I'm holding off on reviewing that for now. Can I build this and run it somehow?

Yeah the UI stuff is old, I need to checkin the updates but had to run last night. And now school calls, but... The download manager doesn't initiate downloads, the client does it, for cases like AIM who want to handle their own downloads. (I thought you already knew this...Ben sent me mail long ago saying `so here's what we're doing for download manager') This is also why there may not seem to be enough attributes, like for helper apps. So with respect to your 'what happens to current downloading code,' well, not much. What download manager basically does is simple. The client gives it information about the file it's downloading, and it becomes the primary nsIWebProgressListener for the transfer that the client sets on its downloader. It then forwards the progress information to three listeners, an optional listener specified by the client to update client UI, a listener to update the download manager UI, and a listener to update the properties dialog. nsIDownloadProgressListener is a js-implemented interface that I'll checkin when I get home. It is basically a lot like the nsProgressDlg.js currently in the tree, but I needed to create a new one because it has to be smart about which download to update (the current nsProgressDlg.js is very hardcoded about just updating some fields in the UI with the information that comes in.) I recall trying to just extend nsProgressDlg.js to handle, but finding out that the amount fo work necessary to force code sharing wasn't worth it. You're right, the datasource needs to be loaded synchronously.

I like the Open/Close idea. It is much cleaner than openDialog'ing downloadManager.xul and then having that call initializeUI because, as you said, we can easily hook in other managers (if such a day ever comes), among other reasons. I intend to fix that when I get home (I'm at school now, the wonders of technology!) so there's not much point in you continuing to look over the code until I do. However, hopefully you can provide insight on the interaction between transfer components and download manager. I think AIM is something of a special case. Perhaps we should let clients specify a save and a cancel callback, and if they don't, default to using nsIWebBrowserPersist.

>The download manager doesn't initiate downloads, the client does it, for cases >like AIM who want to handle their own downloads. (I thought you already knew >this... Right; maybe I'm thinking of mpt's download manager spec where there were pause/resume options, restart download, queue till later, etc. What I tried to say was I don't remember the details of Ben's spec. Is there no pause/resume (for ftp), launch/show-location (which, now that I think of it, just require the output file)? pause/resume requires the nsIRequest, I think. >What download manager basically does is simple. The client gives it >information >about the file it's downloading, and it becomes the primary >nsIWebProgressListener for the transfer that the client sets on its downloader. > It then forwards the progress information to three listeners, an optional >listener specified by the client to update client UI, a listener to update the >download manager UI, and a listener to update the properties dialog. That's the kind of info that should be at the top of nsIDownloadManager.idl. I'm still unclear on how some things would work. For example, there's a "cancel" button in the download mgr's UI, right. What happens when the user clicks that? >nsIDownloadProgressListener is a js-implemented interface that I'll checkin >when I get home. It is basically... This info would be easier for the next person if it were in the .idl file in addition to this bug comment :-). re: cribbing code from nsProgressDlg.js You might look at nsProgressDialog.js, which perhaps has slightly more structure and would be easier to cut/paste code from. Also, it handles some stuff better vis-a-vis making sure completions are detected (and the handling of errors).

>However, hopefully you can provide insight on the interaction between transfer >components and download manager. Yes, I'm well versed on this topic having sorted through it all for bug 27609. I'll dive into that ASAP. > I think AIM is something of a special case. >Perhaps we should let clients specify a save and a cancel callback, and if they >don't, default to using nsIWebBrowserPersist. Yes, something needs to be there for that (I mentioned the cancel case in my previous comment). It is hard to see how the download manager ties into actual downloads from looking at the interface. There's doesn't appear to be anything in the nsIDownloadItem that the download manager can glom onto to do anything useful with. I'm still scratching my head over this. It seems like the .idl files should explain how the interfaces fit together, if nothing else. I'm trying to figure out how I'd modify the code in contentAreaUtils.js to use the download manager. That code creates an nsIWebBrowserPersist. I guess I'd get the nsIDownloadManager service. But then what do I do? The only method that looks plausible is addItem. So I cobble together one of those and call it. But how the heck does the download manager get the progress notifications from the webbrowserpersist? Does one have to do some magic like fetch one of the listener attributes from the download item *after* one adds it, and pass that to the webbrowserpersist? That's way too obtuse, but I can't see anything else more obvious.

Bill, I made many changes today to download manager. Can you take a look at the files in xpfe/components/download-manager/ again? I think you will find them an improvement, especially considering some of them are modelled around your new nsIProgressDialog ;-)

nsbeta1+

Note, if you're still around tonight: it takes lxr about 2-3 hours to show the latest version of a file, and I checked in numerous times tonight (last time a few minutes ago). So you should either pull the files yourself or wait until tomorrow.

I was out yesterday; I see the new files and will be looking at them a little later today.

OK, the new versions of the .idl helped immensely. Remaining questions/issues: (1) I don't quite understand nsIDownloadManager::onClose. The comment says "Called when the download manager front end is closed. Useful for third party managers to let us know when they've closed." I'm guessing the front-end *must* call this when it closes (for some reason)? And I'm unclear as to why some other (3rd party) download manager would need to talk to our implementation at all. But maybe you mean a 3rd party manager front-end or something? (2) The argument on open is an nsIDOMWindowInternal. This is more restrictive than just being an nsIDOMWindow (from an embedding perspective). And can aParent be null to make the download manager window a top-level window? (3) re: removeDownload: There was just a news story yesterday about MS getting flak for storing the names/locations of downloaded media files. When I saw that I thought: "Oh oh, that's our download.rdf file!". Do we need to consider the privacy ramifications of storing download information? (4) re: nsIDownloadItem (in general): Now I understand how this stuff fits together. Making this interface inherit from nsIWebProgressListener explained a lot. But I'm still uneasy about the execessively close relationship between the nsIDownloadManager implementation and the nsIDownloadItem information. There seems to be a lot of assumptions about how nsIDownloadItem will work, beyond the interface expressed by it's idl declaration. Worse, there's this code in nsDownloadManager.cpp: 298 DownloadItem* item = NS_STATIC_CAST(DownloadItem*, aDownloadItem); (http://lxr.mozilla.org/seamonkey/source/xpfe/components/download- manager/src/nsDownloadManager.cpp#298) Basically, this code will only work with one implementation of nsIDownloadItem. I don't have a problem with that, per se, but I don't think the interface should give the illusion that it works with any old implementation of that interface. One way to fix that is to add some sort of createDownload() method to nsIDownloadManager and have that method create an instance of the proper class. If we don't register our implementation of nsIDownloadItem with the component manager, then the caller won't have a way of getting a bogus one (without a whole lot more work). BTW, checking the result of the NS_STATIC_CAST won't ever fail, since we implement that (I believe), using reinterpret_cast<> (or equivalent). So I think we need to examine this issue a little closer. One other thing: What's the story with using the target file path as the key in the RDF data source (and the implementation of nsIDownloadManager's hash table)? I'm still under the impression that this is technically broken on the Mac. I've started to look at the implementation, but I thought I'd give you this feedback before I've finished with the rest of it.

> nsIDownloadManager::OnClose I'm not fond of this. But we need some way to know that the front end is no longer being displayed, so we stop forwarding the notifications to the ui progress listener. I do mean a third party front end. > The argument on open is an nsIDOMWindowInternal. You must have a slightly older version. I already fixed this. Also, I can make it so that parent can be null. > Do we need to consider the privacy ramifications of storing download > information? How are we supposed to have an across-session download manager if we don't store any download information? Storing it unencrypted is another story, I guess...we store things like location bar history and global history in fairly plain view as well. > target path as key You're right; it's not unique on mac. I haven't solved this problem yet as I'm not sure what to use instead (see 'outstanding issues' list at top of nsDownloadManager.cpp). > nsIDownloadItem I'll try to fix it up. Thanks!

By the way, if you're beginning to look at the impl now, please ensure that you've got the latest version. (That you still had the idl with the nsIDOMWindowInternal arg leads me to believe you might not.)

An update for those watching this bug: law and I have been discussing download manager issues via e-mail and testing it for about a week, on a daily basis. So this has not been forgotten about.

Blake, can you attach a patch with changes to the existing code (and build system files)?

Mass-moving remaining Nav team 0.99 bugs to 1.0.

ADT2 per ADT triage team.

i did a brief round of testing with a test build provided by Blake on win2k. most of the questions/issues i ran into are already known, and i didn't crash or encounter Scary Regressions (thus far ;). i kind of brain-dumped my observations and test outline on an internal server: http://hopey.mcom.com/tests/download-mgr-test-build.txt. again, it's rather broad coverage, rather than seeking out edge cases. haven't posted here (or publically) yet, as it's rather long-winded / disorganized (and i'm feeling sleepy at the moment :)...

I've been looking over this stuff over the past week or two and am happy with it. I'm willing to provide sr=ben@netscape.com for this feature to be turned on if Bill is happy.

Have you dealt with: [11:00:33] <jag> Download Manager now builds on the mac [11:00:59] <jag> Too bad I can't test whether it works, since I'm still missing essential patches for the XP hooking up [11:01:13] <jag> But I'm sure blake will deal with that Where's the patch that you intend to check in to enable this? (After all, we need a patch to approve, and *that* patch needs review and super-review as does the code you've already checked in "NPOB".)

Comment on attachment 74542 [details] [diff] [review] patch for files that are part of the build >Index: uriloader/exthandler/makefile.win >@@ -29,6 +29,7 @@ > rdf \ > webshell \ > helperAppDlg \ >+ downloadmanager \ > progressDlg \ > plugin \ > unicharutil \ Watch the whitespace. Have the uriloader module owners approved this new dependency?

r=pink on the mac build changes.

dbaron: that dependency is stale, and can be removed. I've done so in my tree.

sr=blake on jag's mac build changes

Comment on attachment 74542 [details] [diff] [review] patch for files that are part of the build a=dbaron for trunk checkin, since if we're going to have this we're certainly better off getting more testing rather than less. Also checking the r/sr boxes based on the following comments on IRC (times today EST) and the comments in this bug (comment 42 and comment 44) since then: [14:22:58] <blake> dbaron: i've attached the patch to 102477. [14:23:36] <blake> dbaron: the only part that hasn't been r/sr'd is the mac build changes, because jag just sent me those this morning. can you r= those?

Comment on attachment 74673 [details] [diff] [review] fix minor post-landing stuff sr=hewitt

Comment on attachment 74673 [details] [diff] [review] fix minor post-landing stuff reorder the conditions in the if of case "cmd_openfile":

Comment on attachment 74673 [details] [diff] [review] fix minor post-landing stuff a=dbaron for trunk checkin

Okay, this has landed. Release notes: - yes, there is a very annoying column resizing bug. no, you can't resize most of the columns. this will be fixed when download manager is converted to outliner, which is simple to do except for bug 129327 (fix in hand) - there is a column picker, it just has no image (also because of tree) - sometimes you may notice that the right commands are not enabled, but they are if you switch selected items and switch back. this is also due to a tree shortcoming. Please file any bugs to the Download Manager component (as soon as its created, bug 128674).

One more thing: you currently access the manager through Tasks > Tools. This will change with the upcoming menu spec (it will be under Tools > Download Manager).

blake, you wanna dump this to newsgroups if there are bugs that already filed for this once 3-18 builds spin tomorrow?

In an attempt to stave off duplicate bug reports, let me note another thing: there will be preferences to allow you to change the behavior of download manager. For example, you'll be able to decide whether you want an individual progress dialog, the download manager, or nothing at all to appear when you download something. And right now, if you close/cancel the progress dialog, the download will get canceled (as it does in nightlies). Obviously, there will be a way to close the progress dialog without canceling the download (one of the advantages of a manager). This is not difficult to implement, it just needs input from UE.

now in all daily verif builds. *zap*