Last Comment Bug 466276 - .caf files grayed out (not selectable) in Preferences > General > Use the following sound file > Browse...
: .caf files grayed out (not selectable) in Preferences > General > Use the fol...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- normal (vote)
: Thunderbird 18.0
Assigned To: Javi Rueda
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-22 01:44 PST by tom
Modified: 2012-09-13 14:07 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
A fix (1.10 KB, patch)
2009-06-11 11:47 PDT, Ludovic Hirlimann [:Usul]
no flags Details | Diff | Splinter Review
patch (1.18 KB, patch)
2012-08-27 12:22 PDT, Javi Rueda
bwinton: review+
Details | Diff | Splinter Review
patch 2 (2.66 KB, patch)
2012-09-04 02:35 PDT, Javi Rueda
bwinton: review+
bwinton: ui‑review+
Details | Diff | Splinter Review
patch 2 fixed (string) (2.66 KB, patch)
2012-09-13 09:00 PDT, Javi Rueda
leofigueres: review+
leofigueres: ui‑review+
Details | Diff | Splinter Review

Description tom 2008-11-22 01:44:15 PST
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20081006 Shredder/3.0a3

The problem in Thunderbird 2.0.0.18 of Apple .caf sound files (as new mail notifications) not playing has been fixed in Shredder 3.0a. However, .caf files can't be selected in the Browse... dialog box.

However, in Thunderbird 2.0.0.18, the files are selectable but don't play.

I know they play because a .caf file was already selected in my Thunderbird 2.0.0.18 profile, which Shredder imported correctly into the preferences. I just cannot change the sound selected in Shredder 3.0a.

Reproducible: Always

Steps to Reproduce:
1. Start Shredder.
2. Go to Preferences....
3. Under When new messages arrive: check Play a sound.
4. Select Use the following sound file.
5. Click Browse....
6. Find a .caf file. For example:
Macintosh HD:Library:Audio:Apple Loops:Apple:iLife Sound Effects:Ambience:Big Waterfall.caf.
7. All .caf files are grayed out and not clickable. 
Actual Results:  
.caf sound files cannot be selected.

Expected Results:  
Files should be enabled and selectable.

A workaround is to open the profile in an older version of Thunderbird, change the sound file to play, and then reopen it in Shredder. However, this would be inconvenient for end users.
Comment 1 hansen 2009-02-26 15:01:02 PST
I don't have Mac, but it doesn't seem to have been fixed.

I can only find *.wav in the file-picker:
http://mxr.mozilla.org/comm-central/source/mail/components/preferences/general.js#147

By the way, beta2 has just been released.
Comment 2 Ludovic Hirlimann [:Usul] 2009-06-11 11:44:22 PDT
Confirming with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090610 Shredder/3.0b3pre
Comment 3 Ludovic Hirlimann [:Usul] 2009-06-11 11:47:52 PDT
Created attachment 382783 [details] [diff] [review]
A fix

A possible fix - ain't sure it's the right approach - Mark thoughts ?
Comment 4 Mark Banner (:standard8) 2009-06-12 04:28:13 PDT
I think the approach is fine, but we want to make it mac-only. Could use ifdefs, but we may have a better way lying around.
Comment 5 Ludovic Hirlimann [:Usul] 2009-06-12 05:19:27 PDT
(In reply to comment #4)
> I think the approach is fine, but we want to make it mac-only. Could use
> ifdefs, but we may have a better way lying around.

Would removing the filter be better ?
Comment 6 hansen 2009-06-12 06:23:39 PDT
How about something like this:
fp.appendFilter("Audio Files", "*.wav; *.caf");
Where "Audio Files" should be l10n, but I don't really understand the code.
Comment 7 Mark Banner (:standard8) 2009-06-22 02:21:36 PDT
Comment on attachment 382783 [details] [diff] [review]
A fix

iirc we discussed this over irc and said that doing a mac specific version of the filter would be better.
Comment 8 Javi Rueda 2012-08-27 12:22:13 PDT
Created attachment 655700 [details] [diff] [review]
patch

Adding filters only on the MacOS platform.
Comment 9 Blake Winton (:bwinton) (:☕️) 2012-08-27 13:21:39 PDT
Comment on attachment 655700 [details] [diff] [review]
patch

This isn't the greatest UI (I would prefer a "Sound Files" which listed all of the types ;), but it'll do.

r=me.

Thanks,
Blake.
Comment 10 Ian Neal 2012-08-27 13:49:22 PDT
Comment on attachment 655700 [details] [diff] [review]
patch

># HG changeset patch
># Parent 29fbdc5bfaadbbbfb34540e12fe466d41adeee53
># User Javi Rueda <leofigueres@yahoo.com>
>Bug 466276 - .caf files grayed out (not selectable) in Preferences > General > Use the following sound file > Browse...
>
>diff --git a/mail/components/preferences/general.js b/mail/components/preferences/general.js
>--- a/mail/components/preferences/general.js
>+++ b/mail/components/preferences/general.js
>@@ -112,16 +112,17 @@ var gGeneralPane = {
>     // XXX todo, persist the last sound directory and pass it in
>     fp.init(window, document.getElementById("bundlePreferences").getString("soundFilePickerTitle"), nsIFilePicker.modeOpen);
>     fp.appendFilter("*.wav", "*.wav");
>     
>     // On Mac, allow AIFF files too
>     if (Application.platformIsMac) {
>       fp.appendFilter("*.aif", "*.aif");
>       fp.appendFilter("*.aiff", "*.aiff");
>+      fp.appendFilter("*.caf", ".caf");
Shouldn't the 2nd argument be "*.caf"?
Comment 11 Javi Rueda 2012-08-27 14:04:45 PDT
(In reply to Ian Neal from comment #10)
> Comment on attachment 655700 [details] [diff] [review]
> patch
> 
> ># HG changeset patch
> ># Parent 29fbdc5bfaadbbbfb34540e12fe466d41adeee53
> ># User Javi Rueda <leofigueres@yahoo.com>
> >Bug 466276 - .caf files grayed out (not selectable) in Preferences > General > Use the following sound file > Browse...
> >
> >diff --git a/mail/components/preferences/general.js b/mail/components/preferences/general.js
> >--- a/mail/components/preferences/general.js
> >+++ b/mail/components/preferences/general.js
> >@@ -112,16 +112,17 @@ var gGeneralPane = {
> >     // XXX todo, persist the last sound directory and pass it in
> >     fp.init(window, document.getElementById("bundlePreferences").getString("soundFilePickerTitle"), nsIFilePicker.modeOpen);
> >     fp.appendFilter("*.wav", "*.wav");
> >     
> >     // On Mac, allow AIFF files too
> >     if (Application.platformIsMac) {
> >       fp.appendFilter("*.aif", "*.aif");
> >       fp.appendFilter("*.aiff", "*.aiff");
> >+      fp.appendFilter("*.caf", ".caf");
> Shouldn't the 2nd argument be "*.caf"?

Oops. Yes, it is true. I am going to fix it in a while. Sorry for the mistake.

I am not a Macintosh user so I was unable to test it :-(
Comment 12 Javi Rueda 2012-08-27 14:38:38 PDT
I would like to work on the improvement from Blake in comment 9. Patch will not be ready for the next merge, anyway.
Comment 13 Javi Rueda 2012-09-04 02:35:12 PDT
Created attachment 658031 [details] [diff] [review]
patch 2

All files extensions have been grouped so they could be shown as "Sound files".

The string "Sound files" has been created. This will need localization on the other supported languages.

As line length was longer than 80 chararacters, code directly affected was refactorised using variable bundlePrefs and soundFilesText.

Note that this patch was tested on a Windows system, not on a MacOS X. I am not able to cross-compile it, either.
Comment 14 Blake Winton (:bwinton) (:☕️) 2012-09-13 06:59:15 PDT
Comment on attachment 658031 [details] [diff] [review]
patch 2

>+++ b/mail/components/preferences/general.js
>@@ -106,23 +106,24 @@ var gGeneralPane = {
>-    // On Mac, allow AIFF files too
>-    if (Application.platformIsMac) {
>-      fp.appendFilter("*.aif", "*.aif");
>-      fp.appendFilter("*.aiff", "*.aiff");
>-    }
>+    // On Mac, allow AIFF and CAF files too
>+    var bundlePrefs = document.getElementById("bundlePreferences");
>+    var soundFilesText = bundlePrefs.getString("soundFilesDescription");
>+    if (Application.platformIsMac)
>+      fp.appendFilter(soundFilesText, "*.wav; *.aif; *.aiff; *.caf");
>+    else
>+      fp.appendFilter(soundFilesText, "*.wav");

I like it!  I especially like how, on Mac, since there's only one filter now, we don't get the drop-down allowing us to choose between different (unuseful) file types.  :)

>+++ b/mail/locales/en-US/chrome/messenger/preferences/preferences.properties
>@@ -54,16 +54,17 @@ typeDetailsWithTypeOrExt=(%1$S)
>+soundFilesDescription=Sound files

This should be "Sound Files", as per the other filters I've seen.  (The "Save As…" Formats, for example.)

r=me, and ui-r=me with those fixed!

Thanks,
Blake.
Comment 15 Javi Rueda 2012-09-13 09:00:07 PDT
Created attachment 660860 [details] [diff] [review]
patch 2 fixed (string)

Amended string for sound file types. Reviewed in previous comment.
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-09-13 14:07:41 PDT
https://hg.mozilla.org/comm-central/rev/fc2a8fd24ed1

Note You need to log in before you can comment on or make changes to this bug.