Opened 7 years ago
Closed 7 years ago
#29144 closed Bug (fixed)
DjangoTranslation.merge() loses catalog fallbacks
Reported by: | Patryk Zawadzki | Owned by: | Patryk Zawadzki |
---|---|---|---|
Component: | Internationalization | Version: | 2.0 |
Severity: | Normal | Keywords: | |
Cc: | 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
gettext
will properly resolve fallbacks for locales. For example asking for de-de
will result in the following chain of fallbacks: ['de_DE.ISO8859-1', 'de_DE', 'de.ISO8859-1', 'de']
. These are resolved to GNUTranslations
instances where a translation exists and the instances are turned into a linked list with a single head and a chain of fallbacks.
Django then starts with an empty GNUTranslations
subclass (DjangoTranslation
) instance and then merges the GNUTranslations
instance returned by gettext
into itself. Only in the process it forgets to carry over any existing fallbacks so the entire tail of the linked list is lost.
The observable bug is that if a generic translation exists for a language (say pt
) and a sparse territorial variant (say pt-br
) only provides overrides for certain strings, Django will never fall back to the generic translation, instead filling the missing translations from the default locale.
Change History (6)
comment:1 by , 7 years ago
Has patch: | set |
---|
comment:2 by , 7 years ago
Very happy to see this being addressed! (We were bitten by this on Wagtail... https://github.com/wagtail/wagtail/issues/3600 )
When I looked into this some time ago, I think I stumbled at the point of making Django's precedence rules behave correctly with gettext's: to be totally correct, I believe it should apply all of Django's translation-finding rules (LOCALE_PATHS in sequence, then INSTALLED_APPS in sequence, then Django's own) for the most specific locale (de_DE.ISO8859-1
), followed by the same sequence of rules for the next locale (de_DE
), and so on.
In the case of the current PR, I suspect that there are some possible setups where this ordering isn't strictly followed, but as far as I can see, this only arises when there are three or more locale identifiers in play. (The primary catalog is built up with a dict.update
, as per Django's existing code, but the fallbacks are added to the end of the chain - as per the gettext implementation of add_fallback
. So, after it's finished pulling in all the translations, the final order of precedence will be all of the de_DE.ISO8859-1
translations, followed by de_DE
and de.ISO8859-1
and de
for each app - rather than all of the de_DE.ISO8859-1
translations, followed by all of the de_DE
translations, followed by all of the de.ISO8859-1
translations...). Obviously, this is an incredibly obscure and minor point relative to the issue being fixed here (and doing it 100% correctly probably requires replicating or monkeypatching gettext internals), so I'm happy for this to be accepted as-is. I'm only raising this as a gotcha in case a more alert reviewer notices a more severe side-effect :-)
comment:3 by , 7 years ago
Matt, I gave it some thoughts before settling with the simplest solution.
I think the current behavior is the expected one in most real-life cases. You have a base German translation in one app and override parts of it in a project-wide translation. If the app provides Austrian German overrides for strings that your project-level German translation overrides then they are likely no longer relevant and would end up misleading so I believe it's fine to expect you to have to override them again in a project-level Austrian German translation. Going the other way you have to deal with all sorts of fallback corner cases like what to do when the app provides full de-de
and de-at
but no de
while the project provides an incomplete de
translation with sparse de-at
overrides.
comment:4 by , 7 years ago
Summary: | `DjangoTranslation.merge` loses catalog fallbacks → DjangoTranslation.merge() loses catalog fallbacks |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:5 by , 7 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
The patch looks good to me. I'm going to mark it RFC for a final review. Thanks Patryk!
Proposed PR: https://github.com/django/django/pull/9709