jsi18n - get_javascript_catalog obscure and hardcoded english

[Cristiano Coelho]

This is a ticket to get some support in removing the actual hardcoded english fallback (even if default language is not english) in get_javascript_catalog.

Discussion here: ​https://groups.google.com/forum/#!topic/django-developers/gDU8HlZg4Ug

But basically, the current jsi18n will always load english as a fallback language, which will be then overriten by what ever your default language is, but may cause some issues if you don't want to add message strings to your default language (because ids == values) and you end up with english translations.

The proposed change is to remove this english loading completely, and only load the default language as fallback, which will then match the server side behaviour for translations where the above is not an issue.

[Tim Graham]

Please submit a tested pull requested. I removed the code from the patch's description since I don't think anyone will review it there (not to mention, we cannot easily tell what changed).

I'm not sure what you're trying to say by describing the function as "obscure" in the ticket summary. Could you try to rephrase it to describe the desired changes?

Finally, if English fallbacks aren't loaded anymore, couldn't this be backwards-incompatible for some user? (keep in mind, I don't have any experience with this code so maybe there's no problem)

[Tim Graham]

A related ticket is #26319 -- can you say if your changes would also solve that issue? Maybe you can dialog with that ticket author a bit.

[Cristiano Coelho]

Sorry, first ticket, I guess I lost a lot of information here.

With 'obscure' I tried to say that the code is hard to understand, at least for me it is not very obvious why it loads english as default or discards translations if it doesn't find english translations.

About backwards compatibility, there will be a difference in what happens if a translation for the requested language is not found, and the default language is not 'en'. Right now if a translation string is not found in the requested language it will fallback to the default language if it has a value defined and if it is not found it will fallback to 'en'. The new behaviour would not fallback to 'en' but rather to the message id, just like the server side version does (although the server side version also has some oddly hardcoded english to not load any fallback if the requested language is english)

I can't tell for sure if it will fix #26319 since it is not obvious if he is having an issue due to the english special treatment or due to configuration issues. However with the removed english treatment I can't see any reason of why specific english locales wouldn't be loaded.

EDIT: I'm failing to create a pull request, it seems that I have no permission :'(

[Cristiano Coelho]

Work done here: ​https://github.com/cristianocca/django/commit/c6ec94864210406c95ffe5c9a9552c83137e7e1e

Is this the right way to proceed? Should I make this a pull request after?

[Tim Graham]

If it passes the unit tests sure. Can you add a test for the new behavior that's enabled by the change?

[Cristiano Coelho]

I can not seem to run the whole i18n tests completely, for some reason I'm getting many errors related to the makemessages command and msgmerge (see example below) with and without my changes. Oddly when using the real 1.9.4 installation I don't get these errors (it still fails with 2 fails and 2 errors).
I'm running windows 8, python2 and a virtual enviorment with all requirements.txt installed.

More in depth results of i18n tests:
i18n.tests --> ok
i18n.test_compilation --> 1 error
i18n.test_extraction --> 26 errors
i18n.test_percents --> ok

I can not find any javascript related test in those. I did however find a very good ticket that explains why the special english treatment for translations ( https://code.djangoproject.com/ticket/24413 ).
So the change would actually bring back that issue in order to fix the other one.

The real issue is not english, but is actually what happens when you have translations with no values where you expect to get the message id back but the default language is different from the one with no values, which will happen very often when mixing apps (like the admin app)
Example...

• default language = 'es'
• My project has 'es' and 'en' translation files, but I'm lazy and I don't add any translation values for the 'es' ones because it matches the ids and it is the same as my default language. So if I request an 'es' translation, since requested == default, no fallback is added and I always get the expected output.
• Add a new app, which is implemented in english so the developers decided to have all english translation files with no values, and when they tested it all worked fine because their default language was english as well. Now when I use this new app in my project, since my default is 'es', all requests in english will have 'es' as fallback so if any of those 'es' translations have a value, I will end up with spanish rather than english, which is not expected. This is where the team "fixed" the issue by manually checking for english, and so everything worked fine for all django apps, but the issue won't be fixed for other languages rather than english and even worse bring up new issues due to english having a special treatment.

What can we do? I think the only real way to prevent this is to make sure all translations are always defined rather than relying on message ids and remove the special english treatment (this ticket is to remove it from javascript translations, but server side there's check for english).
Any other ideas? Should be there an app specific setting that will tell the translation machinery which translations files are missing its values by purpose because message ids are enough? That way we could actually load values for those files either at compile time or load time. This would allow us to remove any english special treatment while still having all languages load fine for those that are lazy and do not define translation values, but I have no idea how could this be done.

sample error from tests

ERROR: test_no_wrap_enabled (i18n.test_extraction.NoWrapExtractorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Cristiano\Desktop\django_git\django\tests\i18n\test_extraction.py", line 605, in test_no_wrap_enabled
    management.call_command('makemessages', locale=[LOCALE], verbosity=0, no_wrap=True)
  File "C:\Users\Cristiano\Desktop\django_git\venv\lib\site-packages\django\core\management\__init__.py", line 117, in call_command
    return command.execute(*args, **defaults)
  File "C:\Users\Cristiano\Desktop\django_git\venv\lib\site-packages\django\core\management\base.py", line 348, in execute
    output = self.handle(*args, **options)
  File "C:\Users\Cristiano\Desktop\django_git\venv\lib\site-packages\django\core\management\commands\makemessages.py", line 307, in handle
    self.write_po_file(potfile, locale)
  File "C:\Users\Cristiano\Desktop\django_git\venv\lib\site-packages\django\core\management\commands\makemessages.py", line 541, in write_po_file
    "errors happened while running msgmerge\n%s" % errors)
CommandError: errors happened while running msgmerge
C:\Users\Cristiano\Desktop\django_git\django\tests\i18n\commands\locale\de\LC_ME
SSAGES\django.po:2:47: syntax error
msgmerge: se encontr\xf3 el error fatal 1

[Tim Graham]

The Windows failures starting in https://code.djangoproject.com/ticket/25677#comment:10. We need help there as we don't have any Windows expertise besides Ramiro who hasn't had a chance to look at the issue. If you can't solve the issue, just ignore the failures. The test for javascript_catalog seem to be in view_tests.

I'm adding Claude to CC -- maybe he can advise how to proceed with this ticket.

[Cristiano Coelho]

As expected, tests are failing, there are 3 particular tests that tries english translations with a default different than english.

If there's an easy way to mark a translation file as "values == ids" so then it is loaded that way for those files, we could get rid of all the english special treatments while keeping the current behaviour. However that seems like it would not be backwards compatible since for example all the english empty translation files of django should be marked like so.

[Cristiano Coelho]

I would like to add, the test calls my attention that it almost seems like translations are supposed to always have the messages ids in english, look at this:

def test_jsi18n_with_missing_en_files(self):
        """
        The javascript_catalog shouldn't load the fallback language in the
        case that the current selected language is actually the one translated
        from, and hence missing translation files completely.

        This happens easily when you're translating from English to other
        languages and you've set settings.LANGUAGE_CODE to some other language
        than English.
        """
        with self.settings(LANGUAGE_CODE='es'), override('en-us'):
            response = self.client.get('/jsi18n/')
            self.assertNotContains(response, 'esto tiene que ser traducido')

If I read it correctly, it makes the default language 'es' and tries to get an english translation, and the expected result is to get back an english translation EVEN if the english locale is not even defined (look at the languages inside the locale folder of the tests). So basically the expected behaviour is to get a translation for a language that is not even defined and make the test pass by giving english special treatment in the framework.

"The javascript_catalog shouldn't load the fallback language in the case that the current selected language is actually the one translated from, and hence missing translation files completely." --> The logic behind this makes sense, but the actual implementation is completely different (only favoring english) and not keeping in mind that you may have multiple apps with different "base" language so you can't relly on a single setting

[Claude Paroz]

I admit that the javascript i18n definitely needs work. I've tried working on some refactoring in ​this PR. I'm not sure it addresses all concerns of this ticket, but it should be a start. Reviewers welcome.

[Cristiano Coelho]

It looks better with the changes, but it seems that english is still having a special treatment. Is there something you can do in this PR to also remove that?

Do you think you will have these changes done before 1.10? What are you missing?

Also, once your PR is accepted I believe it will also fix the main issue that started this ticket so it can be closed after that, and there should be a new ticket for the issues related to having special treatment to english.

[Claude Paroz]

I just pushed a revision of the PR. And yes, if the reviews are positive, it should be committed soon.

[Claude Paroz <claude@…>]

In 11c60b5:

Reused the DjangoTranslation class for the javascript_catalog view

Thanks Tim Graham and Cristiano Coelho for the reviews.
Refs #26328, #26319.