986850 - Use correct pluralization in preferences menus

Status: RESOLVED FIXED

(Calendar :: Preferences, defect)

Description

[Matthew Mecca [:mmecca]]

The unit selection menus in the preferences dialog should use PluralForm.

Comment 1

[Matthew Mecca [:mmecca]]

Attachment: pluralform-pref.diff

Applies on top of the patch in Bug 476937

Comment 2

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8395258 [details] [diff] [review]
pluralform-pref.diff

Review of attachment 8395258 [details] [diff] [review]:
-----------------------------------------------------------------

Instead of doing this via an attribute I think we should rather use a properties file, I haven't seen the plural forms in DTD files. We should be getting rid of the entities calendar.global.units.minute and calendar.global.units.minutes unless they are really needed, in most cases we want to be able to use the pluralized versions from the properties file.

I see there is already calendar-alarms.properties which has the plural forms in reminderCustomUnitMinutes, maybe we can consolidate these into a more general file like a new global.properties or the existing calendar.properties.

r- for now, I'd appreciate a new patch.

Comment 3

[Matthew Mecca [:mmecca]]

Attachment: Fix v2

Removes calendar.global.units.* entities and reminderCustomUnit* properties, instead using new customUnit_* properties in the calendar.properties file.

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8402224 [details] [diff] [review]
Fix v2

Review of attachment 8402224 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks for the cleanup! r=philipp with this one comment:

::: calendar/locales/en-US/chrome/calendar/calendar.properties
@@ +611,5 @@
> +# LOCALIZATION NOTE (customUnit)
> +# Used to determine the correct plural form of a unit
> +customUnit_minutes=#1 minute;#1 minutes
> +customUnit_hours=#1 hour;#1 hours
> +customUnit_days=#1 day;#1 days

Are these really custom? I'd prefer naming it something like "unitMinutes".

Comment 5

[Matthew Mecca [:mmecca]]

Attachment: Fix v3

Changes the property names. While it was convenient to generate the property names programatically, on further thought I think this way improves code readability. Requesting review on those changes.

Comment 6

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8402413 [details] [diff] [review]
Fix v3

Review of attachment 8402413 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=philipp

::: calendar/base/content/widgets/calendar-alarm-widget.xml
@@ +340,5 @@
>            okButton.setAttribute("aria-label", accessibleString);
> +
> +          let items = unitPopup.getElementsByTagName("xul:menuitem");
> +          for (let menuItem of items) {
> +              pluralString = calGetString("calendar", unitName(menuItem));

cal.calGetString (we have to fix that some day)

Comment 7

[Matthew Mecca [:mmecca]]

https://hg.mozilla.org/comm-central/rev/386eb4315d19