Closed Bug 402520 Opened 13 years ago Closed 11 years ago

[10.5] Make download stack bounce upon finished download

Categories

(Toolkit :: Downloads API, enhancement)

x86
macOS
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- .5-fixed

People

(Reporter: wenzel, Assigned: mstange)

References

Details

Attachments

(1 file)

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).
Product: Firefox → Toolkit
Flags: wanted1.9.2?
The code needed to do this is about one line long and is described here:
http://13bold.com/quick-tricks/bouncing-the-downloads-stack/
Duplicate of this bug: 499376
*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!)
(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.
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. :)
Seeing as that is a C API and not Objective C, probably.
(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?
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.
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)
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?
(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.
(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.
Attached patch v1Splinter Review
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #423420 - Flags: review?(sdwilsh)
Attachment #423420 - Flags: review?(joshmoz)
Comment on attachment 423420 [details] [diff] [review]
v1

r=sdwilsh
Attachment #423420 - Flags: review?(sdwilsh) → review+
Attachment #423420 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/19b64e822f04
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Comment on attachment 423420 [details] [diff] [review]
v1

This would be a nice to have on 1.9.2, and simple to backport.
Attachment #423420 - Flags: approval1.9.2.1?
Flags: in-litmus?
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?
(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.
Attachment #423420 - Flags: approval1.9.2.2? → approval1.9.2.3?
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?
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.
Duplicate of this bug: 560069
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
Attachment #423420 - Flags: approval1.9.2.4? → approval1.9.2.5+
You need to log in before you can comment on or make changes to this bug.