Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weblate not outputing Android <plurals> fields properly, causing app crashes #1550

Closed
eighthave opened this issue Jul 7, 2017 · 18 comments
Closed
Assignees
Labels
bug Something is broken. translate-toolkit Issues which need to be fixed in the translate-toolkit
Milestone

Comments

@eighthave
Copy link
Contributor

eighthave commented Jul 7, 2017

With Hebrew, the user interface shows One and Other as the options:

But the strings.xml then has One and Two filled out, then Many and Other blank:

<plurals name="details_last_update_weeks">
	<item quantity="one">עודכן לפני שבוע</item>
	<item quantity="two">עודכן לפני %1$d שבועות</item>
	<item quantity="many"></item>
	<item quantity="other"></item>
</plurals>

An empty Other string can cause crashes. Especially because we remove blank strings, since they cause other problems. Weblate should ensure that if any item of <plurals> has been translated, then <item quantity="other"> must be translated. Here's the Android source reference on the requirement for other:
https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/content/res/ResourcesImpl.java#268

Server configuration

https://hosted.weblate.org/translate/f-droid/f-droid

@eighthave
Copy link
Contributor Author

../../src/main/res/values-he/strings.xml:355: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 352     <string name="updates__hide_updateable_apps">הסתרת יישומונים</string>
 353     <string name="updates__show_updateable_apps">הצגת יישומונים</string>
 354 
 355     <plurals name="updates__download_updates_for_apps">
 356 	<item quantity="one">הורדת עדכון ליישומון אחד.</item>
 357 	<item quantity="other">הורדת עדכון ל־%1$d יישומונים.</item>
../../src/main/res/values-iw/strings.xml:355: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 352     <string name="updates__hide_updateable_apps">הסתרת יישומונים</string>
 353     <string name="updates__show_updateable_apps">הצגת יישומונים</string>
 354 
 355     <plurals name="updates__download_updates_for_apps">
 356 	<item quantity="one">הורדת עדכון ליישומון אחד.</item>
 357 	<item quantity="other">הורדת עדכון ל־%1$d יישומונים.</item>
../../src/main/res/values-he/strings.xml:437: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 434     <string name="notification_content_single_downloading_update">מתבצעת הורדת עדכון עבור „%1$s“…</string>
 435     <string name="notification_content_single_installing">ההתקנה של „%1$s“ מתבצעת כעת…</string>
 436     <string name="notification_content_single_installed">ההתקנה הצליחה</string>
 437     <plurals name="notification_summary_updates">
 438 	<item quantity="one">עדכון אחד</item>
 439 	<item quantity="other">%1$d עדכונים</item>
../../src/main/res/values-iw/strings.xml:437: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 434     <string name="notification_content_single_downloading_update">מתבצעת הורדת עדכון עבור „%1$s“…</string>
 435     <string name="notification_content_single_installing">ההתקנה של „%1$s“ מתבצעת כעת…</string>
 436     <string name="notification_content_single_installed">ההתקנה הצליחה</string>
 437     <plurals name="notification_summary_updates">
 438 	<item quantity="one">עדכון אחד</item>
 439 	<item quantity="other">%1$d עדכונים</item>
../../src/main/res/values-he/strings.xml:441: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 438 	<item quantity="one">עדכון אחד</item>
 439 	<item quantity="other">%1$d עדכונים</item>
 440 </plurals>
 441     <plurals name="notification_summary_installed">
 442 	<item quantity="one">יישומון הותקן</item>
 443 	<item quantity="other">%1$d יישומונים הותקנו</item>
+ 11 More Occurrences...
../../src/main/res/values-iw/strings.xml:441: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 438 	<item quantity="one">עדכון אחד</item>
 439 	<item quantity="other">%1$d עדכונים</item>
 440 </plurals>
 441     <plurals name="notification_summary_installed">
 442 	<item quantity="one">יישומון הותקן</item>
 443 	<item quantity="other">%1$d יישומונים הותקנו</item>
../../src/main/res/values-he/strings.xml:454: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 451     <string name="notification_title_summary_installed">הותקן בהצלחה</string>
 452     <string name="tts_category_name">קטגוריה %1$s</string>
 453 
 454     <plurals name="tts_view_all_in_category">
 455 	<item quantity="one">הצגת יישומון %1$d בקטגוריה %2$s</item>
 456 	<item quantity="other">הצגת כל %1$d היישומונים בקטגוריה %2$s</item>
../../src/main/res/values-iw/strings.xml:454: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 451     <string name="notification_title_summary_installed">הותקן בהצלחה</string>
 452     <string name="tts_category_name">קטגוריה %1$s</string>
 453 
 454     <plurals name="tts_view_all_in_category">
 455 	<item quantity="one">הצגת יישומון %1$d בקטגוריה %2$s</item>
 456 	<item quantity="other">הצגת כל %1$d היישומונים בקטגוריה %2$s</item>
../../src/main/res/values-he/strings.xml:460: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 457 </plurals>
 458 
 459     <string name="details_last_updated_today">עודכן היום</string>
 460     <plurals name="details_last_update_days">
 461 	<item quantity="one">עודכן לפני יום</item>
 462 	<item quantity="other">עודכן לפני %1$d ימים</item>
../../src/main/res/values-iw/strings.xml:460: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 457 </plurals>
 458 
 459     <string name="details_last_updated_today">עודכן היום</string>
 460     <plurals name="details_last_update_days">
 461 	<item quantity="one">עודכן לפני יום</item>
 462 	<item quantity="other">עודכן לפני %1$d ימים</item>
../../src/main/res/values-he/strings.xml:464: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 461 	<item quantity="one">עודכן לפני יום</item>
 462 	<item quantity="other">עודכן לפני %1$d ימים</item>
 463 </plurals>
 464     <plurals name="details_last_update_weeks">
 465 	<item quantity="one">עודכן לפני שבוע</item>
 466 	<item quantity="other">עודכן לפני %1$d שבועות</item>
../../src/main/res/values-iw/strings.xml:464: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 461 	<item quantity="one">עודכן לפני יום</item>
 462 	<item quantity="other">עודכן לפני %1$d ימים</item>
 463 </plurals>
 464     <plurals name="details_last_update_weeks">
 465 	<item quantity="one">עודכן לפני שבוע</item>
 466 	<item quantity="other">עודכן לפני %1$d שבועות</item>
../../src/main/res/values-he/strings.xml:468: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 465 	<item quantity="one">עודכן לפני שבוע</item>
 466 	<item quantity="other">עודכן לפני %1$d שבועות</item>
 467 </plurals>
 468     <plurals name="details_last_update_months">
 469 	<item quantity="one">עודכן לפני חודש</item>
 470 	<item quantity="other">עודכן לפני %1$d חודשים</item>
../../src/main/res/values-iw/strings.xml:468: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 465 	<item quantity="one">עודכן לפני שבוע</item>
 466 	<item quantity="other">עודכן לפני %1$d שבועות</item>
 467 </plurals>
 468     <plurals name="details_last_update_months">
 469 	<item quantity="one">עודכן לפני חודש</item>
 470 	<item quantity="other">עודכן לפני %1$d חודשים</item>
../../src/main/res/values-he/strings.xml:472: For locale "he" (Hebrew) the following quantities should also be defined: many, two
 469 	<item quantity="one">עודכן לפני חודש</item>
 470 	<item quantity="other">עודכן לפני %1$d חודשים</item>
 471 </plurals>
 472     <plurals name="details_last_update_years">
 473 	<item quantity="one">עודכן לפני שנה</item>
 474 	<item quantity="other">עודכן לפני %1$d שנים</item>
../../src/main/res/values-iw/strings.xml:472: For locale "iw" (Hebrew) the following quantities should also be defined: many, two
 469 	<item quantity="one">עודכן לפני חודש</item>
 470 	<item quantity="other">עודכן לפני %1$d חודשים</item>
 471 </plurals>
 472     <plurals name="details_last_update_years">
 473 	<item quantity="one">עודכן לפני שנה</item>
 474 	<item quantity="other">עודכן לפני %1$d שנים</item>
Priority: 8 / 10
Category: Correctness:Messages
Severity: Error
Explanation: Missing quantity translation.
Different languages have different rules for grammatical agreement with quantity. In English, for example, the quantity 1 is a special case. We write "1 book", but for any other quantity we'd write "n books". This distinction between singular and plural is very common, but other languages make finer distinctions.

This lint check looks at each translation of a <plural> and makes sure that all the quantity strings considered by the given language are provided by this translation.

For example, an English translation must provide a string for quantity="one". Similarly, a Czech translation must provide a string for quantity="few".


To suppress this error, use the issue id "MissingQuantity" as explained in the Suppressing Warnings and Errors section.

@nijel
Copy link
Member

nijel commented Jul 9, 2017

Apparently Gettext is not following CLDR plurals here and it causes problems in this case. Weblate is based on Gettext ones, where there are just two plural forms (n != 1), while CLDR (and Android) use four (one, two, many, other). This will be tricky to address...

@nijel nijel added the bug Something is broken. label Jul 9, 2017
@eighthave
Copy link
Contributor Author

As a temporary workaround, maybe you could just stick the Other value into Two and Many?

@eighthave
Copy link
Contributor Author

eighthave commented Jul 9, 2017

It seems that some languages are including more than One/Other. For example, Russian has One, Few, and Many, but is missing Other:

<plurals name="details_last_update_days">
	<item quantity="one">Обновлено %1$d день назад</item>
	<item quantity="few">Обновлено %1$d дня назад</item>
	<item quantity="many">Обновлено %1$d дней назад</item>
</plurals>

And Slovak includes One, Few, Many, and Other, but Many is apparently not supposed to be there:

<plurals name="updates__download_updates_for_apps">
	<item quantity="one">Stiahnuť aktualizáciu pre %1$d aplikáciu.</item>
	<item quantity="few">Stiahnuť aktualizácie pre %1$d aplikácie.</item>
	<item quantity="many">Stiahnuť aktualizácie pre %1$d aplikácií.</item>
	<item quantity="other">Stiahnuť aktualizácie pre %1$d aplikácií.</item>
</plurals>

@eighthave
Copy link
Contributor Author

Serbian seems to have the correct plurals (One, Few, Other):

<plurals name="tts_view_all_in_category">
	<item quantity="one">Прикажи %1$d апликацију у категорији %2$s</item>
	<item quantity="few">Прикажи све %1$d апликације у категорији %2$s</item>
	<item quantity="other">Прикажи свих %1$d апликација у категорији %2$s</item>
</plurals>

@nijel
Copy link
Member

nijel commented Jul 13, 2017

This is problem only for languages where CLDR plurals do not match the ones commonly used by Gettext. I guess there won't be much of such languages...

PS: Slovak should have one/few/many/other, see http://www.unicode.org/cldr/charts/29/supplemental/language_plural_rules.html#sk.

@nijel
Copy link
Member

nijel commented Jul 14, 2017

Thinking more about this we really have to add plural definitions per translation, which would be based on file format and fallback to language definition if nothing is defined for file format or within the file. This way we could also address different plural forms being used for one language (such as #901).

@nijel
Copy link
Member

nijel commented Jul 14, 2017

Some formats will be easy to support properly:

  • Use CLDR rules for Android (they can be probably extracted from Babel)
  • Use Plural-Forms header for Gettext PO

Others might be a bit tricky and we will probably stick to current language defaults:

Note: The Babel rules are not same as Gettext ones we're currently using, so it might need some more tweaks:

>>> import babel
>>> l = babel.Locale('cs')
>>> l.plural_form.tags
frozenset(['few', 'one', 'many'])
>>> l.plural_form.rules
{'few': 'i in 2..4 and v in 0', 'one': 'i in 1 and v in 0', 'many': 'v not in 0'}
>>> l.plural_form.abstract
[('few', ('and', (('relation', ('in', (u'i', ()), ('range_list', [(('value', (2,)), ('value', (4,)))]))), ('relation', ('in', (u'v', ()), ('range_list', [(('value', (0,)), ('value', (0,)))])))))), ('many', ('not', (('relation', ('in', (u'v', ()), ('range_list', [(('value', (0,)), ('value', (0,)))]))),))), ('one', ('and', (('relation', ('in', (u'i', ()), ('range_list', [(('value', (1,)), ('value', (1,)))]))), ('relation', ('in', (u'v', ()), ('range_list', [(('value', (0,)), ('value', (0,)))]))))))]

The variables are defined in the specification, there is also tool to convert them (written in PHP): https://github.com/mlocati/cldr-to-gettext-plural-rules

nijel referenced this issue in TeamNewPipe/NewPipe Dec 7, 2017
Currently translated at 100.0% (244 of 244 strings)
nijel referenced this issue in TeamNewPipe/NewPipe Jan 2, 2018
Currently translated at 91.8% (224 of 244 strings)
nijel added a commit to WeblateOrg/language-data that referenced this issue Jan 9, 2018
See WeblateOrg/weblate#1550

Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel
Copy link
Member

nijel commented Jan 9, 2018

After digging into this for few hours (trying to add CLDR plurals to Weblate), now I finally understand where the problem is - it's not that the CLDR plural rules would be that different (the differences are there, but they are limited to few languages).

The problem lies a bit deeper:

Translate-toolkit always adds other to the plurals:

https://github.com/translate/translate/blob/441fcd1e40e2902e6d88616e8fd4f5dd0103bf44/translate/storage/aresource.py#L309-L310

It might sound weird, but apparently the root cause is in Babel not returning other for most of the languages it should. There is even bug reported for similar thing since April 21, see python-babel/babel#495

@nijel nijel added the translate-toolkit Issues which need to be fixed in the translate-toolkit label Jan 9, 2018
@nijel nijel self-assigned this Jan 9, 2018
@nijel
Copy link
Member

nijel commented Jan 9, 2018

This should be fixed by following PR which replaces Babel usage in translate-toolkit by own plurals data: translate/translate#3762

It would be really great if somebody with Android knowledge (eg. @eighthave , @TobiGr, @theScrabi ) could check if the tags are correct there - I was not able to find some official docs for Android covering this and the CLDR is still a bit magic for me as a result I'm not sure if the table I've generated correct (I've checked few languages which I know were problematic and they look correct).

nijel added a commit that referenced this issue Jan 9, 2018
It contains several cleanups to remove duplicates and to bring our data
closer to CLDR.

See #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
@eighthave
Copy link
Contributor Author

I don't quite understand what I should check. I looked at this diff of 5d853aa and I have no idea what that data is. I looked at translate/translate@65b4000, and I could see that some of the languages are missing "other". All languages must have at least "other", that is a hard requirement. If a language has only one plurals form, it should be "other". I would include a CI test case that checks that all languages have "other" in them. I can see that be, pl, and uk are missing "other".

@nijel
Copy link
Member

nijel commented Jan 19, 2018

No, not all languages use other for integers (that's what android cares for plurals), see for example Belarusian, Polish or Russian.

And yes translate/translate@65b4000 is think I'd like to get reviewed (or more precisly https://github.com/translate/translate/pull/3762/files).

@eighthave
Copy link
Contributor Author

eighthave commented Jan 19, 2018 via email

@TobiGr
Copy link

TobiGr commented Jan 20, 2018

I agree with @eighthave. Android requires "other" and if it does not exist this can cause a crash.

@nijel
Copy link
Member

nijel commented Jan 22, 2018

So far I thought that Android is simply following CLDR, but that doesn't seem to be the case, so this has to be implemented somewhere (apparently documentation doesn't seem to exist).

To me it really looks weird that it would require the "other" even for languages where it won't be used - the getQuantityString used to evaluate this accepts only integers...

Looking at the code I still think that "other" is not needed as long as all valid choices are there.

So can somebody actually test that other is really needed in all cases? Testing simple string with one of above mentioned languages (eg. Polish) should do that.

@mauriciocolli
Copy link

I've test it and can confirm, some languages don't require the other field, here's some tests that I did:

The test consists on loop through 0 to 220 and check if the the getQuantityString returns the language code or throws an error (using "language_code" + %1$din the strings).
API 25 23 19 16
Without field "other" only
be, pl, ru, uk
same as 25 same as 25, but some fallback same as 19
Without field "other"
(plural deleted if empty)
same as above,
some fallback
same as 25 same as 25 same as 25
Following PR rules strictly ✓, except bem, bez same as 19
  • Exception when fail: Resources$NotFoundException: Plural resource ID #id quantity=x item=other
  • You cannot have an empty plurals tag
  • Deleted plurals just fallback to default

You can find the generator (to generate the strings.xml file and their contents) that follows the rules of PLURAL_TAGS that it's in your pull request (translate/translate#3762) and the app source code here:
https://github.com/mauriciocolli/LanguageTest

There's some already compiled apks if you want to test in your device.

@nijel
Copy link
Member

nijel commented Jan 25, 2018

@mauriciocolli Thanks for verifying this! I've just deployed this change to Hosted Weblate, so that will give these changes some more real life testing.

There are still few languages whose plural rules in CLDR differ to what is usually used in Gettext, so these will need additional fixes on the Weblate side:

nijel added a commit that referenced this issue Jan 29, 2018
We need to support more plural rules for language and this is
preparation work for this.

Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
This is consistent with existing code.

Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
This currently works only for gettext which actually stores plural
formula.

Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
This now closer follows CLDR and alternative plurals are listed
separately.

Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
It is now handled in separate model.

Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
It was just temporary for migration.

Issue #1550

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Jan 29, 2018
Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel
Copy link
Member

nijel commented Jan 30, 2018

Thank you for your report, the issue you have reported has just been fixed in Weblate. The complete fix depends on translate/translate#3762 (which is already deployed in Hosted Weblate).

  • In case you see problem with the fix, please comment on this issue.
  • In case you see similar problem, please open separate issue.
  • If you are happy with the outcome, consider supporting Weblate by donating.

@nijel nijel closed this as completed Jan 31, 2018
nijel added a commit to translate/translate that referenced this issue Feb 8, 2018
These seem to be broken as they are missing the 'other' tag. The problem
is that not all languages use 'other' tag, so adding it back requires
CLDR knowledge. In this case it's better to have full tags defined
inside translate-toolkit than relying on broken library and fixing the
data.

See following issue trackers for more details:

* WeblateOrg/weblate#1550
* python-babel/babel#495
nijel added a commit to WeblateOrg/docker that referenced this issue Mar 12, 2018
This brings several important features and fixes:

- fixes to the JSON storage (see WeblateOrg/weblate#1671)
- Android plural fixes (see WeblateOrg/weblate#1550)
- improved PHP storage
- fixes to the YAML storage

Signed-off-by: Michal Čihař <michal@cihar.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken. translate-toolkit Issues which need to be fixed in the translate-toolkit
Projects
None yet
Development

No branches or pull requests

4 participants