Closed Bug 360513 Opened 13 years ago Closed 13 years ago

Show file type suffix in import/export file picker filter description (wildmat pattern)

Categories

(Calendar :: Import and Export, enhancement)

x86
Windows 2000
enhancement
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: gekacheka, Unassigned)

Details

Attachments

(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061104 Calendar/0.4a1

In file pickers, file filter description should show how (what wildmat patterns) it filters files.  Not everyone knows what a "Calendar" or "iCalendar" file is.

Reproducible: Always

Actual Results:  
1. File > Open:   shows   Files of Type: "Calendar Files" (redundant 'files')
2. File > Import: shows   Files of Type: "iCalendar"
3. File > Export: shows   Files of TYpe: "iCalendar"

Expected Results:  
1. File > Open:   shows   Files of Type: "iCalendar (*.ics)"
2. File > Import: shows   Files of Type: "iCalendar (*.ics)"
3. File > Export: shows   Files of TYpe: "iCalendar (*.ics)"

Prefer automatically appending wildmat patterns, not part of localized string.
(patch -l -p 2 -i file.patch)

Adds (wildmat pattern) after description.
Wildmat pattern taken from actual pattern, so it must be correct, not dependent on localization.

Eliminates use of duplicate description properties (e.g. icsDesc duplicates filterCalendar)

calendar.js: add to open local file picker
import-export.js: add to all import and export file picker filters.
alarms.js: add to sound wave picker
calHtmlExport.js: htmlDesc -> filterHtml
calIcsImportExport.js: icsDesc -> filteriCalendar
calOutlookCSVImportExport.js: csvDesc -> filterOutlookCsv
calendar.properties: remove redundant '... file' or filetype.
Attachment #245425 - Flags: first-review?(lilmatt)
Comment on attachment 245425 [details] [diff] [review]
patch adds (wildmat pattern) after localized description

>diff -rut -x '*[#~]' mozilla/calendar/import-export/calIcsImportExport.js ../cvs20061111b/mozilla/calendar/import-export/calIcsImportExport.js
>--- mozilla/calendar/import-export/calIcsImportExport.js	2006-10-19 08:35:22.000000000 -0400
>@@ -59,7 +59,7 @@
>     var props = sbs.createBundle("chrome://calendar/locale/calendar.properties");
>     return([{defaultExtension:'ics', 
>              extensionFilter:'*.ics', 
>-             description: props.GetStringFromName('icsDesc')}]);
>+             description: props.GetStringFromName('filteriCalendar')}]);
See below CamelCase comment.

> };
> 
> calIcsImporter.prototype.importFromStream =
>@@ -227,7 +227,7 @@
>     var props = sbs.createBundle("chrome://calendar/locale/calendar.properties");
>     return([{defaultExtension:'ics', 
>              extensionFilter:'*.ics', 
>-             description: props.GetStringFromName('icsDesc')}]);
>+             description: props.GetStringFromName('filteriCalendar')}]);
See below CamelCase comment.

>--- mozilla/calendar/locales/en-US/chrome/calendar/calendar.properties	2006-10-29 15:44:08.000000000 -0500
>-#filter
>-filterCalendar=Calendar Files
>-filtervCalendar=vCalendar Files
>+#filter for export/import/open
>+filteriCalendar=iCalendar
>+filtervCalendar=vCalendar
Switch these to be "filterIcalendar" and "filterVcalendar" (and their uses in the code).
Any other CamelCase usage gets confusing.

> filterXcs=iCalendar XML Document
> filterXml=XML Document
>-filterRtf=Rich Text Format (RTF)
>-filterHtml=HTML Files
>+filterRtf=Rich Text Format
>+filterHtml=HTML
> filterCsv=Comma Separated
> filterOutlookCsv=Outlook Comma Separated
> filterRdf=iCalendar RDF
>-filterWav=WAV Files (*.wav)
>+filterWav=Waveform Audio
> 
>--- mozilla/calendar/resources/content/calendar.js	2006-11-06 13:41:13.000000000 -0500
>-    fp.appendFilter(gCalendarBundle.getString("filterCalendar"), "*.ics");
>+    var filterLabel = gCalendarBundle.getString("filteriCalendar");
See above CamelCase comment.


Please also fix wherever "filtervCalendar" is used.

r-, but real close
Attachment #245425 - Flags: first-review?(lilmatt) → first-review-
(patch -l -p 2 -i file.patch)

nit addressed.  I find reversing the iCalendar and vCalendar camelCase ordering from filteriCalendar to filterIcalendar even more confusing and prone to error, so I fixed it to filter_iCalendar (and filter_vCalendar).
Attachment #245425 - Attachment is obsolete: true
Attachment #245990 - Flags: first-review?(lilmatt)
Comment on attachment 245990 [details] [diff] [review]
patch adds (wildmat pattern) after localized description (filteriCalendar -> filter_iCalendar)

> Created an attachment (id=245990) [edit]
>
> so I fixed it to filter_iCalendar (and filter_vCalendar).

That adds a new style to the file and I'd like to avoid that.

Let's compromise. Use "filterIcs" and "filterVcs".

r=lilmatt with that fix
Attachment #245990 - Flags: first-review?(lilmatt) → first-review+
(patch -l -p 2 -i file.patch)

See comment 2 for description of changes.

Fixed property names to filterIcs and filterVcs as required by first review.

Reworded "HTML" to "Web Page", matching Firefox and less redundant, so it now appears as "Save as type:  Web Page (*.html, *.htm)"
Attachment #246668 - Flags: second-review?(mvl)
Comment on attachment 246668 [details] [diff] [review]
patch adds (wildmat pattern) after localized description (filteriCalendar -> filterIcs)

>+            fp.appendFilter(type.description + " ("+type.extensionFilter+")",
>+                            type.extensionFilter);

Joining user-visible strings like this makes me always worry. Does the format work in all languages? I think we should play safe and add the extension filter to the dtd/properties files.

>-filterWav=WAV Files (*.wav)
>+filterWav=Waveform Audio

Like it already was here for wav files.
(In reply to comment #6)
> Does the format work in all languages?
This sounds like something we should ask the localizers.

CCing Cédric Corazza for this.

Cédric: How do other languages/apps handle file wildcard matches such as these? Are there any l10n issues for us to be aware of?

How does Fx and Tb do this?
Severity: trivial → enhancement
(In reply to comment #7)
> CCing Cédric Corazza for this.
> 
> Cédric: How do other languages/apps handle file wildcard matches such as these?
> Are there any l10n issues for us to be aware of?
> 
> How does Fx and Tb do this?

Usually, localizers follow closely en-US strings unless it is against local customs.
For Firefox and Thunderbird, the "File Types" dialogs (in Preferences > Content tab) mention the extension for files.
I think this is not a localization issue, but merely what we want to bring to users.
Most of users don't even know what an HTML page is for instance! They just use their browser to browse the Web. If we could bring us that little peace of additional information --(*.htm, *.html)--, then we could help them to understand a little bit more what is the Web technology (well, it is just a file with 'html' extension, and they would know what file to search on their hard disk if they needed to). That is only an example.
IMHO, we should mention the file extension.
Concerning this patch, "filterWav=Waveform Audio" is untranslatable in French, for instance. I will be obliged to translate that entity as "WAV file (*.wav)"; same for the RTF entity : just translating this without indicating the extension wouldn't ring the bell for most people. I guess this is the same for en-US native speakers.
Hope this could help.
My point is not about the wildcards. It's about concatenating strings. The format is now hardcoded: "<fillin> (<fillin>)". If a language doesn't have brackets or needs a different order, that can't be done. But I don't know if such languages exist.
(In reply to comment #9)
> If a language doesn't have brackets or needs a different order...

Good point.  I'd be interested to know how you would display "WAV files (*.wav)" in RTL languages.

(patch -l -p 2 -i file.patch)

Rewritten to use a parameter so localizers can choose whether to include a preceding space between the filter label and the literal wildmat pattern, and what punctuation to use to offset the wildmat (not all languages use ASCII parentheses, though parentheses may still be appropriate since the wildmats are ASCII).

Using a parameter is preferrable over including the wildmat in the localized string so that (1) the wildmat will always be accurate (e.g., if we decided to add *.xhtml to the html filter wildmat, the localizations won't be out of date and won't need to be updated), and (2) localizers won't be tempted to localize the file extensions by mistake (for example by creating acronyms from translated words).
Attachment #245990 - Attachment is obsolete: true
Attachment #246668 - Attachment is obsolete: true
Attachment #247271 - Flags: second-review?(mvl)
Attachment #246668 - Flags: second-review?(mvl)
(In reply to comment #10)
> I'd be interested to know how you would display "WAV files
> (*.wav)" in RTL languages.

Using directional formatting characters like RLM (right-to-left mark) and LRM (left-to-right mark). The latest patch should be fine for this.
Comment on attachment 247271 [details] [diff] [review]
patch adds (wildmat pattern) after localized description (use parameter for wildmat)

ui-review+ granted; looks good.
Attachment #247271 - Flags: ui-review+
Comment on attachment 247271 [details] [diff] [review]
patch adds (wildmat pattern) after localized description (use parameter for wildmat)

r2=mvl
Attachment #247271 - Flags: second-review?(mvl) → second-review+
patch checked in.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I saw this bug in passing on a calendar update. Have you tested this patch on linux?

I ask because with the linux address book the filepicker was previously showing on linux "cvs format (*.cvs)(*.cvs)" whereas on windows it was showing "cvs format (*.cvs)".

It came down to the fact that the localized description was defined as "cvs format (*.cvs)", but the linux based filepicker adds the file extension onto the string provided, giving the extra "*.cvs".

We "resolved" it by removing the extension from the locale string and leaving windows as no extensions shown (see bug 192666). We possibly should have updated the filepicker, but we decided not to do that at the time.

Apologies if the status is now changed or this doesn't happen on linux for you, but I just thought it might benefit you to know.
Works fine for me on linux.
On Linux, does the filepicker depend on the window manager (e.g., KDE, Gnome) ?  

Possibly relevant changes to file picker on Linux:
bug 80636  Linux filepicker is not platform consistent [gtk filepicker now used]
bug 333653 Add pref to control which filepicker is used [avoid gtk2 filepicker]
verifed with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061215 Calendar/0.4a1
Status: RESOLVED → VERIFIED
Whiteboard: [litmus testcase wanted]
Litmus testcase 3016 created
Whiteboard: [litmus testcase wanted]
You need to log in before you can comment on or make changes to this bug.