402520 - [10.5] Make download stack bounce upon finished download

Status: RESOLVED FIXED

(Toolkit :: Downloads API, enhancement)

Description

[Fred Wenzel [:wenzel]]

See bug 384068 comment 11:
When Apple downloads something into the new Leopard Download folder, it makes it bounce in the dock.

We should do that too.

(Yet this is a teeny little enhancement, and not really part of bug 384068).

Comment 1

[Ricardo]

The code needed to do this is about one line long and is described here:
http://13bold.com/quick-tricks/bouncing-the-downloads-stack/

Comment 3

[Craig Box]

*bump*

One line code fix, which is already attached to the bug; good usability win (at least for me - I never see the download manager!)

Comment 4

[Fred Wenzel [:wenzel]]

(In reply to comment #3)
> One line code fix, which is already attached to the bug; good usability win (at
> least for me - I never see the download manager!)

I'd like this to happen too, but it's neither a one-line fix (mind you, Firefox is not an ObjC application), nor is it attached to this bug as far as I can tell.

Comment 5

[Craig Box]

Would something like this work?

#include <CoreFoundation/CoreFoundation.h>

CFStringRef observedObject = CFSTR("/path/to/file");
CFNotificationCenterRef center = CFNotificationCenterGetDistributedCenter();
CFNotificationCenterPostNotification(center, CFSTR(com.apple.DownloadFileFinished"), observedObject, NULL /* no dictionary */, TRUE);

(I am aware that this technically increases the number of lines substantially, but hopefully a friendly Mac developer will take my stolen idea and turn it into a working patch. :)

Comment 6

[Craig Box]

(The docs on this are at http://developer.apple.com/Mac/library/documentation/UserExperience/Conceptual/PreferencePanes/Tasks/Communication.html)

Comment 7

[Shawn Wilsher :sdwilsh]

Seeing as that is a C API and not Objective C, probably.

Comment 8

[Markus Stange [:mstange]]

(In reply to comment #4)
> (mind you, Firefox is not an ObjC application)

It is, in parts. For example the directory widget/src/cocoa is full of Objective C code.
The hard part about this bug, as far as I can tell, is getting from cross-platform code that knows about finished downloads to platform-specific code that can use Objective C. We need to decide what interfaces to extend (or create).
I'm used to adding methods to nsIWidget, but that seems like entirely the wrong interface...
Maybe nsIAlertsService, which also handles Growl integration? After all, the bounce animation is some kind of alert. What are your thoughts, Shawn?

Comment 9

[Eric Shepherd [:sheppy]]

IMHO adding a method to nsIAlertsService seems like a good idea, perhaps called showDownloadCompleteNotification(). This could accept as input the URL of the downloaded file, and could handle all the calling through to showAlertNotification() for the completed download, plus on Mac bounce the Dock icon.

Comment 10

[Shawn Wilsher :sdwilsh]

I'm not sure the alerts service is the right place.  This sounds like a very specific alert, and it is only for one platform.  If we can use the CoreFoundation API, we can add that code here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/src/nsDownloadManager.cpp#2195 (before or after the windows specific code)

Comment 11

[Markus Stange [:mstange]]

Good, that should work, too. I'm no fan of #ifdefs but as there are already some for Windows I guess it doesn't matter. Do you want to do it, Shawn?
We probably need to check whether the file really was downloaded to ~/Downloads before bouncing, right?

Comment 12

[Shawn Wilsher :sdwilsh]

(In reply to comment #11)
> Good, that should work, too. I'm no fan of #ifdefs but as there are already
> some for Windows I guess it doesn't matter. Do you want to do it, Shawn?
> We probably need to check whether the file really was downloaded to ~/Downloads
> before bouncing, right?
Yeah, this file is already fairly ifdefy, so I don't have a problem adding more here.  I switched to windows recently, so me making the patch is a little difficult.  And yeah, we should make sure that the file was downloaded to whatever nsDownloadManagner::GetDefaultDownloadsDirectory returns.

Comment 13

[Markus Stange [:mstange]]

(In reply to comment #12)
> And yeah, we should make sure that the file was downloaded to
> whatever nsDownloadManagner::GetDefaultDownloadsDirectory returns.

Actually, we don't even have to do that: we put the file's path into the notification which is used by OS X to decide whether it wants to bounce.

Comment 14

[Markus Stange [:mstange]]

Attachment: v1

Comment 15

[Shawn Wilsher :sdwilsh]

Comment on attachment 423420 [details] [diff] [review]
v1

r=sdwilsh

Comment 16

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/19b64e822f04

Comment 17

[Shawn Wilsher :sdwilsh]

Comment on attachment 423420 [details] [diff] [review]
v1

This would be a nice to have on 1.9.2, and simple to backport.

Comment 18

[Craig Box]

Thanks so much for moving so quickly on this.  If it were backported, does that mean it's likely to make it into Firefox 3.6.1 or similar?

Comment 19

[Shawn Wilsher :sdwilsh]

(In reply to comment #18)
> Thanks so much for moving so quickly on this.  If it were backported, does that
> mean it's likely to make it into Firefox 3.6.1 or similar?
Yes.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 423420 [details] [diff] [review]
v1

We'll need to evaluate this sort of UI change before we take it on the branch.

Is this consistent with the behaviour of other applications?

Comment 21

[Markus Stange [:mstange]]

Yes, it's native behaviour and consistent with at least Safari and Chrome. Conceptually we're only telling the OS that a download has finished - the fact that this results in a bounce is the decision of the OS.

Comment 23

[LegNeato]

Comment on attachment 423420 [details] [diff] [review]
v1

a=LegNeato for 1.9.2.5. Please ONLY land this on mozilla-1.9.2 default, as we
are still working on 1.9.2.4 on the relbranch

Comment 24

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/739f3aed6187