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 Patryk Zawadzki, 7 years ago

Has patch: set

comment:2 by Matt Westcott, 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 Patryk Zawadzki, 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 Tim Graham, 7 years ago

Summary: `DjangoTranslation.merge` loses catalog fallbacksDjangoTranslation.merge() loses catalog fallbacks
Triage Stage: UnreviewedAccepted

comment:5 by Carlton Gibson, 7 years ago

Triage Stage: AcceptedReady for checkin

The patch looks good to me. I'm going to mark it RFC for a final review. Thanks Patryk!

comment:6 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In a20aae4:

Fixed #29144 -- Made untranslated strings for territorial language variants use translations from the generic language variant.

Note: See TracTickets for help on using tickets.
Back to Top