Closed
Bug 502799
Opened 14 years ago
Closed 14 years ago
add new nsISound method for the event sounds
Categories
(Core :: Widget, enhancement)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: masayuki, Assigned: masayuki)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
25.29 KB,
patch
|
roc
:
review+
neil
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
9.83 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
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)
Attachment #387157 -
Flags: superreview?(roc)
Attachment #387157 -
Flags: superreview+
Attachment #387157 -
Flags: review?(roc)
Attachment #387157 -
Flags: review+
Assignee | ||
Comment 1•14 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•14 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•14 years ago
|
Attachment #387157 -
Flags: review?(neil) → review+
Assignee | ||
Comment 3•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/91bde0e380a2
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Updated•14 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•14 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 4•14 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...
Comment 5•14 years ago
|
||
Was the naming of EVENT_AELRT_DIALOG_OPEN on purpose, or is this also a typo?
Assignee | ||
Comment 6•14 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•14 years ago
|
||
probably, this fixes them... (checking on tryserver now.)
Assignee | ||
Updated•14 years ago
|
Attachment #387775 -
Flags: superreview?(roc)
Attachment #387775 -
Flags: review?(roc)
Assignee | ||
Comment 8•14 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•14 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.
Description
•