[Django] #29144: `DjangoTranslation.merge` loses catalog fallbacks
Django
-------------------------------------+-------------------------------------
Reporter: Patryk | Owner: Patryk Zawadzki
Zawadzki |
Type: Bug | Status: assigned
Component: | Version: 2.0
Internationalization |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29144>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Zawadzki
Type: Bug | Status: assigned
Component: | Version: 2.0
Internationalization |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
Proposed PR: https://github.com/django/django/pull/9709
--
Ticket URL: <https://code.djangoproject.com/ticket/29144#comment:1>
Django
-------------------------------------+-------------------------------------
| Zawadzki
Type: Bug | Status: assigned
Component: | Version: 2.0
Internationalization |
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matt Westcott):
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 :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/29144#comment:2>
Django
-------------------------------------+-------------------------------------
| Zawadzki
Type: Bug | Status: assigned
Component: | Version: 2.0
Internationalization |
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Patryk Zawadzki):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29144#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Patryk Zawadzki | Owner: Patryk
| Zawadzki
Type: Bug | Status: assigned
Component: | Version: 2.0
Internationalization |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29144#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Patryk Zawadzki | Owner: Patryk
| Zawadzki
Type: Bug | Status: assigned
Component: | Version: 2.0
Internationalization |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
The patch looks good to me. I'm going to mark it RFC for a final review.
Thanks Patryk!
--
Ticket URL: <https://code.djangoproject.com/ticket/29144#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Patryk Zawadzki | Owner: Patryk
| Zawadzki
Component: | Version: 2.0
Internationalization |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a20aae414e762e4d9043e76b0bf8eccd334c8ebc" a20aae4]:
{{{
#!CommitTicketReference repository=""
revision="a20aae414e762e4d9043e76b0bf8eccd334c8ebc"
Fixed #29144 -- Made untranslated strings for territorial language
variants use translations from the generic language variant.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29144#comment:6>