Closed
Bug 16498
Opened 25 years ago
Closed 20 years ago
Finished Downloading sounds
Categories
(SeaMonkey :: Download & File Handling, enhancement, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
Future
People
(Reporter: slschroe, Assigned: csthomas)
References
Details
Attachments
(3 files, 5 obsolete files)
980 bytes,
patch
|
janv
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
2.76 KB,
patch
|
janv
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
807 bytes,
patch
|
janv
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
In Navigator, I would love to have the computer make some sort of sound when a file is finished downloading. Often I am doing something else at the time, and am not at my computer. After a certain period of time, my screen goes blank and I have no way of knowing it's finished, other than going to my computer and reactivating the screen. This would be handy for sending e-mail messages, too. If I have large files to send, it can take a while, and I run into the same situation.
Updated•25 years ago
|
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → LATER
Summary: Finished Downloading sounds → SilentDownload: Finished Downloading sounds
Comment 1•25 years ago
|
||
Good idea. SilentDL will not be in 1.0 release. Latering.
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Moving from Silent Download (this component being deleted) to Installer.
Comment 3•22 years ago
|
||
LATER is deprecated per bug 35839.
Status: VERIFIED → REOPENED
Resolution: LATER → ---
Comment 4•22 years ago
|
||
->default
Assignee: dougt → dveditz
Status: REOPENED → NEW
QA Contact: gbush → bugzilla
Updated•22 years ago
|
Summary: SilentDownload: Finished Downloading sounds → Finished Downloading sounds
Comment 5•22 years ago
|
||
not installer. Sound like download manager to me
Assignee: dveditz → blaker
Component: Installer → Download Manager
QA Contact: bugzilla → sairuh
Updated•22 years ago
|
Target Milestone: --- → Future
Updated•22 years ago
|
QA Contact: sairuh → petersen
Assignee | ||
Comment 7•21 years ago
|
||
I'm working on this now.
Assignee | ||
Comment 8•21 years ago
|
||
If you don't have both of the 2 prefs set, you get no sound, and an error in your JS console. I suspect I should do a try{}catch() around the pref accesses, but I don't know for sure. Maybe someone will enlighten me. As far as I can tell, this works whether or not the specified finished_sound_url exists (if it doesn't, you just get a beep). Also, I don't know how to detect the difference between a cancelled download and a completed one, so with this patch, cancelling a download will also result in the sound playing. It seems to me that playing sound on cancel could be annoying, since the user already knows they're cancelling the download. This might only work for the download manager. I don't know if separate download dialogs use the same code or not.
Assignee | ||
Comment 9•21 years ago
|
||
-Doesn't cause any JS errors now. -Now detects the difference between a cancelled download and a completed one. -Definitely only work for the download manager. Progress dialogs don't use this code. I'll take a look at making the change in C++ instead of JS (I'm told that both download methods should share code there).
Assignee | ||
Comment 10•21 years ago
|
||
This patch applies to 2 different files. It works for progress dialogs and download manager. The only real issue I see is the need to maintain the same code in 2 locations. I have some somewhat-ugly C++ code that only needs to be applied to one file which I could clean up if this patch isn't acceptable.
Assignee | ||
Updated•21 years ago
|
Attachment #133495 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #133507 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #133813 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 11•21 years ago
|
||
Comment on attachment 133813 [details] [diff] [review] New patch >+ if (aStatus<0x80000000) //is there a better way to detect this? Components.isSuccessCode(aStatus) perhaps. See http://www.mozilla.org/scriptable/components_object.html#_isSuccessCode > >+ var sound = Components.classes["@mozilla.org/sound;1"] >+ .createInstance(Components.interfaces.nsISound); What if this fails?
Comment 12•21 years ago
|
||
Comment on attachment 133813 [details] [diff] [review] New patch As discussed on IRC.
Attachment #133813 -
Flags: review?(neil.parkwaycc.co.uk) → review-
Assignee | ||
Comment 13•21 years ago
|
||
Personally, I find these sounds somewhat annoying. How about a pref for minimum file size before playing a sound? For a big download, notification is good. When I save a bunch of small pages, notification is annoying. It looks like I could just check mMaxBytes and compare that to such a pref. Maybe that should be a separate bug (?).
Assignee | ||
Updated•21 years ago
|
Attachment #133813 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #134069 -
Flags: superreview?(bzbarsky)
Attachment #134069 -
Flags: review?(neil.parkwaycc.co.uk)
Assignee | ||
Updated•21 years ago
|
Attachment #134069 -
Attachment is obsolete: true
Attachment #134069 -
Flags: superreview?(bzbarsky)
Attachment #134069 -
Flags: review?(neil.parkwaycc.co.uk)
Assignee | ||
Comment 14•21 years ago
|
||
Fixed indentation, getting of null branch, and used null checks instead of NS_SUCCEEDED(rv). Sorry for the bugspam ;).
Assignee | ||
Updated•21 years ago
|
Attachment #134075 -
Flags: superreview?(bzbarsky)
Attachment #134075 -
Flags: review?(varga)
Comment 15•21 years ago
|
||
Comment on attachment 134075 [details] [diff] [review] Re-revised patch looks good r=varga
Attachment #134075 -
Flags: review?(varga) → review+
Comment 16•21 years ago
|
||
This needs module owner approval from a UI owner before I'll even look at it...
Assignee | ||
Comment 17•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #134076 -
Flags: superreview?(bzbarsky)
Attachment #134076 -
Flags: review?(varga)
Updated•21 years ago
|
Attachment #134076 -
Flags: review?(varga) → review+
Comment 18•21 years ago
|
||
Marking blocker as ideally we only want finish sounds for true downloads rather than when opening helper apps. (But feel free to check this in first ;-)
Depends on: 132456
Updated•21 years ago
|
Attachment #134076 -
Flags: superreview?(bzbarsky) → superreview+
Comment 19•21 years ago
|
||
Comment on attachment 134075 [details] [diff] [review] Re-revised patch >Index: xpfe/components/download-manager/src/nsDownloadManager.cpp >+ if (!StringBeginsWith(soundStr, NS_LITERAL_CSTRING("file://"))) What if it's a jar:// url? Or a chrome:// one? Or resource:// ? For that matter, is there a reason this can't work with http:// ? Or data: ?
Assignee | ||
Comment 20•21 years ago
|
||
Re comment #19: Since there doesn't seem to be a reliable way to detect system sounds, this always uses a URL now. I tested it with file:// and http:// URLs.
Assignee | ||
Updated•21 years ago
|
Attachment #134075 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #134186 -
Flags: superreview?(bzbarsky)
Attachment #134186 -
Flags: review?(varga)
Comment 21•21 years ago
|
||
Comment on attachment 134186 [details] [diff] [review] Ditch support for system sounds. looks good
Attachment #134186 -
Flags: superreview?(bzbarsky) → superreview+
Updated•21 years ago
|
Attachment #134186 -
Flags: review?(varga) → review+
Updated•21 years ago
|
Attachment #134075 -
Flags: superreview?(bz-vacation)
Comment 22•21 years ago
|
||
Updated•21 years ago
|
Attachment #134560 -
Flags: superreview?(bz-vacation)
Attachment #134560 -
Flags: review?(varga)
Comment 23•21 years ago
|
||
Comment on attachment 134560 [details] [diff] [review] Do we need to patch all.js too? I suppose we could do that.. though I'd prefer that we just put the try/catch around the pref get in the JS (which I forgot when I reviewed... :( )
Attachment #134560 -
Flags: superreview?(bz-vacation) → superreview+
Comment 24•21 years ago
|
||
Er, this is not JS. What am I smoking? In any case, we should find a better way of documenting prefs than bloating the pref hashtables.
Comment 25•21 years ago
|
||
Regarding comment #13, I agree this is a very good Idea, as I would want a sound for a download that is very quick. However, checking for filesize will to determine of we play a sound or not will have different effects depending of the type of connection. As an example, with my cable connection, sometimes I download files of a few megs in seconds only. I wouldn't like the sound for a download that only lasts a few seconds. Perhaps the threshold could be set at 5 seconds, or something similar.
Comment 26•21 years ago
|
||
Just a quick question: what's the status of this bug? I see some conflicting info: - According to http://www.mozilla.org/status/2003-11-10.html this feature should work now. - But I can't seem to get it to work on my WinXP box and i see no comments here about "patched checked in". Furthermore, this bug is also still marked as NEW. Thanks
Assignee | ||
Comment 27•21 years ago
|
||
Patrick, the code to support this feature is present, but you'll need to apply the all.js patch to add the pref "browser.download.finished_sound_url" and give it a value before a sound will actually be played. You'd probably want to set it to something like "file:///c:/Windows/Media/notify.wav". Someone still needs to review the all.js patch, and then I guess the next step is making this user-visible through one of the pref dialogs (if the UI people approve). I suppose that could be done in a separate bug if we wanted to resolve this one.
Comment 28•21 years ago
|
||
Chris: thanks for your reply. This was my bad :-\ I added the pref to user.js, thinking it would end up in all.js that way...sorry for the spam.
Comment 29•20 years ago
|
||
FWIW: one other thing, which perhaps is of use to others trying this. Initially i used the same syntax as for instance is used for the New Mail sound: user_pref("mail.biff.play_sound.url","c:\\NewMail.wav"); however it seems this pref requires the syntax as given by Chris in comment 27 "file:///c:/FileDownloaded.wav"
Assignee | ||
Comment 30•20 years ago
|
||
It's because I gave up on figuring out how to support the other format of sounds, and since then my free time has been reduced a lot. I don't think it would require more than 3 lines of code if someone wanted to try. Varga still needs to review+ the all.js patch too.
Comment 31•20 years ago
|
||
Another thing: could it be that the sound doesn't play while there's a SAVE AS dialog open?
Assignee | ||
Comment 32•20 years ago
|
||
Interesting... I guess the save dialog blocks download manager status changes.
Updated•20 years ago
|
Attachment #134560 -
Flags: review?(varga) → review+
Comment 33•20 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 25 years ago → 20 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•20 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 34•20 years ago
|
||
assigning bugs I fixed to myself
Assignee: firefox → cst
Status: REOPENED → NEW
Assignee | ||
Updated•20 years ago
|
Status: NEW → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•