Closed
Bug 466276
Opened 16 years ago
Closed 12 years ago
.caf files grayed out (not selectable) in Preferences > General > Use the following sound file > Browse...
Categories
(Thunderbird :: Preferences, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 18.0
People
(Reporter: tom, Assigned: javirid)
Details
Attachments
(1 file, 3 obsolete files)
2.66 KB,
patch
|
javirid
:
review+
javirid
:
ui-review+
|
Details | Diff | Splinter Review |
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.
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.
Version: unspecified → Trunk
Comment 2•15 years ago
|
||
Confirming with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090610 Shredder/3.0b3pre
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•15 years ago
|
||
A possible fix - ain't sure it's the right approach - Mark thoughts ?
Attachment #382783 -
Flags: review?(bugzilla)
Comment 4•15 years ago
|
||
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•15 years ago
|
||
(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 ?
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.
Updated•15 years ago
|
Attachment #382783 -
Flags: review?(bugzilla)
Comment 7•15 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
Adding filters only on the MacOS platform.
Assignee: nobody → leofigueres
Attachment #655700 -
Flags: review?(bwinton)
Comment 9•12 years ago
|
||
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.
Attachment #655700 -
Flags: review?(bwinton) → review+
Comment 10•12 years ago
|
||
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"?
Assignee | ||
Comment 11•12 years ago
|
||
(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 :-(
Assignee | ||
Comment 12•12 years ago
|
||
I would like to work on the improvement from Blake in comment 9. Patch will not be ready for the next merge, anyway.
Assignee | ||
Comment 13•12 years ago
|
||
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.
Attachment #655700 -
Attachment is obsolete: true
Attachment #658031 -
Flags: review?(bwinton)
Comment 14•12 years ago
|
||
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.
Attachment #658031 -
Flags: ui-review+
Attachment #658031 -
Flags: review?(bwinton)
Attachment #658031 -
Flags: review+
Updated•12 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 15•12 years ago
|
||
Amended string for sound file types. Reviewed in previous comment.
Attachment #382783 -
Attachment is obsolete: true
Attachment #658031 -
Attachment is obsolete: true
Attachment #660860 -
Flags: ui-review+
Attachment #660860 -
Flags: review+
Comment 16•12 years ago
|
||
https://hg.mozilla.org/comm-central/rev/fc2a8fd24ed1
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
You need to log in
before you can comment on or make changes to this bug.
Description
•