Closed Bug 604451 Opened 11 years ago Closed 10 years ago
Provide a way to play the "ding" system sound used to indicate typing too many characters in a text field on Windows
In order to fix bug 603190, we'll need a new value for nsISound::PlayEventSound for the "ding" sound played when someone types more characters than the maximum allowed value inside a text field.
nsISound::beep() seems to play that sound. Tested by setting the specific sound ("Standardton Warnsignal" in my german Windows) to a distinct sound, then evaluating Components.classes["@mozilla.org/sound;1"].createInstance(Components.interfaces.nsISound).beep() in the error console and with a mspaint dialog box that has a length-limited input field. Entering too much into the mspaint dialog created the distinct sound and beep() did too, so beep() should make the correct sound for typing too many characters even if a user has custom sound settings. Additionally, it may already be cross-plattform, so using it may be the best solution.
No, we should use nsISound::PlayEventSound. And nsSound for Windows should call beep() if it's correct behavior. It seems that Windows plays the sound for each discarded character. I mean, when IME commits a couple of characters, Windows plays the sound a couple of times.
OS: Mac OS X → Windows 7
The default beep is the one that is used when max length is reached in an edit box in windows. I verified by changing it to a custom sound and reaching the max length.
Comment on attachment 558374 [details] [diff] [review] Patch for new constant in nsISound and Win32 nsSound for max length reached > +#define NS_SYSSOUND_EDITOR_MAX_LEN NS_LITERAL_STRING("_moz_editormaxlen") > + else if (aSoundAlias.Equals(NS_SYSSOUND_EDITOR_MAX_LEN)) > + eventId = EVENT_EDITOR_MAX_LEN; You don't need to support the new event sound by PlaySystemSound(). All callers should call PlayEventSound().
Removed PlaySystemSound support for the new event as per Masayuki Nakano's suggestion.
Attachment #558458 - Flags: review?(tellrob) → review+
Bug 603190 for which this bug was originally created is going to go with a visual cue instead when max length is reached. I will still push this patch though for other XUL apps such as Thunderbird that may want to support it to get platform default behaviour.
(In reply to Brian R. Bondy [:bbondy] from comment #6) > Bug 603190 for which this bug was originally created is going to go with a > visual cue instead when max length is reached. I will still push this patch > though for other XUL apps such as Thunderbird that may want to support it to > get platform default behaviour. They're not going to get that behavior by merely having this patch in, since the editor code won't be using it... :/
I know they wouldn't without specifically implementing it themselves :) Would it be better for me to mark this task as RESOLVED | INVALID ?
No, I think you should definitely get this landed. I just wanted to mention that it won't automatically give those applications what they want...
Pushed to try: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=4a73b5bc2681 I selected all platforms instead of only just Windows because there is an interface which gets compiled on all platforms.
Comment on attachment 558458 [details] [diff] [review] Patch for new constant in nsISound and Win32 nsSound for max length reached. v2 Super review requested for the minor interface change.
Attachment #558458 - Flags: superreview?(roc)
Attachment #558458 - Flags: superreview?(roc) → superreview+
Why does this affect add-on compatibility?
I'm not sure which action items come from marking that keyword, but I was told on IRC to put it and request super review for any and all interface changes. Pls advise if the keyword is only for modifications and removals.
The addon-compat keyword is used for changes which can potentially break existing add-ons. On a closer look, this patch changes the IID of nsISound, so it's possible for it to break binary add-ons which use that interface. Therefore, you can ignore my comment, I was wrong! :-)
That brings me to another question... I was told to always update the IID of every interface I change. Is that right? If so that implies addon-compat for all interface changes :)
(In reply to Brian R. Bondy [:bbondy] from comment #15) > That brings me to another question... I was told to always update the IID of > every interface I change. Is that right? If so that implies addon-compat for > all interface changes :) Yes. However, adding a constant doesn't change the vtable layout, so maybe we shouldn't change the IID in this case? Benjamin would know best.
Pushed to mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/4a5eb5183ed9
It is not necessary to rev the IID for just a new constant, but I don't think it would hurt anyone.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.