Closed Bug 176602 Opened 22 years ago Closed 22 years ago

Make typeaheadfind beep less annoying

Categories

(SeaMonkey :: Find In Page, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.3alpha

People

(Reporter: aaronlev, Assigned: aaronlev)

References

Details

(Keywords: polish, Whiteboard: works in windows, needs testing/help in Linux and Mac)

Attachments

(2 files, 14 obsolete files)

(deleted), audio/wav
Details
(deleted), patch
aaronlev
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
This is separate from the bug to add a pref for the beep -- we'll do that as well. What I want to do here is make the beep only occur if at least 1 character has been successfully found, or if / or ' has been used to start the find. That way it won't happen when you just accidentally type a stray character in the browser. I also want to use a nicer sound for the beep. I found some public domain sounds using google/lycos media search.
Attached audio beep_drop_of_water.au (obsolete) (deleted) —
Attached audio beep_pure_saw.au (obsolete) (deleted) —
Attached audio idg_blip-intermed-2895.wav (obsolete) (deleted) —
Attached audio inflecto-Intermed-613.wav (obsolete) (deleted) —
Attached audio water.wav (obsolete) (deleted) —
Attached audio woob-Public_D-151.wav (obsolete) (deleted) —
Well, that's a start -- any of those okay? There are more freeware sounds here: http://www.flashkit.com/soundfx/Interfaces/
Blocks: isearch
Status: NEW → ASSIGNED
Keywords: polish, ui
Priority: -- → P2
Target Milestone: --- → mozilla1.3alpha
Be aware that mozilla's sound doesn't necessarily work on all platforms, or has problems which mean that it should not be used by default. For instance, on unix, due to bug 110385, nsSound blocks until the sound is finished playing, which can cause very annoying delays (especially if you're trying to type).
Let's choose a short sound then? A little boop? I think, once a bad key is pressed it's not terrible if there's a pause, because the rest won't be found anyway. We can make it work.
Attached patch A pleasant sound, I think this one will suffice (obsolete) (deleted) — Splinter Review
Attachment #104054 - Attachment is obsolete: true
Attachment #104055 - Attachment is obsolete: true
Attachment #104056 - Attachment is obsolete: true
Attachment #104057 - Attachment is obsolete: true
Attachment #104058 - Attachment is obsolete: true
Attachment #104060 - Attachment is obsolete: true
I'd prefer something about a quarter of that duration. The pitch is good.
Does anyone have a sound editing tool that can speed it up without changing the pitch?
Attached audio Half as long (any faster and it sounds real bad) (obsolete) (deleted) —
I found a pretty good Windows sound editor called Acoustica here: http://download.com.com/3001-2170-9055050.html
cool. :)
Sounds good =)
On Windows, this is only working when I use the debugger and trace into and step over this code: BOOL PlaySound(const char *aSoundFile,HMODULE aModule,DWORD aOptions) { return (mPlay) ? mPlay(aSoundFile, aModule, aOptions) : FALSE; } If I just let it run it normally doesn't work. Would someone be willing to test this on another platform? After applying this patch, you have to copy the sound file into mozilla/extensions/typeaheadfind/resources/content/notfound.wav
Attachment #104085 - Attachment is obsolete: true
Depends on: 176747
The current patch doesn't work in linux either (haven't tried stepping in the debugger). I'm happy to test future patches on linux. I like the choice of sound. If we do this, I want to argue strongly for exposing the pref. Based on my experience with chatzilla, the problem isn't the length of the sound; the problem is the time nsSound takes to initiate and finish the sound operation (moz was locked up for much longer than the length of the sound itself, often several seconds -- it was very noticable and frustrating). Also, it's very common for sound not to work -- especially on linux and other Unix machines, but I know a lot of Windows users who don't have working sound either. Also, what happens if I do typeahead find while some other app has the sound system locked? When I first downloaded the sound sample and tried to play it, play hung and I had to kill it, because realplayer is currently streaming a radio program, and apparently realplayer doesn't play nice about sharing the sound device. I've seen bugs that say the flash plugin does the same thing. I'd hate to see my whole browser hang for that reason. This sound behavior must be optional, unless we fix nsSound to be asynchronous.
I agree, it should be a pref -- we have a bug for typeahead prefs open somewhere and I plan to implement it there.
All sounds should be optional, really. There are some circumstances where beeping just isn't wanted.
> it's not terrible if there's a pause Um... yes it is. The basic issue is that the sound playing is synchronous. Happening on the UI thread. while that sound is playing there is no painting, no reflow, nothing. That may not be so bad if the sound actually plays. But from a brief look at the gtk impl, it just punts the playing over to esd. So either you have esd running and then all is happy, or you don't have esd running and we don't start it (then there is just no sound), or we try to start it and then we get contention for the audio device if something else is using it (read: same as the "Flash hangs mozilla" bug). Please figure out which of the two possibilities actually takes place when esd is not running...
Well it looks like there is no traction on bug 110385. I'm tempted to make the typeaheadfind bloop sound off by default on Unix/Linux, at least until that's fixed.
Depends on: 110385
I can't get sound to play off a chrome URL, unless I change nsSound to play sounds from memory synchronously :P
Well someone else try my patch in Windows to see if I'm crazy? It only works if I change this line in widget/src/windows/nsSound.cpp - theMM.PlaySound(mLastSound, nsnull, SND_MEMORY | SND_NODEFAULT | SND_SYNC); + theMM.PlaySound(mLastSound, nsnull, SND_MEMORY | SND_NODEFAULT | SND_ASYNC); Is that true for anyone else?
Attachment #104160 - Attachment is obsolete: true
Attached patch Even shorter sound (obsolete) (deleted) — Splinter Review
Attachment #104093 - Attachment is obsolete: true
Need testing on other platforms. Can someone help me determine what happens on Linux when esd is not running? Seeking r=, sr=
Attachment #104312 - Attachment is obsolete: true
Need testing on other platforms. Can someone help me determine what happens on Linux when esd is not running? Seeking r=, sr=
Attachment #104333 - Attachment is obsolete: true
Attachment #104309 - Attachment is obsolete: true
Comment on attachment 104334 [details] [diff] [review] Impl our own sound for typeaheadfind beep, add pref for beep, #ifdef DEBUG some useless code in widget/src/windows/nsSound.cpp, and decrease typeaheadfind timeout a little >+ // After each keystroke, destrouy sound object to free up memory ^^^^^^^^ You've got a typo there.
Tested on Redhat 8.0, there is no sound. I traced into the nsSound.cpp under /widget/src/gtk, and am sure that the following code had been excuted: nsSound::OnStreamComplete() { ... fd = (*EsdPlayStreamFallback)(mask, rate, NULL, "mozillaSound"); ... write(fd, string, stringLen); close(fd); ... } but still can't hear anything. Is there any other code used nsISound::Play method? So that I can determine this is a nsSound bug or TypeAheadFind bug.
Mailnews uses nsISound::PlaySystemSound(), but that's the only other thing I can find that uses nsISound to play a custom wav file when new mail arrives. You can set that up in mailnews prefs. The code to play the wav file in mailnews is in base/prefs/resources/content/pref-notifications.js.
aaron, out of curiosity, are you able to test this on a mac? mail notification sounds, iirc, were/are an issue on mac os's as well. or, are you planning for this to be a win32/unix-only fix?
yes, there is sound when new mail arrives, but it's from my build-in speaker, not the sound card. I think it would be from nsISound::Beep(). the pref-notifications.js seems like some pref setting & sound preview stuff.
Kyle, there's a setting in Windows that allows you to specify your own wav sound file for the new message sound. Do you have that in Linux?
> Do you have that in Linux? Yes.
Whiteboard: works in windows, needs testing/help in Linux
Now it works on my RedHat 8.0 after installed a patch (arts-2.2.2-1.i386.rpm), even if there is no esd.
It actually works? That's great! Maybe someone can test it on a mac now.
Kyle, please perform the following test: 1) Have no ESD running 2) Have something using /dev/dsp (an mp3 player should do fine) 3) Make sure to use a soundcard without hardware mixing support 4) Try to use nsISound What happens? Is there just no sound? Or does Mozilla hang?
Attachment #104335 - Attachment is patch: false
Attachment #104335 - Attachment mime type: text/plain → audio/wav
I think we should keep sound pref'd off on Unix/Linux until we get bug 110385 (asynchronous sound) fixed for that platform, and until we make sure there are no problems with system hangs with sound in Linux. Seeking r=/sr=
Attachment #104334 - Attachment is obsolete: true
It doesn't make any sound on Redhat 7.3. nsSound::OnStreamComplete does get called, but apparently the code after that doesn't work (I haven't had a chance to chase it any farther than that). Even aside from the asynchrony issue, nsSound apparently just plain doesn't work on most linux systems, even those where sound is configured.
Filed bug 179138 with more details on the problems with nsSound::Play on linux. I'm fine with the idea of checking this in preffed off for linux until we get the linux sound problems straightened out, if it works on other platforms.
Whiteboard: works in windows, needs testing/help in Linux → works in windows, needs testing/help in Linux and Mac
sorry, I was using a gtk2 build. the gtk build still doesn't work (no sound). So the clue is what's the different between widget\src\gtk\nsSound.cpp and widget\src\gtk2\nsSound.cpp.
Thanks Kyle. I still think this is worthy of r= now, and we can work out gtk nsSound problems later (bug 179138). Seeking r=kyle.
Comment on attachment 105610 [details] [diff] [review] Same patch as before, but prefs off sound on Unix r=kyle with one doubt: does wav file work on all platforms? Seems there is no other wav files in mozilla except the calendar module. bz, I'll do your test on Monday.
Attachment #105610 - Flags: review+
Comment on attachment 105610 [details] [diff] [review] Same patch as before, but prefs off sound on Unix + mSoundInterface = do_CreateInstance("@mozilla.org/sound;1"); + mSoundInterface->PlaySystemSound(""); How about a null-check? + PRPackedBool mIsSoundInitialized; What's the point of making this packed?
Attachment #105610 - Flags: needs-work+
Attached patch Same thing, with Boris' tweaks (deleted) — Splinter Review
Seekin sr=bz
Attachment #105610 - Attachment is obsolete: true
Comment on attachment 105696 [details] [diff] [review] Same thing, with Boris' tweaks Carrying forth Kyle's r= (How does one do that in the new Bugzilla, without making it look like you've r='d your own bug?)
Attachment #105696 - Flags: review+
Attachment #105696 - Flags: superreview+
Fixed -- however the sound is off by default in Linux. You have to put this in your .js file to get sound in Linux: user_pref("accessibility.typeaheadfind.enablesound", true);
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
bz, here is my test result on RH8 (I used a gtk2 build, because the gtk build can't make any sound) > 1) Have no ESD running can make sound. if there is no esd process, mozilla will start it. > 2) Have something using /dev/dsp (an mp3 player should do fine) no sound if I'm using xmms to play a mp3 file. when I closed xmms, sound is back. > 3) Make sure to use a soundcard without hardware mixing support I don't know how to make sure that. > 4) Try to use nsISound I applied this patch, turned on the pref in unix.js and typed some wrong char to trigger nsISound. In any cases, mozilla didn't hang/crash.
I forgot to ask (or check): with sound disabled, as in linux, does that mean it will beep as before, or will it just be silent? Is there a way to select between those two behaviors?
With sound disabled, it won't beep. There's no way to select the old beep behavior.
is the sound also disabled by default on Mac? when i used today's (2002.11.12.07) mach-o build (on OS 10.2.2), no sounds are played when type ahead find doesn't find a match.
Filed RFE bug 179784 requesting that the pref point to a sound url (so it could be changed), or "beep" to use the system beep, or null to disable sound. Sorry, I should have thought of suggesting that earlier since it would have been a small change to what Aaron already had here. It's no big deal, though, a little project for a rainy afternoon when one of us is bored. :-)
No, it means nsSound::Play() isn't working on Mac either.
nsSound::Play works just fine on Mac. For me, I get beeps the first few times TAF fails, but it stops beeping aftera a while.
hrm, how odd --i never got any sound at all with TAF on mach-o. do i need to blast the volume or something? ;)
oh, it's a mach-o issue --i've spun off bug 179824 for that. with a cfm build (today's commercial on 10.2.2), sound does work, albeit with a rather grundgey sound. ;) also vrfy'ing as fixed on win2k.
Status: RESOLVED → VERIFIED
oh, and filed bug 179817 to fix sounds for type ahead find on linux.
looks like bug 179824 on mach-o is a regression caused by the fix to this bug.
Are people happy with the new typeaheadfind sound? We're trying to minimize the number of prefs exposed, because there are too many already. I haven't heard any complaints in a while, so I'm thinking we might be okay with the current behavior.
>Are people happy with the new typeaheadfind sound? Well now that you ask it, not really, it just sounds strange. I want the beep back.
I thought something was broken when this noise first showed up :) I turn it off completely - with every new build!
If you look at bug 168574, you will see that gtk-1.x has a bug which means you can't switch off the standard beep using the xset command on unix (or the gnome and kde user interfaces for it). This may be an issue...
Just to drop in my $0.02 -- everything works fine on WinME, and I think the choice of sound is good. It isn't incredibly loud, and it does the job. Rather than the windows "ding", or the system beep (worse), the current bloop provides the needed feedback without too much of a distraction.
Component: Keyboard: Navigation → Keyboard: Find as you Type
*** Bug 168574 has been marked as a duplicate of this bug. ***
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: