User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2b) Gecko/20021027 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2b) Gecko/20021027 In "Notifications", I check "Play a sound", "Custom wav file" and chose my wave file; when I hit "Preview", I hear nothing but the awful standard "beep". Same when I receive an email :( Btw, I suggest that, for each account, a different action (not necessarily play a sound, in fact) can be done. This could be mixed with message filters. Reproducible: Always Steps to Reproduce: 1. Edit notifications 2. Choose a .wav file 3. Hit preview Actual Results: Nothing Expected Results: Play a wav

->Mailnews

Yes. This happens to me too ... ever since the 1.2 b (Oct 16, I think). Russel

I top my machine and Moz seems to require more and more memory. I kill it before my machine feels bad.

Comment on attachment 104847 [details] strace -o when it bugs Sorry!!! This attachment concerns bug #177107

Comment on attachment 104848 [details] top: memory leak? Sorry! This attachement concerns bug #177107 and I don't know how to remove it :(

If I choose a custom sound notification through the browse button and then type file:// in front of the absolute path to the custom wave file, also the "awful" beep doesn't come anymore when hitting the preview button. I hope it helps in any way!

i tried adding file:// in front of the path but this doesn't work...

reassigning to the correct person

From http://bugzilla.mozilla.org/show_bug.cgi?id=64462#c148: ------- Additional Comment #149 From Seth Spitzer 2002-09-19 16:41 ------- I need to fix the path handling code. see bug 169414 I need to make it work for both file:// and native paths the side effect of this bug is that on linux, any native path will be the system beep. Sebastian, Ian, are you both on Linux? When trying to add file:\\ (or file:\\\), see bug 180009 for that.

Are there any news about MacOS X and file path to change the sound beep to a custom wav file?

*** Bug 194632 has been marked as a duplicate of this bug. ***

Setting All/All per comment 12.

This same bug is seen on Solaris.

Does any moziila-sound work in Mozilla? I only get beeps. When receiving emails (not just when testing the settings), when popups block (see bug #191614 & bug #195929)...

*** Bug 180009 has been marked as a duplicate of this bug. ***

Isn't this a dup of bug 169414.

NS_NewStreamLoader is failing since IOService.NewChannelFromURI() fails if URI is created from a StandardURL. If the URI is created using IOService.NewURI() then the NS_StreamLoader() works. nsStatusBiffManager fails because nsIFileURL cannot be QIed from a StandardUrl.

over to dmose, our linux + mailnews guy.

Comment on attachment 119843 [details] [diff] [review] Fixes notification sound problems on linux I am trying to find someone who will review the patch.

Just adding those (unsigned char) casts is not enough because you didn't hit all the uses of s[...]. I suggest you replace these macros with inline functions which take an unsigned char* parameter. Macros evil! Other than that, looks good. I'll sr a new patch. But I'm not a module owner here; try getting review from pavlov for the GTK stuff. I'll probably let review for the mailnews stuff slide, since it really has nothing to do with mailnews per se.

question: if the patch on this bug is only fixing sound on Linux, should we reopen the bug about sound not working on OS X which got duped to this one?

Yes, reopen the OSX bug.

Setting Platform/OS back to PC/Linux per comment 23. OS X is bug 194632.

Changes to nsStandardURL (bug 157135) causes the creation of the nsIFileURL to fail. The nsIFileURL is not really needed anyway.

Marc, are you going to update GTK part of the patch?

Yes, I am working on it. I am trying to make the gtk nsSound::PlaySystemSound work.

nsSound::Play recreates URI if NS_NewStreamLoader fails nsSound::PlaySystemSound now assumes that soundAlias is a native file path (similar to Windows). It creates a nsIFileURL using nsILocalFile.

Comment on attachment 121565 [details] [diff] [review] Make nsSound::Play and nsSound::PlaySystemSound work bleh, don't change those macros to functions, espically non-inlined ones. get rid of the spaces after (s and before )s. You should probably check aSoundAlias for NULL before calling strcmp in PlaySystemSound.

He SHOULD be using functions, but they should be marked inline. Using macros it's too easy to get the signed-ness of the char* parameter wrong.

Robert suggested that I make functions instead. Back to macros now.

Comment on attachment 121575 [details] [diff] [review] Sound patch v2 roc: theres no reason to use inline functions for things like this. These are very localized macros and its pretty clear what types are getting operated one.

Did you notice the bug where we were passing a 'char*' as the macro parameter instead of 'unsigned char*' which was causing an error when we tried to OR values together?

Comment on attachment 121575 [details] [diff] [review] Sound patch v2 If we have to use macros, at least put parentheses around the macro bodies to avoid potential problems.

Comment on attachment 121547 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp sr=roc+moz I guess Seth should check this.

Comment on attachment 121547 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp assuming win32 and mac os x are ok, r=sspitzer before checking in, please change: soundURL->SetSpec( soundURLSpec ); to soundURL->SetSpec(soundURLSpec);

I don't have this platform so I cannot compile or test it.

I don't have this platform so I cannot compile or test it.

Removed extra whitespace.

mac implementation of nsSound doesn't use NS_NewStreamLoader so it is ok.

Comment on attachment 121696 [details] [diff] [review] nsSound::Play() patch for OS/2 sr=blizzard

Comment on attachment 121696 [details] [diff] [review] nsSound::Play() patch for OS/2 You really need mkaply or javier to review this, not me. In general, though, since you're on the stack, it's better to use an nsCAutoString for spec, and you also need to check return values from aURL->GetSpec() and NS_NewURI.

Comment on attachment 121697 [details] [diff] [review] nsSound::Play() patch for Windows Note that this code has the same issues as the OS/2 patch.

use nsCAutoString and check return value of GetSpec and NS_NewURI

Comment on attachment 122336 [details] [diff] [review] nsSound::Play() patch for OS/2 v1.1 sr=blizzard

Comment on attachment 122337 [details] [diff] [review] nsSound::Play() patch for Windows v1.1 sr=blizzard

Comment on attachment 121698 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp what happens now if the file doesn't exists? (does Play() return a failure for all platforms?)

Use NS_NewURI and QueryInterface to create nsIFileURL. Creating a nsIFileURL from STANDARDURL doesn't work anymore (bug 157135).

Comment on attachment 122872 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp v2 >Index: nsStatusBarBiffManager.cpp >=================================================================== >RCS file: /cvsroot/mozilla/mailnews/base/src/nsStatusBarBiffManager.cpp,v >retrieving revision 1.26 > > [...] > >- >- rv = soundURL->SetSpec(soundURLSpec); >- if (NS_SUCCEEDED(rv)) { >+ nsCOMPtr<nsIFileURL> soundURL = do_QueryInterface(fileURI); >+ if (soundURL) { Use do_QueryInterface(fileURI, &rv) and then check the value of rv, not of soundURL. Also, it doesn't look like Seth's previous question has been answered.

Use do_QueryInterface(fileURI, &rv) and check rv Regarding Seth's question: My previous Attachment 121698 [details] [diff] had removed the check for the existence of the wav file. Attachment 122872 [details] [diff] restored the original functionality which addressed Seth's concern.

Comment on attachment 123086 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp v2.1 sr=dmose@mozilla.org

Comment on attachment 122335 [details] [diff] [review] GTK sound patch v2.1 r=pavlov

Comment on attachment 122335 [details] [diff] [review] GTK sound patch v2.1 >Index: nsSound.cpp >=================================================================== >RCS file: /cvsroot/mozilla/widget/src/gtk/nsSound.cpp,v >retrieving revision 1.34 >diff -u -r1.34 nsSound.cpp >--- nsSound.cpp 8 Jan 2003 22:58:03 -0000 1.34 >+++ nsSound.cpp 2 May 2003 22:03:26 -0000 >@@ -181,13 +182,13 @@ > bits_per_sample = GET_WORD(string, i); > i+=2; > >- rate = samples_per_sec * channels * (bits_per_sample / 8); >+ rate = samples_per_sec; I'm not completely sure about this, but if you guys think it's right and it works, ok. > NS_IMETHODIMP nsSound::PlaySystemSound(const char *aSoundAlias) > { >- return Beep(); >+ if (!aSoundAlias) return NS_ERROR_FAILURE; >+ if (strcmp( aSoundAlias, "_moz_mailbeep" ) == 0) >+ { >+ return Beep(); >+ } >+ else >+ { No need for else-after return. Just end the |if| block. >+ nsresult rv; >+ nsCOMPtr <nsIURI> fileURI; >+ >+ // create a nsILocalFile and then a nsIFileURL from that >+ nsCOMPtr <nsILocalFile> soundFile = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv); >+ NS_ENSURE_SUCCESS(rv,rv); >+ >+ rv = soundFile->InitWithNativePath(nsDependentCString(aSoundAlias)); >+ NS_ENSURE_SUCCESS(rv,rv); >+ There's a shortcut for this: NS_NewNativeLocalFile(nsDependentCString(aSoundAlias), PR_TRUE, getter_AddRefs(soundFile)); Fix those and sr=bryner (note: I also asked darin to take a look at the stream loader usage here as I'm not that familiar with it).

Comment on attachment 122335 [details] [diff] [review] GTK sound patch v2.1 > nsCOMPtr<nsIStreamLoader> loader; > rv = NS_NewStreamLoader(getter_AddRefs(loader), aURL, this); >+ if (rv == NS_ERROR_NO_INTERFACE) >+ { >+ // the URL was created from a StandardURL, create a new URI that can be used with NS_NewStreamLoader >+ nsCOMPtr<nsIURI> uri; >+ nsCAutoString spec; >+ rv = aURL->GetSpec(spec); >+ NS_ENSURE_SUCCESS(rv,rv); >+ rv = NS_NewURI(getter_AddRefs(uri), spec.get()); >+ NS_ENSURE_SUCCESS(rv,rv); >+ rv = NS_NewStreamLoader(getter_AddRefs(loader), uri, this); >+ } > return rv; this looks like a major hack to me. what problem are you trying to work around? who is creating a nsStandardURL directly? we should be fixing that code instead of adding wacky workarounds like this in this code. >+ if (strcmp( aSoundAlias, "_moz_mailbeep" ) == 0) >+ { >+ return Beep(); >+ } >+ else nit: |else| after |return| is pointless. >+ return Play(fileURL); >+ } nit: do this instead: rv = Play(fileURL); return rv; this will help minimize footprint since the compiler will be able to coalesce all of the |return rv| statements into one.

Most of the calls to nsSound.Play() are in javascript/xul and they use standard-url interface to create the url. If those calls are changed and nsStatusBarBiffManager.cpp is patched then this bug would be fixed. Is the following appropriate to create a url in js? var ioService = Components.classes["@mozilla.org/network/io-service;1"] .getService(Components.interfaces.nsIIOService); var url = ioService.newURI(soundURL, null, null); If so, I can create a patch to update the javascript/xul files. The files involved are extensions/irc/xul/content/static.js, xpfe/browser/resources/content/navigator.js, mailnews/base/prefs/resources/content/pref-notifications.js, extensions/cookie/resources/content/pref-popups.xul.

>var ioService = Components.classes["@mozilla.org/network/io-service;1"] > .getService(Components.interfaces.nsIIOService); >var url = ioService.newURI(soundURL, null, null); this is mostly correct. is |soundURL| always something local? or can it be a remote file? if so, then you may need to worry about passing in the charset of the document from which |soundURL| was extracted as the second parameter to newURI. darin

The url for mail notification is always local.

Create a url using IOService.NewURL(). This will allow nsSound.Play() to work without modification.

Comment on attachment 124034 [details] [diff] [review] Patch for pref-notifications.js r=dmose@mozilla.org

Comment on attachment 124034 [details] [diff] [review] Patch for pref-notifications.js this looks fine to me, but i suspect things will fail if the user enters a non-ASCII file:// URL... but that is probably not something to worry about in this bug.

Comment on attachment 124034 [details] [diff] [review] Patch for pref-notifications.js sr=darin (provided seth is happy with the change too)

Comment on attachment 123086 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp v2.1 Can I get an review?

Comment on attachment 123086 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp v2.1 Can I get a review?

Comment on attachment 123086 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp v2.1 Can I get a review?

I told asa that I'd review this today (6/3), but I want to test on win32 first. thanks for being patient.

Comment on attachment 123086 [details] [diff] [review] Patch for nsStatusBarBiffManager.cpp v2.1 sorry for the delay. r=sspitzer, tested this on win32, both c:\ and file:// urls (tested preview and and the biff code) this has a=sspitzer/asa for 1.4. I'll land on trunk and branch.

over to mloiselle@yahoo.com for credit.

Comment on attachment 124034 [details] [diff] [review] Patch for pref-notifications.js a=sspitzer,asa

a=adt to land this on the 1.4 branch.

fixed on trunk and branch.

Trunk and Branch build 2003-06-05: Linux RH 8 Reopening. I select Edit|Preferences, go to Mail & Newgroups, Notifications panel. Select the Browse button to select a wav file, then select the Preview button and the custom wav file doesn't play, instead I hear the system file. Since this isn't working, it's not working in mail as well.

If you use file:///path/sound.wav for the wav filename, it will work on RH8. A sound setting of /path/file.wav requires bug 179138 to be fixed (patch is available, waiting for sr).

Setting back to Resolved/Fixed. Branch and Trunk build: Mac 10.1.5, WinMe - ok. Note: On WinMe using the form file:///path/sound.wav made it work. If I referenced the exact path c:\path\sound.wav then it didn't work. still need to check linux.

Branch and Trunk build 2003-06-09: Linux RH 7.2 (Esther's system) Verified Fixed.

Just for the record, this is still broken on OS X 10.2.6 with build 2003060908. But since it seems to be fixed for everyone else, I'll leave this alone, and we'll call the OS X problem bug 194632.

and before anyone asks, yes, I did use a file:/// style url to the wav file

Please go to bug# 194632 for updated results using WinMe and Mac builds.

Using linux trunk build 2003061008, this is still broken. attempting to use file:///usr/local/mozilla/res/samples/test.wav as the mail notification sound. Clicking the "Preview" button in the Mail & Newsgroups -> Notifications screen does nothing. Afterwards, I find the following in the Javascript console: Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISound.play]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://messenger/content/pref-notifications.js :: PreviewSound :: line 69" data: no] The following commands play the sound perfectly: aplay /usr/local/mozilla/res/samples/test.wav esdplay /usr/local/mozilla/res/samples/test.wav

Here is a possible workaround: In the browser load the wav file to get the file url. Copy the file url and then go to the Notification preference and paste the url. Comment# 75 stated that sound was working on WinMe and Mac 10.1.5 using the 2003-06-06 build. Then using today's build (2003-06-10) I noticed that it wasn't working. Using the copy/paste workaround plays the correct sound. I may have followed this procedure on 6/6 when first verifying this bug. Interesting... I compared the string I typed and the string that was pasted and they look the same.

Re: the workaround in comment 81, I was already using the file: form, and it does not work. file:///usr/local/mozilla/res/samples/test.wav

The gtk nsISound implementation uses the library libesd.so. Can you check that you have it? Try 'whereis libesd.so' to locate it. If you don't have it, you would get NS_ERROR_FAILURE. On my system (RedHat) there is a libesd.so -> libesd.so.0 in /usr/lib.

Ah! Thanks, Marc. Now that is indeed useful information. On Debian 2.2, libesd is installed as /usr/lib/libesd.so.0 (as part of the libesd0 0.2.23-3 package) I made a symlink from /usr/lib/libesd.so.0 to /usr/lib/libesd.so and then ran ldconfig. After that, Mozilla was able to play the sound with no problems. Thanks!

*** Bug 198864 has been marked as a duplicate of this bug. ***

*** Bug 211149 has been marked as a duplicate of this bug. ***

I've been using Mozilla on RedHat 8 for over a year now and I have never heard mozilla play the sound file correctly since I set it up (with preview or when receiving mail). I set up the sound when I installed mozilla 1.0 and I've upgraded at every stable release (now using 1.4final linux binary). Since using 1.4final, I've tried the file:// and regular path. I've tested esound and can play the wave files fine using esdplay. I do have /usr/lib/libesd.so. I get a system beep when NOT using the file:// path, but nothing otherwise. System new mail sound does not work either (is it even supposed to in linux+gnome? -- it should!) Is this really fixed? Are the above patches in 1.4final? Am I SOL?

It works for me on Moz 1.4, Mandrake 9.1, KDE if I use file:///home/rjn/mychime.wav But it doesn't (and never has) if I use /home/rjn/mychime.wav

Yow! It's a data dependent bug - this depends on the wav file!! By playing with sox, I've found the following are required to make it work for me: 1)Use file:/// in the path. 2)File can be mono or stereo, but must be at sample rate of 32kHz or below. Rates of 33kHz or above (such as the standard 44.1k) will not play in Moz using "preview", (although they are fine using 'play filename.wav' in bash) 3)Despite the file's specified data rate, they all play too fast. 4)Mono and stereo versions of the same file sound different in Moz (factor of 2 different speeds) The output of 'file chime_*' is: chime_32k_OK.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, stereo 32000 Hz chime_44k1_fails.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, stereo 44100 Hz chime_32k_OK-mono.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, mono 32000 Hz I've put the 3 files on the web - they are about 1 MB each. See: www.abia42.hemscott.net/chime_32k_OK.wav www.abia42.hemscott.net/chime_32k_OK-mono.wav www.abia42.hemscott.net/chime_44k1_fails.wav I hope this is useful! Richard P.S. I've tried several different source sound files and many different rates. My comments are true for all of them. I'm using Moz 1.4, Linux (Mandrake 9.1), KDE.

Richard and Jeremy, please see bug 179138. A patch for that bug is not in 1.4 (patch is approval1.4.x?). That patch is included in 1.5a.

*** Bug 214771 has been marked as a duplicate of this bug. ***

*** Bug 215638 has been marked as a duplicate of this bug. ***

Yeah, this is definitely working on Linux in 1.5... as I suddenly discovered when the first post-upgrade piece of mail arrived. :-)