Opened 3 years ago
Closed 14 months ago
#34221 closed Bug (fixed)
Plural-Forms in .po files break Django's translation precedence.
| Reported by: | Stefano Parmesan | Owned by: | Claude Paroz |
|---|---|---|---|
| Component: | Internationalization | Version: | 4.1 |
| Severity: | Normal | Keywords: | i18n |
| Cc: | Claude Paroz, Dmytro Litvinov, viktorparipas92 | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
According to the documentation, Django reads translations from LOCALE_PATHS first, then from apps in INSTALLED_APPS, and then from its own folders.
This behaviour breaks if the files in LOCALE_PATHS contain Plural-Forms while those found in INSTALLED_APPS don't.
To test this behaviour I created a test project on github here:
https://github.com/armisael/test-django-i18n-preference
The project contains a .po file with Plural-Forms and a single translation that is also included in one of the dependencies. A single test checks which translation is picked.
With the current configuration, the test fails by using the dependency translation. It passes if:
1) the dependency is removed from INSTALLED_APPS; or if
2) Plural-Forms is removed from the .po file.
1) proves that i18n is correctly configured, and that the dependency translation is picked over the project one; 2) proves that Plural-Forms is involved in the bug.
Change History (14)
comment:1 by , 3 years ago
| Cc: | added |
|---|
comment:2 by , 3 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 3 years ago
Hi, I'm stuck at writing test for this one, how do i include dependency in the test?
comment:4 by , 3 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 3 years ago
hey Claude, I tested the patch with the test project that Stefano has provided but it did not work.
comment:7 by , 18 months ago
| Cc: | added |
|---|
comment:8 by , 17 months ago
| Cc: | added |
|---|
comment:10 by , 14 months ago
We also came across this, which caused a bug in Translation https://code.djangoproject.com/ticket/35760 , as diagnosed by Sarah Boyce in this comment. Looking forward to a workaround-free fix 👍
comment:11 by , 14 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:13 by , 14 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Thanks for the detailed report and test project!
To solve #30439 and allow for different plural forms for a same language, we had to refrain from merging all translations in a unique catalog, as was done before. However, your setup demonstrates that the algorithm to decide between merge or separating catalogs is flawed (the third catalog from LOCALE_PATH having the same plural forms as the Django one, it is merge with it, hence loosing it's priority over the intermediary django-extension catalog).
The following patch should fix the issue:
diff --git a/django/utils/translation/trans_real.py b/django/utils/translation/trans_real.py index c1e64d4ebd..f4d05b8f76 100644 --- a/django/utils/translation/trans_real.py +++ b/django/utils/translation/trans_real.py @@ -96,11 +96,9 @@ class TranslationCatalog: yield from cat.keys() def update(self, trans): - # Merge if plural function is the same, else prepend. - for cat, plural in zip(self._catalogs, self._plurals): - if trans.plural.__code__ == plural.__code__: - cat.update(trans._catalog) - break + # Merge if plural function is the same as the top catalog, else prepend. + if trans.plural.__code__ == self._plurals[0]: + self._catalogs[0].update(trans._catalog) else: self._catalogs.insert(0, trans._catalog.copy()) self._plurals.insert(0, trans.plural)