The nsIDownloadManager interface will continue to be available on the next ESR train (Gecko 24), but will be deprecated starting from Gecko 26 and will be decommissioned when most products have migrated to the new Downloads.jsm API. It seems that Thunderbird uses the pristine Toolkit Download Manager code, without many customizations. The two possible ways forward are for Thunderbird to continue to use it, and move all the code to comm-central, or to migrate to Downloads.jsm, that has better performance but would require a new interface to be developed (if download management is wanted). Moving the old code to comm-central has the implication that it would need to be maintained with possibly non-trivial changes, in case other parts of the download mechanism (like nsIExternalHelperAppService) also change to improve performance and accommodate new features.

Paolo any chance for you to produce a patch ?

We should first determine in which direction to go. The Downloads.jsm migration requires a user interface implementation that I can't commit to doing myself, though it is a very interesting project on which I'll gladly help with any information I can provide. If the outcome is that the nsIDownloadManager code will be moved as is, I'll help with the operation when the time comes.

I'd definitely favor moving to Downloads.jsm

Would it be difficult to port the Toolkit Download Manager code to using Download.jsm?

(In reply to Marco Castelluccio [:marco] from comment #4) > Would it be difficult to port the Toolkit Download Manager code to using > Download.jsm? The old user interface code accessed SQLite directly, but apart from that, new code that uses Downloads.jsm would be much simpler than the old one.

(In reply to :Paolo Amadini from comment #5) > (In reply to Marco Castelluccio [:marco] from comment #4) > > Would it be difficult to port the Toolkit Download Manager code to using > > Download.jsm? > > The old user interface code accessed SQLite directly, but apart from that, > new code that uses Downloads.jsm would be much simpler than the old one. I see. In your opinion, would it be better to create different download manager modules for the different products or a single one shared in toolkit? (I mean when the needs of the products are the same).

(In reply to Marco Castelluccio [:marco] from comment #6) > I see. In your opinion, would it be better to create different download > manager modules for the different products or a single one shared in > toolkit? (I mean when the needs of the products are the same). It depends on the complexity and user interface needs, a shared front-end module could work better if the standalone window becomes complex enough or the interface styling needs are the same. If the front-end code is simple enough and there are different styling needs, separate modules may be more maintainable. Any functionality that is not a purely front-end one should be implemented directly in the Downloads API anyways.

Adding common search terms to summary for easier retrieval & to avoid duplicates

(In reply to Magnus Melin from comment #3) > I'd definitely favor moving to Downloads.jsm +1. I'm also strongly in favour of migrating to the new js downloads manager with new UI per this bug, as this will solve a whole range of UX problems around the current old nsIDownloadManager used by TB, which is mostly unavailable or even empty when it's needed. So who can code this for TB? I've seen Joshua Cranmer [:jcranmer] doing great work in structurally similar Bug 858337 - Implement header parsing in JSMime... Doing some duplicate code path cleanup along the idea of Bug 549719 would also be very promising in this respect (probably for both FF and TB). Fyi: Meta Bug 825588 - (jsdownloads) Asynchronous JavaScript API for downloads

CC Joshua :) (In reply to Thomas D. from comment #9) > (In reply to Magnus Melin from comment #3) > > I'd definitely favor moving to Downloads.jsm > > +1. I'm also strongly in favour of migrating to the new js downloads manager > with new UI per this bug, as this will solve a whole range of UX problems > around the current old nsIDownloadManager used by TB, which is mostly > unavailable or even empty when it's needed. > > So who can code this for TB? I've seen Joshua Cranmer [:jcranmer] doing > great work in structurally similar Bug 858337 - Implement header parsing in > JSMime... > > Doing some duplicate code path cleanup along the idea of Bug 549719 would > also be very promising in this respect (probably for both FF and TB). > > Fyi: Meta Bug 825588 - (jsdownloads) Asynchronous JavaScript API for > downloads

(In reply to Thomas D. from comment #9) > So who can code this for TB? I've seen Joshua Cranmer [:jcranmer] doing > great work in structurally similar Bug 858337 - Implement header parsing in > JSMime... No, that is not structurally similar code. And I have no time to implement this; working on JSMime takes a very large portion of my spare time and it is still a multi-year endeavor.

(In reply to :Paolo Amadini from comment #7) > (In reply to Marco Castelluccio [:marco] from comment #6) > > I see. In your opinion, would it be better to create different download > > manager modules for the different products or a single one shared in > > toolkit? (I mean when the needs of the products are the same). > > It depends on the complexity and user interface needs, a shared front-end > module could work better if the standalone window becomes complex enough or > the interface styling needs are the same. > > If the front-end code is simple enough and there are different styling > needs, separate modules may be more maintainable. Any functionality that is > not a purely front-end one should be implemented directly in the Downloads > API anyways. I think Thunderbird would be pretty happy if there would be a shared front-end - it's after all a low priority for a mail client, even though we obviously want it to work. If there's not a shared front-end we just end up copying the files possibly with minor changes. For showing a list of downloads I'd suggest something like showing a tab with what ff has in about:downloads would be just fine. I'm not sure what to do about the download indicator, it would be nice to have. Maybe add it next to the appmenu like firefox... xref bug 726444 as the bug that implemented the new dl ui for firefox i guess.

I agree with Magnus that showing the about:downloads tab is a good first step. Porting (or, even better - moving to toolkit) the downloads indicator would be the next step. I can easily see both being usable in Thunderbird (though I can already hear the outcry of people missing the window). My 2c: 1) I think we should go with Downloads.jsm 2) I think we should start by attempting to move the about:downloads tab into toolkit where Thunderbird and other XUL apps can access it. 3) Once that's completed, we should see if we can move the indicator toolbar button itself to toolkit. If 2 or 3 is simply not possible, we might have to try a port - though that's probably the move of last resort. Any volunteers?

(In reply to Mike Conley (:mconley) from comment #13) > I agree with Magnus that showing the about:downloads tab is a good first > step. A word of caution, while porting most of the XUL elements is fine, the JavaScript behind those should be rewritten from scratch. It will be very easy, and much less work than porting any of the other existing front-ends in mozilla-central. The existing front-ends: - "about:downloads" integrates with Places and is thus not portable to Thunderbird. - Both the Downloads Panel and "about:downloads" use an unneeded legacy middle-layer. - Boot2Gecko uses a WebIDL API cross-app middle-layer that is not needed here. - The new Android front-end is probably the closest to what you need (bug 901360). The Android front-end has not landed yet, but its aboutDownloads.js implementation can be of inspiration, even though you might not need all its features.

Who might be able to fix this bug? I recall there was an interested party on some related bug... While this might look as an RFE at first sight, we are technically required to do this as the old stuff phases out, and practically it will solve a whole range of bad bugs in the current design which render the current TB implementation/default behaviour of download manager all but useless. Hence bug/major. For a tentative list of UX problems, see Bug 571858 Comment 3.

I have a will to take this, but before implementation I would like to clarify that "Saved files" view contains only attachment files, not messages. Right?

Yes, only attachments.

Hey hiro - I took a brief pass at this during the summit, and here was the stack that I got when breakpointing on adding a download via the nsIDownloadManager.idl: #0 nsDownloadManager::AddDownloadToDB (this=0x123c22950, aName=@0x11af868a8, aSource=@0x7fff5fbfb5d0, aTarget=@0x7fff5fbfb570, aTempPath=@0x7fff5fbfb4d0, aStartTime=1413409402041127, aEndTime=1413409402041127, aMimeType=@0x7fff5fbfb410, aPreferredApp=@0x7fff5fbfb470, aPreferredAction=4, aPrivate=false, aNewGUID=@0x11af868c8) at nsDownloadManager.cpp:833 #1 0x0000000105cb6530 in nsDownloadManager::AddDownload (this=0x123c22950, aDownloadType=0, aSource=0x11de22408, aTarget=0x118a3e600, aDisplayName=@0x10a500b98, aMIMEInfo=0x11ad7d3a0, aStartTime=1413409402041127, aTempFile=0x0, aCancelable=0x12943e378, aIsPrivate=false, aDownload=0x11e3fa798) at nsDownloadManager.cpp:1620 #2 0x0000000105e42a83 in nsDownloadProxy::Init (this=0x11e3fa780, aSource=0x11de22408, aTarget=0x118a3e600, aDisplayName=@0x10a500b98, aMIMEInfo=0x11ad7d3a0, aStartTime=1413409402041127, aTempFile=0x0, aCancelable=0x12943e378, aIsPrivate=false) at nsDownloadProxy.h:51 #3 0x0000000101700134 in nsSaveMsgListener::InitializeDownload (this=0x12943e360, aRequest=0x123be8240, aBytesDownloaded=32768) at /Users/mikeconley/Projects/comm-central/mailnews/base/src/nsMessenger.cpp:1740 #4 0x00000001017011aa in nsSaveMsgListener::OnDataAvailable (this=0x12943e360, request=0x123be8240, aSupport=0x11de22408, inStream=0x11ad99158, srcOffset=0, count=32768) at /Users/mikeconley/Projects/comm-central/mailnews/base/src/nsMessenger.cpp:1901 #5 0x000000010170150f in non-virtual thunk to nsSaveMsgListener::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long long, unsigned int) (this=0x12943e370, request=0x123be8240, aSupport=0x11de22408, inStream=0x11ad99158, srcOffset=0, count=32768) at /Users/mikeconley/Projects/comm-central/mailnews/base/src/nsMessenger.cpp:1938 #6 0x0000000101bd2f1e in nsMimeBaseEmitter::WriteHelper (this=0x123cc3d40, buf=@0x123ee30a8, countWritten=0x7fff5fbfbd6c) at /Users/mikeconley/Projects/comm-central/mailnews/mime/emitters/nsMimeBaseEmitter.cpp:488 #7 0x0000000101bd2c7a in nsMimeBaseEmitter::Write (this=0x123cc3d40, buf=@0x7fff5fbfbdf0, amountWritten=0x7fff5fbfbe1c) at /Users/mikeconley/Projects/comm-central/mailnews/mime/emitters/nsMimeBaseEmitter.cpp:446 #8 0x0000000101ba8214 in mime_output_fn (buf=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", size=54, stream_closure=0x118a10ac0) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimemoz2.cpp:942 #9 0x0000000101b9d85c in MimeOptions_write (opt=0x11e54e220, name=@0x7fff5fbfbf30, data=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", length=54, user_visible_p=true) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimei.cpp:1755 #10 0x0000000101b99454 in MimeObject_write (obj=0x114eed0b0, output=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", length=54, user_visible_p=true) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimei.cpp:1790 #11 0x0000000101b940ad in MimeExternalObject_parse_decoded_buffer (buf=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", size=54, obj=0x114eed0b0) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeeobj.cpp:220 #12 0x0000000101b8fca3 in mime_decode_base64_buffer (data=0x118a83b00, buffer=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", length=0, outSize=0x7fff5fbfc070) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeenc.cpp:299 #13 0x0000000101b8f8bc in MimeDecoderWrite (data=0x118a83b00, buffer=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", size=72, outSize=0x7fff5fbfc070) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeenc.cpp:819 #14 0x0000000101b9f6db in MimeLeaf_parse_buffer (buffer=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", size=72, obj=0x114eed0b0) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeleaf.cpp:152 #15 0x0000000101b93fad in MimeExternalObject_parse_buffer (buffer=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", size=72, obj=0x114eed0b0) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeeobj.cpp:191 #16 0x0000000101bb53c7 in MimeMultipart_parse_child_line (obj=0x118f1b900, line=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", length=72, first_line_p=false) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimemult.cpp:658 #17 0x0000000101bb4684 in MimeMultipart_parse_line (line=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", length=73, obj=0x118f1b900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimemult.cpp:306 #18 0x0000000101b7d3da in convert_and_send_buffer (buf=0x116c2b400 "��Xz|��z�zW����\017������o��h�0\b1�1Xap��!nhg\030g8�p��i�\027FuaGcYZzjDcLPhacMX\n", length=73, convert_newlines_p=true, per_line_fn=0x101bb37f0 <MimeMultipart_parse_line(char const*, int, MimeObject*)>, closure=0x118f1b900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimebuf.cpp:152 #19 0x0000000101b7d181 in mime_LineBuffer (net_buffer=0x116c2ac00 "jPVYeny9xXr1elf13umP0g/U5+uX6+/Rb9f/aMAwCDHIMVhhcMjgviFuaGcYZzjDcLPhacMX\n\n", net_buffer_size=73, bufferP=0x118f1b938, buffer_sizeP=0x118f1b948, buffer_fpP=0x118f1b950, convert_newlines_p=true, per_line_fn=0x101bb37f0 <MimeMultipart_parse_line(char const*, int, MimeObject*)>, closure=0x118f1b900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimebuf.cpp:238 #20 0x0000000101bb6555 in MimeObject_parse_buffer (buffer=0x116c2ac00 "jPVYeny9xXr1elf13umP0g/U5+uX6+/Rb9f/aMAwCDHIMVhhcMjgviFuaGcYZzjDcLPhacMX\n\n", size=73, obj=0x118f1b900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeobj.cpp:240 #21 0x0000000101baf414 in MimeMessage_parse_line (aLine=0x116c2ac00 "jPVYeny9xXr1elf13umP0g/U5+uX6+/Rb9f/aMAwCDHIMVhhcMjgviFuaGcYZzjDcLPhacMX\n\n", aLength=73, obj=0x118c5e900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimemsg.cpp:204 #22 0x0000000101b7d3da in convert_and_send_buffer (buf=0x116c2ac00 "jPVYeny9xXr1elf13umP0g/U5+uX6+/Rb9f/aMAwCDHIMVhhcMjgviFuaGcYZzjDcLPhacMX\n\n", length=73, convert_newlines_p=true, per_line_fn=0x101baeff0 <MimeMessage_parse_line(char const*, int, MimeObject*)>, closure=0x118c5e900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimebuf.cpp:152 #23 0x0000000101b7d181 in mime_LineBuffer (net_buffer=0x123c06e26 "jPVYeny9xXr1elf13umP0g/U5+uX6+/Rb9f/aMAwCDHIMVhhcMjgviFuaGcYZzjDcLPhacMX\r\n", '�' <repeats 126 times>..., net_buffer_size=74, bufferP=0x118c5e938, buffer_sizeP=0x118c5e948, buffer_fpP=0x118c5e950, convert_newlines_p=true, per_line_fn=0x101baeff0 <MimeMessage_parse_line(char const*, int, MimeObject*)>, closure=0x118c5e900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimebuf.cpp:238 #24 0x0000000101bb6555 in MimeObject_parse_buffer (buffer=0x123c03000 "ICAgICAgICAgICAgICAgIDw/eHBhY2tldCBlbmQ9J3cnPz7/4gxYSUNDX1BST0ZJTEUAAQEA\r\nAAxITGlubwIQAABtbnRyUkdCIFhZWiAHzgACAAkABgAxAABhY3NwTVNGVAAAAABJRUMgc1JH\r\nQg", 'A' <repeats 15 times>, "QAA9tYAAQAAAADTLUhQIC", 'A' <repeats 14 times>..., size=15984, obj=0x118c5e900) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimeobj.cpp:240 #25 0x0000000101ba6554 in mime_display_stream_write (stream=0x118a3ab80, buf=0x123c03000 "ICAgICAgICAgICAgICAgIDw/eHBhY2tldCBlbmQ9J3cnPz7/4gxYSUNDX1BST0ZJTEUAAQEA\r\nAAxITGlubwIQAABtbnRyUkdCIFhZWiAHzgACAAkABgAxAABhY3NwTVNGVAAAAABJRUMgc1JH\r\nQg", 'A' <repeats 15 times>, "QAA9tYAAQAAAADTLUhQIC", 'A' <repeats 14 times>..., size=15984) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/mimemoz2.cpp:985 #26 0x0000000101bcf593 in nsStreamConverter::OnDataAvailable (this=0x12a51abc0, request=0x123be8240, ctxt=0x11de22408, aIStream=0x114e77880, sourceOffset=0, aLength=15984) at /Users/mikeconley/Projects/comm-central/mailnews/mime/src/nsStreamConverter.cpp:939 #27 0x0000000101f414ac in nsStreamListenerTee::OnDataAvailable (this=0x118c483d0, request=0x123be8240, context=0x11de22408, input=0x11ad99278, offset=0, count=15984) at nsStreamListenerTee.cpp:93 #28 0x0000000101aee678 in (anonymous namespace)::SyncRunnable5<nsIStreamListener, nsIRequest*, nsISupports*, nsIInputStream*, unsigned long long, unsigned int>::Run (this=0x1190a3060) at /Users/mikeconley/Projects/comm-central/mailnews/imap/src/nsSyncRunnableHelpers.cpp:250 #29 0x0000000101da3d36 in nsThread::ProcessNextEvent (this=0x100615b80, aMayWait=false, aResult=0x7fff5fbfd233) at nsThread.cpp:830 #30 0x0000000101df554b in NS_ProcessPendingEvents (aThread=0x100615b80, aTimeout=20) at nsThreadUtils.cpp:207 #31 0x00000001047c4f99 in nsBaseAppShell::NativeEventCallback (this=0x11355f240) at nsBaseAppShell.cpp:98 #32 0x000000010483c5f1 in nsAppShell::ProcessGeckoEvents (aInfo=0x11355f240) at nsAppShell.mm:362 #33 0x00007fff85c2eb31 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ () #34 0x00007fff85c2e455 in __CFRunLoopDoSources0 () #35 0x00007fff85c517f5 in __CFRunLoopRun () #36 0x00007fff85c510e2 in CFRunLoopRunSpecific () #37 0x00007fff8bc65eb4 in RunCurrentEventLoopInMode () #38 0x00007fff8bc65c52 in ReceiveNextEventCommon () #39 0x00007fff8bc65ae3 in BlockUntilNextEventMatchingListInMode () #40 0x00007fff87afb533 in _DPSNextEvent () #41 0x00007fff87afadf2 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #42 0x000000010483b287 in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x1135ffe50, _cmd=0x7fff88329404, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff743321c0, flag=1 '\001') at nsAppShell.mm:118 #43 0x00007fff87af21a3 in -[NSApplication run] () #44 0x000000010483cf67 in nsAppShell::Run (this=0x11355f240) at nsAppShell.mm:635 #45 0x0000000105dc17ac in nsAppStartup::Run (this=0x114e86650) at nsAppStartup.cpp:280 #46 0x0000000105e65d07 in XREMain::XRE_mainRun (this=0x7fff5fbff168) at /Users/mikeconley/Projects/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4095 #47 0x0000000105e6654f in XREMain::XRE_main (this=0x7fff5fbff168, argc=5, argv=0x7fff5fbffa88, aAppData=0x7fff5fbff418) at /Users/mikeconley/Projects/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4168 #48 0x0000000105e669ed in XRE_main (argc=5, argv=0x7fff5fbffa88, aAppData=0x7fff5fbff418, aFlags=0) at /Users/mikeconley/Projects/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4382 #49 0x0000000100001e13 in do_main (argc=5, argv=0x7fff5fbffa88, xreDirectory=0x100611c80) at /Users/mikeconley/Projects/comm-central/mail/app/nsMailApp.cpp:198 #50 0x0000000100001316 in main (argc=5, argv=0x7fff5fbffa88) at /Users/mikeconley/Projects/comm-central/mail/app/nsMailApp.cpp:383 So it looks like (beyond opening the download window) Thunderbird never makes direct reference to the nsIDownloadManager - instead, it goes through nsDownloadProxy, which implements nsITransfer.

Thanks Magnus and Mike. Both of your replies were very quick! The stack trace was very helpful for me. I was wondering how nsIDownloadManagerUI knows downloaded files. As far as I tested before no attachment files appeared on the download manager window! (There was a bug, bug 914517 actually...) I could go step a forward now. Here are steps I am planning about migration. 1. Create about:downloads page using DownloadLegacy.js as both of firefox desktop does firefox android will do. 2. Create MsgDownloads.jsm using Downloads.jsm directly so that we can put sender name or something else in the "Saved files" view. 3. Replace nsIMessenber.saveAttachment (and other methods) with the MsgDownloads.jsm. These steps will be split into smaller pieces. Last night I wrote some codes for about:downloads. I will open a bug for the first step soon.

So Thunderbird has the new Saved Files TAB these days. Is there anything to still do in this bug?

The download indicator with related ui was mentioned. But then we should update the bug summary.

(In reply to Magnus Melin from comment #22) > The download indicator with related ui was mentioned. But then we should > update the bug summary. Cool, I've seen this is being used as a tracking bug so I've kept it open, but I just clarified the dependencies in that Thunderbird won't be affected by the decommissioning of nsIDownloadManager anymore.