add new nsISound method for the event sounds

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Widget
--
enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla1.9.2a1
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
Created attachment 387157 [details] [diff] [review]
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.
Attachment #387157 - Flags: superreview?(roc)
Attachment #387157 - Flags: review?(roc)
(Assignee)

Updated

8 years ago
Blocks: 248380
Attachment #387157 - Flags: superreview?(roc)
Attachment #387157 - Flags: superreview+
Attachment #387157 - Flags: review?(roc)
Attachment #387157 - Flags: review+
(Assignee)

Comment 1

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

Thank you, roc.

And I'd like Neil to review the nsPIPromptService related part.
Attachment #387157 - Flags: review?(neil)
(Assignee)

Comment 2

8 years ago
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 ;-)

Updated

8 years ago
Attachment #387157 - Flags: review?(neil) → review+
(Assignee)

Comment 3

8 years ago
http://hg.mozilla.org/mozilla-central/rev/91bde0e380a2
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Updated

8 years ago
Keywords: dev-doc-needed
(Assignee)

Updated

8 years ago
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Updated

8 years ago
Depends on: 503246
(Assignee)

Updated

8 years ago
Depends on: 503248

Comment 4

8 years ago
(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...
Was the naming of EVENT_AELRT_DIALOG_OPEN on purpose, or is this also a typo?
(Assignee)

Comment 6

8 years ago
(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.
(Assignee)

Comment 7

8 years ago
Created attachment 387775 [details] [diff] [review]
Patch for misspelling

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

Updated

8 years ago
Attachment #387775 - Flags: superreview?(roc)
Attachment #387775 - Flags: review?(roc)
(Assignee)

Comment 8

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

ok, all green on tryserver.
Attachment #387775 - Flags: superreview?(roc)
Attachment #387775 - Flags: superreview+
Attachment #387775 - Flags: review?(roc)
Attachment #387775 - Flags: review+
(Assignee)

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/dfb9c08bcc9d

landed the new patch.
You need to log in before you can comment on or make changes to this bug.