chat sound not playing if mail notification sound disabled

RESOLVED FIXED in Thunderbird 17.0


7 years ago
6 years ago


(Reporter: realRaven, Assigned: realRaven)


15 Branch
Thunderbird 17.0

Firefox Tracking Flags

(Not tracked)



(1 attachment, 4 obsolete attachments)



7 years ago
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

enabled while having mail.biff.play_sound disabled

Actual results:

no notification sounds played back when my name was mentioned in IRC

Expected results:

chat notification sounds should play even if mail notification sounds are disabled


7 years ago
Severity: normal → trivial


7 years ago
Component: General → Instant Messaging

Comment 1

7 years ago
Tech Notes for patching:

Pref check within PlayBiffSound() needs to move outside and be checked before calling PlayBiffSound() instead; code:

I will see if I can get around to writing that one soon...

Comment 2

7 years ago
Created attachment 646910 [details] [diff] [review]
moved check out of PlayBiffSound into folder listener
Assignee: nobody → axelg
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All

Comment 3

7 years ago
Created attachment 646913 [details] [diff] [review]
Same patch as before w/o whitespace mods
Attachment #646910 - Attachment is obsolete: true
Attachment #646913 - Flags: review?

Comment 4

7 years ago
Created attachment 646915 [details] [diff] [review]
Samne patch without duplicate comment

removed duplicate comment
Attachment #646913 - Attachment is obsolete: true
Attachment #646913 - Flags: review?
Attachment #646915 - Flags: review?(irving)

Comment 5

7 years ago
Created attachment 646921 [details] [diff] [review]
This one even compiles :P

Ok, it was pointed out that previous code was incomplete, and how to compile a single file. This one compiles and should also work as designed. I have yet to make a full build for testing.
Attachment #646915 - Attachment is obsolete: true
Attachment #646915 - Flags: review?(irving)

Comment 6

7 years ago
Created attachment 646925 [details] [diff] [review]
moved a comment

moved a comment to a more appropriate place
Attachment #646921 - Attachment is obsolete: true
Axel, is it intentional that there's no review request on the patch attached in comment 6?

Comment 8

7 years ago
probably not, who should I add for this one?
Attachment #646925 - Flags: review?(irving)
Comment on attachment 646925 [details] [diff] [review]
moved a comment

Review of attachment 646925 [details] [diff] [review]:

Looks fine, works in my quick test.

::: mailnews/base/src/nsStatusBarBiffManager.cpp
@@ +170,5 @@
>      // if we fail along the way, don't return.
>      // we still need to update the UI.    
>      if (newValue == nsIMsgFolder::nsMsgBiffState_NewMail) {
> +      nsresult rv;
> +      nsCOMPtr<nsIPrefBranch> pref(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));

This retrieves the pref service twice per notification (once here, and once inside the call to PlayBiffSound()) but I can't see a simple way to only get it once, and it's not a huge performance hit anyway.
Attachment #646925 - Flags: review?(irving) → review+
Pushed to comm-central trunk:
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 11

7 years ago
Thanks Florian for taking this over. My build system defunct for the last fortnight :(
Target Milestone: --- → Thunderbird 17.0
You need to log in before you can comment on or make changes to this bug.