Last Comment Bug 655703 - accessibility.typeaheadfind.enablesound is true by default, this is a PITA of 98.34% of linux users
: accessibility.typeaheadfind.enablesound is true by default, this is a PITA of...
Status: RESOLVED WONTFIX
:
Product: Toolkit
Classification: Components
Component: General (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: ---
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
Mentors:
Depends on: 663935
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-09 06:52 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2011-06-24 10:18 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
-
fixed


Attachments
disable (1.14 KB, patch)
2011-05-09 08:25 PDT, Benoit Jacob [:bjacob] (mostly away)
dbolter: review+
jpr: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Backout patch for aurora and beta (1.34 KB, patch)
2011-06-13 12:33 PDT, :Ehsan Akhgari
bugzilla: approval‑mozilla‑aurora+
bugzilla: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Benoit Jacob [:bjacob] (mostly away) 2011-05-09 06:52:04 PDT
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.
Comment 1 Benoit Jacob [:bjacob] (mostly away) 2011-05-09 07:19:58 PDT
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.
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2011-05-09 08:25:02 PDT
Created attachment 531051 [details] [diff] [review]
disable
Comment 3 David Bolter [:davidb] 2011-05-09 17:20:48 PDT
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).
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2011-05-10 06:13:29 PDT
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?
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2011-05-10 06:21:16 PDT
http://hg.mozilla.org/mozilla-central/rev/a16d106105ed
Comment 6 JP Rosevear [:jpr] 2011-05-12 14:45:17 PDT
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 7 christian 2011-05-16 12:21:11 PDT
Transplanted to mozilla-aurora:
    http://hg.mozilla.org/releases/mozilla-aurora/rev/4ab472de1c86
Comment 8 :Ehsan Akhgari 2011-05-16 13:16:20 PDT
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.
Comment 9 :Ehsan Akhgari 2011-05-16 13:16:57 PDT
JP, see comment 8 please.
Comment 10 christian 2011-05-16 13:25:01 PDT
For the record, I agree with comment 8. I merely transplanted because of the approval :-)
Comment 11 JP Rosevear [:jpr] 2011-05-17 06:18:13 PDT
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.
Comment 12 Benoit Jacob [:bjacob] (mostly away) 2011-05-17 06:35:09 PDT
Also, rules are here to be violated (French proverb).
Comment 13 :Ehsan Akhgari 2011-05-17 08:35:30 PDT
(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.  :(
Comment 14 Daniel Cater 2011-05-17 08:51:42 PDT
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 15 JP Rosevear [:jpr] 2011-05-17 11:52:27 PDT
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.
Comment 16 Jesse Ruderman 2011-05-18 02:27:19 PDT
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)?
Comment 17 Peter Kasting 2011-05-19 17:22:22 PDT
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.
Comment 18 David Bolter [:davidb] 2011-06-11 07:12:16 PDT
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".
Comment 19 David Bolter [:davidb] 2011-06-11 07:52:29 PDT
Wouldn't a better fix here be making the default pref [accessibility.typeaheadfind.soundURL = default] instead of 'beep' or am I misunderstanding things.
Comment 20 Mike Gorse 2011-06-12 21:32:58 PDT
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.
Comment 21 Benoit Jacob [:bjacob] (mostly away) 2011-06-13 04:26:23 PDT
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.
Comment 22 David Bolter [:davidb] 2011-06-13 05:28:37 PDT
(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?
Comment 23 Benoit Jacob [:bjacob] (mostly away) 2011-06-13 06:36:30 PDT
(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.
Comment 24 :Ehsan Akhgari 2011-06-13 11:55:25 PDT
(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.
Comment 25 David Bolter [:davidb] 2011-06-13 12:07:08 PDT
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).
Comment 26 Benoit Jacob [:bjacob] (mostly away) 2011-06-13 12:18:09 PDT
(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.
Comment 27 :Ehsan Akhgari 2011-06-13 12:26:16 PDT
Backed out: http://hg.mozilla.org/mozilla-central/rev/dcd56c5311ac
Comment 28 :Ehsan Akhgari 2011-06-13 12:31:59 PDT
(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.
Comment 29 :Ehsan Akhgari 2011-06-13 12:33:48 PDT
Created attachment 538978 [details] [diff] [review]
Backout patch for aurora and beta
Comment 30 David Bolter [:davidb] 2011-06-13 12:53:41 PDT
I filed bug 663933 for the preferences topic. Benoit can you comment on comment 19? (I think there is still something bug worthy here)
Comment 31 Benoit Jacob [:bjacob] (mostly away) 2011-06-13 13:04:48 PDT
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.
Comment 32 :Ehsan Akhgari 2011-06-13 13:27:20 PDT
Thanks for filing the followups guys.  :-)
Comment 33 Johnathan Nightingale [:johnath] 2011-06-13 14:24:51 PDT
Comment on attachment 538978 [details] [diff] [review]
Backout patch for aurora and beta

Discussed in beta triage - please land this today
Comment 35 Johnathan Nightingale [:johnath] 2011-06-15 11:40:17 PDT
Don't need tracking6 here, but thanks for the backout!
Comment 36 James Andrewartha 2011-06-23 23:55:57 PDT
So is this in Firefox 5 or not? The relnotes link here.
Comment 37 JP Rosevear [:jpr] 2011-06-24 10:18:00 PDT
Not.  I will see about getting the relnotes fixed.

Note You need to log in before you can comment on or make changes to this bug.