502799 - add new nsISound method for the event sounds

Status: RESOLVED FIXED

(Core :: Widget, enhancement)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch v1.0

I added to some event sounds to nsISound::playSystemSound but the method only has a *string* param.  And they are only defined in C++ (NS_SYSSOUND_*). So, javascript caller cannot use the predefined consts.

I think the param should be an unsigned long. It has some advantages, we can compare quickly, we can use switch-case statement and Javascript callers can use nsISound.*.

I'm going to add some sound events for improving the shell integration. It will increase the string param checking counts. So, it can make damage for the performance of playSystemSound. Therefore, I think I should add a new API now.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 387157 [details] [diff] [review]
Patch v1.0

Thank you, roc.

And I'd like Neil to review the nsPIPromptService related part.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

I wrote a document of nsISound. I'll update it after I land the patch.
https://developer.mozilla.org/en/nsISound

After that, please fix the English text by the native speakers ;-)

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

http://hg.mozilla.org/mozilla-central/rev/91bde0e380a2

Comment 4

[Peter Weilbacher]

(In reply to comment #2)
> After that, please fix the English text by the native speakers ;-)

I'm not a native speaker and the text looks fine, but I wonder if you meant to name EVENT_NEW_MAIL_RECIEVED like this. Not a bit deal, though...

Comment 5

[Martin Schröder [:mschroeder]]

Was the naming of EVENT_AELRT_DIALOG_OPEN on purpose, or is this also a typo?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to comment #4)
> (In reply to comment #2)
> > After that, please fix the English text by the native speakers ;-)
> 
> I'm not a native speaker and the text looks fine, but I wonder if you meant to
> name EVENT_NEW_MAIL_RECIEVED like this. Not a bit deal, though...

(In reply to comment #5)
> Was the naming of EVENT_AELRT_DIALOG_OPEN on purpose, or is this also a typo?

Oh, thank you for pointing out the misspellings in the const names.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Patch for misspelling

probably, this fixes them... (checking on tryserver now.)

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 387775 [details] [diff] [review]
Patch for misspelling

ok, all green on tryserver.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

http://hg.mozilla.org/mozilla-central/rev/dfb9c08bcc9d

landed the new patch.