The download manager is currently very unfriendly to embedders. For starters, we open a chrome url with it; for parenting, we look for a browser window at times; and for extension authors, we force it to open in a window. We can fix this by providing an interface to show the UI, and allow people to override it.

This doesn't seem to regress any existing behavior, and allows for embedders and extension authors to override this and control it how they see fit.

Hrm, shouldn't the new interface be used as a service? Is ifreq the usual way to expose windows to embedders.

(In reply to comment #2) > Hrm, shouldn't the new interface be used as a service? No other UI launchers are, and I'm not sure of the benefit of doing so (other than not having to create instance every time). Is ifreq the usual way to expose windows to embedders. Yes because they may not have a DOM window to pass in (they may have their own thing). Some older interfaces just have nsISupports, but biesi has told me that nsIInterfaceRequester is much cleaner.

(In reply to comment #3) > (In reply to comment #2) > > Hrm, shouldn't the new interface be used as a service? > No other UI launchers are, and I'm not sure of the benefit of doing so (other > than not having to create instance every time). which launchers? isn't that reason enough given that the download manager itself is a service (as in, you cannot have two DMs open).

nsIHelperAppLauncherDialog and nsIContentDispatchChooser to list off the ones I know of off-hand. I do see your argument for doing it as a service though. I can do that.

service instead

<mconnor> sdwilsh: I'd say so <mconnor> for not using the timer (answers Mano's question on irc)

Updated to trunk. I also moved the JS source file to a more sensible location.

Comment on attachment 278471 [details] [diff] [review] v1.2 r=mano with the custom factory removed; Expect regressions from the timer removal... :-/

for checkin

for whatever reason that last attachment isn't actually what I had locally... I'm checking in without the factory stuff.

(In reply to comment #9) > Expect regressions from the timer removal... :-/ Fully expected sadly :( Checking in toolkit/components/downloads/public/Makefile.in; new revision: 1.7; previous revision: 1.6 Checking in toolkit/components/downloads/public/nsIDownloadManager.idl; new revision: 1.20; previous revision: 1.19 Checking in toolkit/components/downloads/public/nsIDownloadManagerUI.idl; initial revision: 1.1 Checking in toolkit/components/downloads/src/Makefile.in; new revision: 1.15; previous revision: 1.14 Checking in toolkit/components/downloads/src/nsDownloadManager.cpp; new revision: 1.109; previous revision: 1.108 Checking in toolkit/components/downloads/src/nsDownloadManager.h; new revision: 1.36; previous revision: 1.35 Checking in toolkit/components/downloads/src/nsDownloadManagerUI.js; initial revision: 1.1 Checking in toolkit/components/downloads/src/nsDownloadProxy.h; new revision: 1.21; previous revision: 1.20

I'm requesting dev-doc-needed here since extension authors can override this implementation and create their own UI or put the existing UI in a tab. Not sure if we really need to document it though :/

I believe this caused bug 393982

What documentation beyond what's linked to from http://developer.mozilla.org/en/docs/Firefox_3_for_developers#Download_manager is needed? I'd like some input on that before I determine what more I should write up.

Basically, if an add-on author wants to provide a different UI for the download manager (that overrides the default one), they need to implement nsIDownloadManagerUI. At some point, I plan on looking over all the docs for the download manager and sprucing up the empty spots still...

The nsIDownloadManagerUI interface now has reference docs: http://developer.mozilla.org/en/docs/nsIDownloadManagerUI Marking as complete.