Closed Bug 655703 Opened 13 years ago Closed 13 years ago

accessibility.typeaheadfind.enablesound is true by default, this is a PITA of 98.34% of linux users

Categories

(Toolkit :: General, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox5 + fixed
firefox6 - fixed

People

(Reporter: bjacob, Assigned: bjacob)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I had a 23.483432432 dB hearing loss today as I was using earphones, did Ctrl+F in Firefox and triggered the dreaded sound.

No other application that I know has used sounds for that by default since 1974.

Please disable.

Note: mentioning linux because I've only noticed this on linux so far. Might be OS-independent.
To make it more interesting, this is a system beep, so it's not affected by usual mixer levels in ALSA.

hg blame blames revision 1, so the cowards who did this are now hiding behind CVS.
Attached patch disable (obsolete) — Splinter Review
Attachment #531051 - Flags: review?(bolterbugz)
Comment on attachment 531051 [details] [diff] [review]
disable

r=me. Thanks for the laugh.

(I starting looking in cvs blame but had a bag put over my head and was rolled down a hill).
Attachment #531051 - Flags: review?(bolterbugz) → review+
Comment on attachment 531051 [details] [diff] [review]
disable

Firefox 5 users might have sensitive ears, too. If that were confirmed, would we be interested in saving them?
Attachment #531051 - Flags: approval-mozilla-aurora?
http://hg.mozilla.org/mozilla-central/rev/a16d106105ed
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #531051 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Please land for Aurora by Monday May 16 or the approval will potentially be lost.  Please mark as status-firefox5 fixed when you do.
Comment on attachment 531051 [details] [diff] [review]
disable

I would like to argue that this patch does not meet Aurora requirements, since it's not a backout, it doesn't fix a regression on aurora, and it's not a test change.

Re-noming for approval-aurora? requesting for an a-.  I think it should also be backed out from Aurora.
Attachment #531051 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?
JP, see comment 8 please.
For the record, I agree with comment 8. I merely transplanted because of the approval :-)
This was ok'ed in the Aurora triage meeting, I recorded the decision.  As I recall the belief was this was pref only, safe and a platform parity issue.  We did not track this bug, but approved it just as we did for other safe fixes that were not necessarily tracked.  I agree this one was a stretch.  Since approval was granted and the patch landed, I think the + needs to stay.  A separate issue can be raised to release-drivers about the criteria.
Also, rules are here to be violated (French proverb).
(In reply to comment #12)
> Also, rules are here to be violated (French proverb).

Well, not really.

In this particular case, I don't really care if we change the value of the pref in Firefox or not.  There are two reasons why I brought this up:

1. This includes unnecessary churn on Aurora, which may make things like inspecting the changes landed there on the next Aurora merge harder without any good reason.

2. I don't think that we want these types of small fixes to get on Aurora or Beta as a practice.  We should fight our old mental model of "this change is safe I think, so it can land on a branch", because this has been proven wrong many times in practice, and if nothing else, one person filing a bug about this on Beta and nominating the bug for tracking would waste precious drivers' time with something that they shouldn't have had to deal with.

I'm still not convinced why this should stay on Aurora, but I guess it might be too late now.  :(
I don't even hear the beep on Ubuntu 11.04 using Firefox 4.0.1 so I'm not sure how widespread the enabling of the system beep in Linux distros is. Probably less than 98.34%. The beep occurs as can be seen when setting "Enable visual system bell" in the system settings, but no actual sound is produced. The PC speaker is disabled by default in the mainline kernel last time I looked.
Comment on attachment 531051 [details] [diff] [review]
disable

Ok, this bug probably should have received more reflection than it did, but given that we merged to beta this morning I think backing it out will be more of a problem than keeping it.  I'm re-marking as aurora+ since it was approved and it landed.  Food for thought for future triage.
Attachment #531051 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
It seems strange to me that we just overrode a long-standing UI/accessibility decision (see bug 266067 and bug 260679) due to a poor implementation on Linux.  In a new bug report, not addressing the reasons given in the old ones.

Personally, I hate having a not-found sound.

Firefox was one of the first GUI apps to use find-as-you-type, so there wasn't much precedent at the time.  Chrome beeps. IE and Safari don't. Neither do Notepad++ (Windows, Ctrl+Alt+I) or TextWrangler (Mac, Cmd+Opt+F).

Was the sound crucial for accessibility (for screenreader users)?
Chrome beeps, and intends to keep that behavior, having discussed this recently.

I agree with Jesse that making the Linux implementation work better would have been a better route, but I'm clearly biased.
Assignee: nobody → bjacob
Regarding the right behavior for type ahead find, Marco, Trevor, Mike, thoughts? (See Jesse's question on comment 16). We might want a follow up bug for the "right fix".
Wouldn't a better fix here be making the default pref [accessibility.typeaheadfind.soundURL = default] instead of 'beep' or am I misunderstanding things.
A few comments:

The sound doesn't seem particularly loud to me, but then I'm used to my computer making sound, and, also, it is definitely not a system beep on my system (with Firefox 4.0.1).  Changing the volume of "Master" affects its volume, and changing the volume of "Beep" does not.  However, I've got a nonstandard configuration of Pulseaudio that uses the default Alsa device rather than hw.  It wouldn't surprise me if Firefox was trying to play the sound through some mechanism other than Pulse and failing on a typical Linux system, falling back to a system beep.  If this is happening, then it could make the sound either too loud or not play at all (if Beep is muted), so we might need a new bug.

I like having the beep--I otherwise don't have an indication that text wasn't found and I can stop typing.  I'm curious whether it is annoying to a majority of users if it plays at a reasonable volume.  As someone pointed out on bug#266067, someone who is annoyed by the beep might Google for ways to turn it off, but, if it is off by default, then users would have know way of knowing that it is an optional feature that they can turn on.  So this would be an argument for having it on by default.  If most users don't want it even with Linux issues fixed, then maybe that would outweigh the reasons for having it on, but, in that case, it would be nice to have it on by default if accessibility and/or a screen reader is running on the platform.
I would be less opposed to having this sound enabled by default if it weren't a system beep. I realize that some configs either play system beeps as regular sounds or not at all, but here on Debian Squeeze I really got a good old system beep.

Also, if Firefox is to use sounds by default, we should have a single GUI-visible setting to turn them all off at once. Most users will only look at the Preferences dialog, look for the option, and if it isn't there they will just conclude that the option doesn't exist. We're not talking about an advanced-users-only setting here.
(In reply to comment #21)
> Also, if Firefox is to use sounds by default, we should have a single
> GUI-visible setting to turn them all off at once.

Media too?  Could be useful IMO. Limi?
(In reply to comment #22)
> Media too?  Could be useful IMO. Limi?

I wasn't thinking about content here, but why not have a setting for that too. I'd just keep that a separate setting.
(In reply to comment #21)
> I would be less opposed to having this sound enabled by default if it weren't a
> system beep.

That is not the point.  This is a bug in the Linux distro, not Firefox.  If nobody can present a reason why this is something that we want, I think we should backout the patch.  And we should do that pretty quickly.  Otherwise, we'll end up pushing something to 400 million users without being sure that we really want it.

> I realize that some configs either play system beeps as regular
> sounds or not at all, but here on Debian Squeeze I really got a good old system
> beep.

This is a bug that you should file with Debian, no matter what decision we reach here.

> Also, if Firefox is to use sounds by default, we should have a single
> GUI-visible setting to turn them all off at once. Most users will only look at
> the Preferences dialog, look for the option, and if it isn't there they will
> just conclude that the option doesn't exist. We're not talking about an
> advanced-users-only setting here.

This is also beyond the topic.  You should file a new bug for this.

I personally think that until somebody can present a strong reason why we want this, we should back it out.  I just realized that if I were a blind person, this might regress my experience in using the browser, so at least we should be sure to replace the auditory cue with another one.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yes I agree with the comment 24 assessment. I think the right thing to do now is back this out for accessibility. We should turn this into an evangelism bug, and file follow ups for the comment 21 (and 22,23).
(In reply to comment #24)
> (In reply to comment #21)
> > I would be less opposed to having this sound enabled by default if it weren't a
> > system beep.
> 
> That is not the point.  This is a bug in the Linux distro, not Firefox.

I disagree: if Firefox requires a system beep, it is valid for the linux distro to actually produce a system beep. (It is also valid for it not to, of course)

System beeps are a nuisance so no application should use them.

>  If
> nobody can present a reason why this is something that we want, I think we
> should backout the patch.

I think I've presented a reason. On linux distros which actually render system beeps as system beeps, it's a nuisance.

> And we should do that pretty quickly.  Otherwise,
> we'll end up pushing something to 400 million users without being sure that
> we really want it.

Did we really want to produce system beeps in the first place? If yes, are we OK that this is completely undefined behavior, since the linux distro is free to render it as a system beep, as a regular sound, or as no sound at all?

> 
> > I realize that some configs either play system beeps as regular
> > sounds or not at all, but here on Debian Squeeze I really got a good old system
> > beep.
> 
> This is a bug that you should file with Debian, no matter what decision we
> reach here.

Do you really think that if I file a bug saying "debian renders system beeps as system beeps", people will agree that's a bug?

> 
> > Also, if Firefox is to use sounds by default, we should have a single
> > GUI-visible setting to turn them all off at once. Most users will only look at
> > the Preferences dialog, look for the option, and if it isn't there they will
> > just conclude that the option doesn't exist. We're not talking about an
> > advanced-users-only setting here.
> 
> This is also beyond the topic.  You should file a new bug for this.

Let's first see the outcome of the present discussion

> 
> I personally think that until somebody can present a strong reason why we
> want this, we should back it out.  I just realized that if I were a blind
> person, this might regress my experience in using the browser, so at least
> we should be sure to replace the auditory cue with another one.

If we seriously think that these sounds are useful, then why do we use a mechanism (system beeps) that has completely undefined semantics (see above) and often results in no sound at all (see above)?

If we're serious about these sounds, we need to make them real regular sounds, not system beeps that can behave however they want.

I'm losing interest, so decide whatever you want.
Backed out: http://hg.mozilla.org/mozilla-central/rev/dcd56c5311ac
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → WONTFIX
(In reply to comment #26)
> (In reply to comment #24)
> > (In reply to comment #21)
> > > I would be less opposed to having this sound enabled by default if it weren't a
> > > system beep.
> > 
> > That is not the point.  This is a bug in the Linux distro, not Firefox.
> 
> I disagree: if Firefox requires a system beep, it is valid for the linux
> distro to actually produce a system beep. (It is also valid for it not to,
> of course)
> 
> System beeps are a nuisance so no application should use them.

Even if that is the only queue provided by the application?

> >  If
> > nobody can present a reason why this is something that we want, I think we
> > should backout the patch.
> 
> I think I've presented a reason. On linux distros which actually render
> system beeps as system beeps, it's a nuisance.

The question that needs to be answered is: do we want to disable an accessibility feature that we have shipped for many years because of an annoyance in the APIs on a linux distro?  I think the answer is no.

(Please note that I'm personally annoyed by this beep, and in fact I have it disabled in my own profile.  But we're not talking about how we feel about Firefox, we're talking about what's best for our users.)

> > And we should do that pretty quickly.  Otherwise,
> > we'll end up pushing something to 400 million users without being sure that
> > we really want it.
> 
> Did we really want to produce system beeps in the first place? If yes, are
> we OK that this is completely undefined behavior, since the linux distro is
> free to render it as a system beep, as a regular sound, or as no sound at
> all?

You're answering questions with more questions.  :-)

> > 
> > > I realize that some configs either play system beeps as regular
> > > sounds or not at all, but here on Debian Squeeze I really got a good old system
> > > beep.
> > 
> > This is a bug that you should file with Debian, no matter what decision we
> > reach here.
> 
> Do you really think that if I file a bug saying "debian renders system beeps
> as system beeps", people will agree that's a bug?

If you explain why it's a problem to them, why wouldn't them?  I think we all agree that using mainboard beeps in the 21st century is less than perfect.

> > I personally think that until somebody can present a strong reason why we
> > want this, we should back it out.  I just realized that if I were a blind
> > person, this might regress my experience in using the browser, so at least
> > we should be sure to replace the auditory cue with another one.
> 
> If we seriously think that these sounds are useful, then why do we use a
> mechanism (system beeps) that has completely undefined semantics (see above)
> and often results in no sound at all (see above)?

That is a topic for another bug.

> If we're serious about these sounds, we need to make them real regular
> sounds, not system beeps that can behave however they want.

I agree.
Attachment #538978 - Flags: approval-mozilla-beta?
Attachment #538978 - Flags: approval-mozilla-aurora?
Attachment #531051 - Attachment is obsolete: true
I filed bug 663933 for the preferences topic. Benoit can you comment on comment 19? (I think there is still something bug worthy here)
Depends on: 663935
For sure, see the end of comment 28, we want to use regular sounds instead of system beeps for that. I filed bug 663935 about that.
Thanks for filing the followups guys.  :-)
Comment on attachment 538978 [details] [diff] [review]
Backout patch for aurora and beta

Discussed in beta triage - please land this today
Attachment #538978 - Flags: approval-mozilla-beta?
Attachment #538978 - Flags: approval-mozilla-beta+
Attachment #538978 - Flags: approval-mozilla-aurora?
Attachment #538978 - Flags: approval-mozilla-aurora+
Don't need tracking6 here, but thanks for the backout!
So is this in Firefox 5 or not? The relnotes link here.
Not.  I will see about getting the relnotes fixed.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: