Closed Bug 482460 Opened 15 years ago Closed 15 years ago

Recurrence summary in Edit Event dialog can't be correctly translated when local languages have grammatical genders for weekdays, months, ordinal numbers... .

Categories

(Calendar :: General, defect)

Product:
Calendar
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: bv1578, Assigned: bv1578)

Details

Attachments

(1 file, 1 obsolete file)

patch with required changes
23.46 KB, patch
Fallen
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; it-IT; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090308 Lightning/1.0pre Shredder/3.0b3pre

Recurrence summary in edit event dialog represents the repeat pattern of a recurring event in natural language. Many languages have feminine/masculine gender nouns in particular for weekdays, and without a minimal management of this aspect, some sentences in recurrence summary sound strange in those languages, or seem the result of translation errors because gender between (e.g.) weekdays nouns and articles or ordinal numbers nouns (depending on language) don't match.

The issue is known, as examples see: 
https://bugzilla.mozilla.org/show_bug.cgi?id=463273#c39
https://bugzilla.mozilla.org/show_bug.cgi?id=463273#c41
a very similar bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=338167#c0


Reproducible: Always

Steps to Reproduce:
1. On a localized version of Sunbird\Lightning with a local language that has grammatical genders for weekday, write a recurring event with a custom repeat rule that contains a weekday or a ordinal number of feminine gender.


Actual Results:  
genders between weekday noun and ordinal noun (or article) don't match in recurrence summary.



Expected Results:  
genders should match.
I've started to write a patch that takes (partially) care of the issue for weekdays names.
I've added in calendar-event-dialog.properties file some strings with a "Female" suffix (one for every string with a weekday noun inside) that could be used to write the same sentences with grammar rules for feminine gender: 

weeklyNthOn              ->   weeklyNthOnFemale        
monthlyEveryOfEvery      ->   monthlyEveryOfEveryFemale  
monthlyNthOfEvery        ->   monthlyNthOfEveryFemale    
yearlyOnEveryNthOfNth    ->   yearlyOnEveryNthOfNthFemale
yearlyNthOnNthOf         ->   yearlyNthOnNthOfFemale

Code inside calendar-dialog-utils.js will select "female" strings instead of normal strings if weekday gender is feminie. The gender of a weekday should be specified with furthers strings with "Gender" suffix (a string for every weekday) that Localizer should file with a "female" or "male" string according to the weekday gender in their language.
E.g. Polish
(copied from https://bugzilla.mozilla.org/show_bug.cgi?id=338167#c0):

repeatDetailsDay1=niedziela        niedziela (Sunday) feminine gender noun
repeatDetailsDay1Gender=female   <-
repeatDetailsDay2=poniedzialek     poniedzialek (Monday) masculine gender noun
repeatDetailsDay2Gender=male     <-
repeatDetailsDay3=wtorek           wtorek (Thuesday) masculine gender noun
repeatDetailsDay3Gender=male     <-
repeatDetailsDay4=sroda            sroda (Wednesday) feminine gender noun
repeatDetailsDay4Gender=female   <-
repeatDetailsDay5=czwartek         czwartek (Thursday) masculine gender noun
repeatDetailsDay5Gender=male     <-
repeatDetailsDay6=piatek           piatek (Friday) masculine gender noun
repeatDetailsDay6Gender=male     <-
repeatDetailsDay7=sobota           sobota (Saturday) feminine gender noun
repeatDetailsDay7Gender=female   <-
 
Obviously, inside "female" rule strings, should also be added strings for ordinal numbers nouns for feminine gender (will be selected together with female rule string):

repeatDetailsOrdinal1Female         (female noun of "first")
repeatDetailsOrdinal2Female         (female noun of "second")
....
repeatDetailsOrdinal-1Female        (female noun of "last")

Furthermore, for languages that have articles related to the noun (not only related to the gender), we could consider to change ordinal strings (male and female form) and make them contain the article:
                                     English        (e.g. Italian)
                                   -----------       ------------
repeatDetailsOrdinal1: "first"  -> "the first"    -> "il primo" 
repeatDetailsOrdinal2: "second" -> "the second"   -> "il secondo"
...
repeatDetailsOrdinal-1: "last"  -> "the last"     -> "l'ultimo"

but this involve changing of some existing strings.
The same has been done for drop-down menu in Edit Recurrence window (see calendar-event-dialog.dtd in bug 463273 http://tinyurl.com/bmgxfz).


I know, it doesn't fix every cases in every language, but it gives little more flexibility and, with a few code (to switch male/female it needs only a 'if'), it allows to fix cases like:

- Polish : https://bugzilla.mozilla.org/show_bug.cgi?id=338167#c0
 First Monday -> Pierwszy poniedzialek  (poniedzialek masculine noun) 
 First Sunday -> Pierwsza niedziela     (niedziela feminine noun)

- Italian: https://bugzilla.mozilla.org/show_bug.cgi?id=463273#c39
 The First Monday -> Il primo lunedì    (Lunedì masculine noun)
 The First Sunday -> La prima domenica  (Domenica feminine noun)
 
I've already asked to Philipp Kewisch that agree but he's told me to ask for some localizers' opinion too.
What do you think? Could a concept like this be useful for enough languages or does the issue need a different approach?
It's great to see that even here it's possible to progress!

May I suggest to use 'f' and 'm' instead of 'female' and 'male'?
Decathlon originally planned on using m and f but I'd prefer to be more verbose, to make it easier to understand for possible new localizers. Since the words male and female are quite similar I personally think its ok to put up that burden.
Assignee: nobody → bv1578
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Great idea :-) CC: calendar-l10n@mozilla.bugs
(In reply to comment #3)
> Decathlon originally planned on using m and f but I'd prefer to be more
> verbose, to make it easier to understand for possible new localizers. Since the
> words male and female are quite similar I personally think its ok to put up
> that burden.

Well, we have more cryptic things in our code... :) Such as plural strings, for example. At least these words could be 'masculine' and 'feminine' then, as I believe genders are named like that in English. Words 'male' and 'female' just don't sound in such context, at least for me.
Would it be worthwhile to generalize this setup a bit? Not all languages are based on gender, or not fully. I know that Dutch is a bit inconsistent in that respect.
Instead of 'gender', just use 'class' or 'type' or 'label'. Then have repeatDetailsDay1Class=Foo
and
repeatDetailsOrdinal1Foo=...

Or am I trying to solve a non-problem?
(In reply to comment #6)
> Would it be worthwhile to generalize this setup a bit? Not all languages are
> based on gender, or not fully. 
> Instead of 'gender', just use 'class' or 'type' or 'label'. Then have
> repeatDetailsDay1Class=Foo
> and
> repeatDetailsOrdinal1Foo=...
> 
> Or am I trying to solve a non-problem?

Reading on Wikipedia (http://en.wikipedia.org/wiki/Grammatical_gender) it wouldn't seem a "non-problem":
"In linguistics, grammatical genders, sometimes also called noun classes, are classes of nouns reflected in the behavior of associated words"
moreover:
"Although some authors use the term "noun class" as a synonym or an extension of "grammatical gender", for others they are separate concepts. One can in fact say that grammatical gender is a type of noun class."

so, we could consider to use something like this:

Gender      ->    Nounclass  
masculine   ->    Class1    (not used as suffix for rule strings to avoid
                             changing existing string)
feminine    ->    Class2

or is it a bit cryptic?
By the way, are there languages with more than two genders/classes for weekdays?
Attached patch proposed patch (obsolete) — — Splinter Review 
First attempt.
I've done everything in comment #1 and I've used a mix of terms from comment #5 and comment #6:
'Nounclass' instead of 'Gender' for rule strings suffix and 'feminine'\'masculine' instead of 'female'\'male' for string selection, but we can also use something different.
I've also added comments and localization notes.
Attachment #367912 - Flags: review?(philipp)
We should get a post out in .l10n about this to gather some wider data. I'm not sure that all locales can go with just two noun classes, though in this special case, it might work.

In terms of wallclock time, what's the target milestone for this bug? If it would have one.
Comment on attachment 367912 [details] [diff] [review]
proposed patch

>diff --git a/calendar/base/content/calendar-dialog-utils.js b/calendar/base/content/calendar-dialog-utils.js
>--- a/calendar/base/content/calendar-dialog-utils.js
>+++ b/calendar/base/content/calendar-dialog-utils.js
>@@ -79,12 +79,21 @@ function recurrenceRule2String(recurrenc
>                 return Math.abs(day) % 8;
>             }
>             function day_position(day) {
>                 let dow = day_of_week(day);
>                 return (Math.abs(day) - dow) / 8 * (day < 0 ? -1 : 1);
>             }
>+            function nounClass(aDayString, aruleString) {
>+                if (getRString(aDayString + "Nounclass") == "feminine") {
>+                    // noun class equivalent to feminine grammatical gender
>+                    return aruleString + "Feminine";    
>+                } else {
>+                    // noun class equivalent to masculine grammatical gender
>+                    return aruleString;
>+                }
>+            }
Shorter (and fixed caps on parameter):

function nounClass(aDayString, aRuleString) {
    let nounClass = getRString(aDayString + "Nounclass");
    return aRuleString + nounClass.substr(0, 1).toUpperCase() + nounClass.substr(1);
}

For this you need to change the rulestring part to blablaMasculine/blablaFeminine, but this way we are future proof if there are other noun classes.


>+                    //select noun class (grammatical gender) according to the first day of the list
>+                            //select noun class (grammatical gender) for yearlyString
>+                            //select noun class (grammatical gender) for ordinalString
For these lines: Space after //, wrap at 80 characters.


Space after

>@@ -698,6 +723,7 @@ function updateLink() {
>           // HACK the url-link doesn't crop when setting the value in onLoad
>           setElementValue("url-link", itemUrlString);
>           setElementValue("url-link", itemUrlString, "href");
>         }, 0);
>     }
> }
>+

Extra newline not needed.

> repeatDetailsDay1=Sunday
>+repeatDetailsDay1Nounclass=feminine
Why is Sunday feminine? Is that intended?
> # LOCALIZATION NOTE (monthlyEveryOfEvery):
> # Edit recurrence window -> Recurrence pattern -> Monthly repeat rules
> # %1$S - weekday
> # #2 - interval
> # i.e. "every MONDAY of every 3 months" 
> #  more specific: every monday of January, April, July, October
> monthlyEveryOfEvery=every %1$S of every month;every %1$S of every #2 months
...Add a newline here and also in other places between a string with localization note and the next note.

>+# LOCALIZATION NOTE (monthlyEveryOfEveryFeminine):
>+# Edit recurrence window -> Recurrence pattern -> Monthly repeat rules
>+# Same as monthlyEveryOfEvery (see previous string) but only for languages where
>+# weekday (%1$S) is a feminine gender noun (or belongs to a different noun class).
>+# %1$S - weekday
>+# #2 - interval
>+# i.e. "every MONDAY of every 3 months" 
>+#  more specific: every monday of January, April, July, October
>+monthlyEveryOfEveryFeminine=every %1$S of every month;every %1$S of every #2 months




>-repeatDetailsOrdinal1=first
>-repeatDetailsOrdinal2=second
>-repeatDetailsOrdinal3=third
>-repeatDetailsOrdinal4=fourth
>-repeatDetailsOrdinal5=fifth
>-repeatDetailsOrdinal-1=last
>+repeatDetailsOrdinal1=the first
>+repeatDetailsOrdinal2=the second
>+repeatDetailsOrdinal3=the third
>+repeatDetailsOrdinal4=the fourth
>+repeatDetailsOrdinal5=the fifth
>+repeatDetailsOrdinal-1=the last
Since you are changing the strings, you need to change the string names. Please make sure you do this throughout the patch.


r=philipp codewise, I'll set the bits as soon as I have tested this a bit.
I've done required changes.
I've also deleted some comments.
After comment #9 and with 'nounClass()' function proposed in comment #10, 
to make easier to add others noun classes, I've turned back to my previous idea (comment #7) and I've used:
"nounclass1" instead of "masculine"
"nounclass2" instead of "feminine"
and I've added localization notes that invite localizer to add themselves strings for others noun classes if their languages need them.
I don't know if I've done the right thing. Please, before a complete review, could you check localization notes (for a better English as well)? If it needs, I can add an attachment with the previous (corrected) patch with masculine/feminine and/or with previous localization notes.


About this bug, another issue should be considered to get a better translation.
As a matter of fact some languages need a plural for weekdays nouns inside sentences like "Every ..." e.g. in Italian language (see also https://wiki.mozilla.org/Talk:L20n:Examples):

   "every Saturday", "every Sunday of March", ...
-> "tutti i Sabati", "tutte le Domeniche di Marzo", ...

with "Sabati" and "Domeniche" plurals of "Sabato" (Saturday) and "Domenica" (Sunday).
Differences between first part of both strings ("tutti i" and "tutte le") is due to grammatical gender and it's well handled by the patch about noun classes, remains plural of weekdays to match.

With the same approach of the previous patch, I've thought to use a "control string" to enable weekdays plurals for languages that need them in sentences like "Every ..." and add seven strings for weekdays plurals (e.g. repeatDetailsDayxPlural). These plurals don't need to be processed by PluralForm function because they refer all to the same case: "every ...", so every localizer can write the right form in these seven strings and a  localization note will explain the use of these plurals and how to write the "control string" to make them active.
A function (like nounClass() in the previous patch) will select singular/plural weekday if the "control string" is 'true'.
Something like:
-inside calendar-event-dialog.properties file:

  StringsWith_Every_NeedPlural=true (or false)     (<- "control string")
  
  repeatDetailsDay1Plural=Sundays
  repeatDetailsDay2Plural=Mondays
  repeatDetailsDay3Plural=Tuesdays
  repeatDetailsDay4Plural=Wednesdays
  repeatDetailsDay5Plural=Thursdays
  repeatDetailsDay6Plural=Fridays
  repeatDetailsDay7Plural=Saturdays  

-inside calendar-dialog-utils.js:

  function pluralWeekdays(aDayString) {
      let plural = getRString("StringsWith_Every_NeedPlural") == "true";
      return plural ? aDayString + "Plural" : aDayString;
  }
  
and using 'pluralWeekdays(dayString)' instead of 'dayString' for strings that need the weekdays plural (that are three, one of them only for the 'singular' part) i.e.:
weeklyNthOn          -> every %1$S;every #2 weeks on %1$S (first part only)
monthlyEveryOfEvery  -> every %1$S of every month;every %1$S of every #2 months
yearlyOnEveryNthOfNth-> every %1$S of %2$S;every #3 years on every %1$S of %2$S

What do you think about that?
Attachment #367912 - Attachment is obsolete: true
Attachment #368901 - Flags: review?(philipp)
Attachment #367912 - Flags: review?(philipp)
Decathlon, thanks for this bug!
I tried translate calendar-event-dialog.properties (on Russian) but I receive only grammatical horror. 
Article about Russian grammar - http://en.wikipedia.org/wiki/Russian_grammar
Hello Alexey,
as I said in comment #1  this little patch isn't exactly the panacea ;-) for this kind of problem and IMHO it will be impossible to fix every case for every language. 
With the patch, I think you get as many errors, or horrors :-), as you got without the patch (maybe the same -1 ;-)?) because the patch doesn't consider any grammatical rule for any language, it only merely adds one more possibility: you can use  different strings for different weekday genders (or noun classes). Inside these new strings you can use ordinals with gender accordance to weekday and change some words to match the weekday gender, but new strings have the same rules and the same construction and they don't consider at all grammatical rules, which, for a language like Russian, might be very complex (I think).
By the way, could you write some specific examples in Russian language and explain them? You know, Wikipedia is a bit generic.
>By the way, could you write some specific examples in Russian language and
explain them?
I added a few examples in Russian on a page in the https://wiki.mozilla.org/Talk:L20n:Examples
(In reply to comment #14)
> >By the way, could you write some specific examples in Russian language and
> explain them?
> I added a few examples in Russian on a page in the
> https://wiki.mozilla.org/Talk:L20n:Examples

Second patch helps a little in Russian language too.

Adding a third noun class 'nounclass3' for neuter gender (Sunday -> repeatDetailsDay1Nounclass=nounclass3), you could add a third string with suffix 'Nounclass3' to every rule string in calendar-event-dialog.properties:

weeklyNthOnNounclass3
monthlyEveryOfEveryNounclass3
yearlyOnEveryNthOfNthNounclass3
etc.

and this will fix cases where "every" depends on weekday gender (see rule strings above):
Every Monday -> Каждый понедельник //m   (Monday    nounclass1)
Every Tuesday -> Каждый вторник //m      (Tuesday   nounclass1)
Every Wednesday -> Каждую среду //f      (Wednesday nounclass2)
Every Thursday -> Каждый четверг //m     (Thursday  nounclass1)
Every Friday -> Каждую пятницу //f       (Friday    nounclass2)
Every Saturday -> Каждую субботу //f     (Saturday  nounclass2)
Every Sunday -> Каждое воскресенье //n   (Sunday    nounclass3)

e.g. rule string weeklyNthOn:
weeklyNthOnNounclass1=Каждый %1$S;...
weeklyNthOnNounclass2=Каждую %1$S;...
weeklyNthOnNounclass3=Каждое %1$S;...


The patch should also allow to fix this case:
The third Monday of the month -> Третий Понедельник месяца
The third Sunday of the month -> Третье Воскресенье месяца  
because (it seems) "third" ("Третий" and "Третье") depends on grammatical gender of weekday.

For the differences in plurals (the "years" case on Wiki page), the patch doesn't help, but Russian has Plural rule #7 (https://developer.mozilla.org/En/Localization_and_Plurals) and has 3 forms, so, *if I've fully understood PluralForm functioning*, it would need a third form in every rule string.
 e.g. rule string 'yearlyNthOn' should be:

yearlyNthOn=every %1$S %2$S;every #3 years on %1$S %2$S;every #3 years on %1$S %2$S
 
  first string for 1,21,31,41 ... years (год)
  second string for 2,3,4,22,... years (года)
  third string for 5,6,7,... years (лет)
(actually, to match plural case 21, 31,... , the first form should be equal to the others, but I think case '1' should be considered a particular case in Russian too and this would require a code change or another string).
I'm sitting here pondering back and forth if we shouldn't just add a jsm for some version of l20n and start using that.
Attachment #368901 - Flags: review?(philipp) → review+
Comment on attachment 368901 [details] [diff] [review]
patch with required changes

Codewise this looks good, although I haven't tested it and can't comment on how much it helps localizers. Given the previous comments it sounds promising though.

Decathlon, should I check this in as it is, or is there anything else that needs changing? Please set the checkin-needed keyword as needed.
(In reply to comment #17)
> Decathlon, should I check this in as it is, or is there anything else that
> needs changing? Please set the checkin-needed keyword as needed.

What about issue, and proposed solution, in comment #11?
If solution is fine, I can post an updated patch or add another one, only for that issue, after the current one has been checked in. If a separate patch is preferable, and if localizers agree with localization notes, for me you can check in the current one.
(In reply to comment #18)
> What about issue, and proposed solution, in comment #11?
> If solution is fine, I can post an updated patch or add another one, only for
> that issue, after the current one has been checked in. If a separate patch is
> preferable, and if localizers agree with localization notes, for me you can
> check in the current one.
Since the patch is quite large, I think its better to create a separate bug. I promise I'll review faster next time :-)
Keywords: checkin-needed
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/4ed0f72884eb>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Just out of curiosity: is it OK to remove all Nounclass2 lines for locales that don't need it?
You need to have each locale line in the file, you can't just remove them. Either keep the value empty or just use the same value as for Nounclass1. I'd prefer the latter.
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: