Blocktrans should tolerate bad locale data

[Simon Litchfield]

Currently, blocktrans crashes entire pages when it tries to merge strings that aren't properly formatted in the locale data files.

For example, if %(person)s is the original, the spanish translated version might erroneously include %(persona)s. If that's being blocktrans'd in your base template, it'll crash your whole spanish site.

Translators work separately to template writers, and are generally less technical. Mistakes happen. They are minor and shouldn't bring sites down. Especially with tools like django-rosetta in their hands.

IMHO, the default behaviour here should be to ignore (or perhaps throw a warning) on improperly formatted translation strings.

[Aymeric Augustin]

Django goes to great lengths to ensure template authors can't break a page, because they don't have the same skill set as developers.

This argument applies even more to translators: HTML authors know what code is, translators using POEdit don't.

For this reason, I support the suggestion.

I'm not sure it's an easy picking — please write a patch and prove me wrong :)

[Claude Paroz]

Fix KeyError, with tests

[Aymeric Augustin]

I think the proper solution is to output settings.TEMPLATE_STRING_IF_INVALID when the translation string is invalid.

I'm against hardcoding a fallback to 'en':

• if someone builds a website in Greek and the English translation is wrong, this doesn't fix the error;
• if someone builds a website in Turkish and the German translation is wrong, I don't know what happens, but if it works it's a hack :)

Currently, the 'en' locale is special-cased in only one place: django.views.i18n.javascript_catalog. This has bugged me for a long time, the implementation is incredibly convoluted, and I still can't figure out why it's necessary. Apparently, you were involved in #14924, so you probably know this piece of code better than I do :)

[Claude Paroz]

I understand your concern about using the 'en' translation. But I'm opposed to use the TEMPLATE_STRING_IF_INVALID setting. If a translation has an error, the logical fallback is to use the original untranslated string, not the empty string (which is default for TEMPLATE_STRING_IF_INVALID).

So I will try to improve the patch by replacing the 'en' hardcoded value by a NullTranslations instance (which corresponds to the original string, whatever the original language is).

[Claude Paroz]

Same patch without hardcoding 'en' fallback

[Aymeric Augustin]

To apply the patch, I first removed the chunk about the binary .mo file, and then I ran: msgfmt -o tests/regressiontests/i18n/other/locale/fr/LC_MESSAGES/django.mo tests/regressiontests/i18n/other/locale/fr/LC_MESSAGES/django.po

The fix and the tests for the original issue are OK. You're right, TEMPLATE_STRING_IF_INVALID isn't a good idea here.

Regarding the changes to translation.override, I don't understand why you added the deactivate; it isn't used anywhere by this patch; is it a fix for a separate issue?

[Claude Paroz]

I added the deactivate_all call to allow for translation.override to set no translation at all. While I was completing the docs, I also realized that the *existing* deactivate parameter was undocumented. This last bit could have been part of a separate issue.

[Aymeric Augustin]

Oh right, you modified the signature in the docs, not in the code. How did I manage to mis-read that? :/

[Jannis Leidel]

Looks good.

[Jannis Leidel]

In [16723]:

Fixed #16516 -- Relaxed the blocktrans rendering a little by falling back to the default language if resolving one of the arguments fails, raising a KeyError. Thanks, Claude Paroz and Aymeric Augustin.