Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#13514 closed (fixed)

Only last package javascript catalog is loaded in case of missing default translation.

Reported by: jtiai Owned by: nobody
Component: Internationalization Version: master
Severity: Keywords: i18n javascript
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I had accidentally defined LANGUAGE_CODE = 'en_us' . Browser requested finnish translation (fi). Which we do have. We do not have en translations at all.

Probably due changes in r13069 now it tried to discard earlier loaded translations and load only finnish translation. View only loaded catalog from last module defined in packages list.

Workaround: Define LANGUAGE_CODE = 'fi' and everything worked as expected.

See also tickets #3594 and #13388.

Attachments (3)

13514-regression-test.diff (12.2 KB) - added by ramiro 5 years ago.
Patch with a regression test for this ticket. If/when applying don't forget to re-generate the .mo files
13514-regression-test.2.diff (14.9 KB) - added by ramiro 5 years ago.
Fixed the regression test (use javascript_quote() in the assertContain() call, fixed typo in app2's fr djangojs.po)
13514-regression-test.3.diff (19.5 KB) - added by ramiro 5 years ago.
Added a test to verify things work when both request and default locales are different from English

Download all attachments as: .zip

Change History (14)

comment:1 follow-ups: Changed 5 years ago by arkx

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Hello, I'm the one who provided the patch to fix #3594. What I'd like to see is a failing test case where you demonstrate the problem you are facing. You can use the tests introduced in #3594 as an example: http://code.djangoproject.com/browser/django/trunk/tests/regressiontests/views/tests/i18n.py. If this seems daunting you could also upload a test project that demonstrates the issue so that we could take a look.

Did I undestand correctly that you wanted a Finnish translation to appear and it did not, displaying the default (as specified by LANGUAGE_CODE setting) English instead? Which language(s) are you translating to and which from? Are you having trouble with normal translations or JavaScript translations? Please note that both of the tickets you refer to are related to JavaScript translations only.

For reference, some background: because English is often the language people translate from (for example in Django's official translations), there is often no reason to create English translation files (just as you note, you don't have them at all). In this case, when English is the selected language the system will try to look for English translation and when it doesn't find one, it will return the untranslated string, which happens to be in English.

The problem was that JavaScript translation catalog would include the LANGUAGE_CODE translation if it's something other than English. This would cause trouble in the scenario where you are translating from English to Finnish for example, and your LANGUAGE_CODE is set to Finnish. The translation catalog is set to Finnish because of LANGUAGE_CODE, but then when the system subsequently tries to find translation for the currently selected language (English), it will not find anything, because English is the language being translated from. And hence Finnish translation is served to a user who requested English translation despite the fact that English version exists.

I fixed this by setting a flag if English translation files could not be found during the catalog's construction and checking for that flag when at the last step the system is looking for active language's translation files and the language happens to be English. Then and only then is the catalog that had been set to LANGUAGE_CODE in previous step discarded. Note that nothing is discarded unless the English translation files are missing and English is the currently activated language choice, which should not happen if a browser requests the Finnish translation.

Take a look at the function javascript_catalog at http://code.djangoproject.com/browser/django/trunk/django/views/i18n.py if you want to see exactly how the JavaScript translation catalog is constructed.

comment:2 Changed 5 years ago by arkx

Note that LANGUAGE_CODE should be set according to standard language format: http://docs.djangoproject.com/en/dev/topics/i18n/#term-language-code. You had set it to a locale name. You might also want to check out how Django discovers language preferences for more info.

comment:3 in reply to: ↑ 1 Changed 5 years ago by jtiai

  • Summary changed from Only last package catalog is loaded in case of missing default translation. to Only last package javascript catalog is loaded in case of missing default translation.

Replying to arkx:

Hello, I'm the one who provided the patch to fix #3594. What I'd like to see is a failing test case where you demonstrate the problem you are facing. You can use the tests introduced in #3594 as an example: http://code.djangoproject.com/browser/django/trunk/tests/regressiontests/views/tests/i18n.py. If this seems daunting you could also upload a test project that demonstrates the issue so that we could take a look.

Did I undestand correctly that you wanted a Finnish translation to appear and it did not, displaying the default (as specified by LANGUAGE_CODE setting) English instead? Which language(s) are you translating to and which from? Are you having trouble with normal translations or JavaScript translations? Please note that both of the tickets you refer to are related to JavaScript translations only.

I'm using javascript translation (should have been more precise there).

I was having incorrect default (en-us) translation and I tried to have Finnish translations appear. I've multiple catalogs (around 20 apps with their own catalogs, djangojs - to be precise).

When browser requested Finnish translations only last application package listed in packages list was loaded for some reason.

Excerpt of urls.py:

js_info_dict = {
    'packages': ('app1', 'app2', 'app3', 'app4',),
}

urlpatterns = patterns('',
    (r'^jsi18n/$', 'django.views.i18n.javascript_catalog', js_info_dict),
)

Now in my case I don't have english translations, I had default language defined as "en-us", and browser requested "fi" translation.

I got only translations from last application package in example case app4.

For reference, some background: because English is often the language people translate from (for example in Django's official translations), there is often no reason to create English translation files (just as you note, you don't have them at all). In this case, when English is the selected language the system will try to look for English translation and when it doesn't find one, it will return the untranslated string, which happens to be in English.

The problem was that JavaScript translation catalog would include the LANGUAGE_CODE translation if it's something other than English. This would cause trouble in the scenario where you are translating from English to Finnish for example, and your LANGUAGE_CODE is set to Finnish. The translation catalog is set to Finnish because of LANGUAGE_CODE, but then when the system subsequently tries to find translation for the currently selected language (English), it will not find anything, because English is the language being translated from. And hence Finnish translation is served to a user who requested English translation despite the fact that English version exists.

I fixed this by setting a flag if English translation files could not be found during the catalog's construction and checking for that flag when at the last step the system is looking for active language's translation files and the language happens to be English. Then and only then is the catalog that had been set to LANGUAGE_CODE in previous step discarded. Note that nothing is discarded unless the English translation files are missing and English is the currently activated language choice, which should not happen if a browser requests the Finnish translation.

In my case it exactly happened like that. I had Finnish as a active language (verified from a debugger) and all else but last catalog defined in my packages list was discarded. Last loaded catalog was in correct language though (Finnish).

Take a look at the function javascript_catalog at http://code.djangoproject.com/browser/django/trunk/django/views/i18n.py if you want to see exactly how the JavaScript translation catalog is constructed.

I did and it seems that last change made in that file seems to override everything with last catalog ever loaded from packages list. (previously there was t.update, now there is just assignment)

Maybe this only happens when you have multiple catalogs that should be joined together instead of just loading one catalog? At least in test case provided I didn't found anything related to multiple catalogs.

System also works in my case if rolling back views.i18n two changesets (before changes made).

comment:4 Changed 5 years ago by anonymous

I spoke with jezdez about this and he told me he would have a look at it, if time permits. The issue you describe seems like a regression caused by the fix in [13068]. I do not quite understand what the issue in #13388 was, but I suggest reverting the change for now and looking for an alternative way to fix it if necessary.

comment:5 Changed 5 years ago by ramiro

The 13514-regression-test.diff patch attached adds a regression test (run it with runtests.py views.JsI18NTestsMultiPackage) implementing the scenario jtiai reports.

Some notes:

  • It also moves some pre-existing javascript_catalog I18N view tests from tests/regressiontests/admin_views/tests.py to tests/regressiontests/views/tests/i18n.py because that view isn't admin- specific and to have all of them in one place. This means the tests/regressiontests/views/locale/*/LC_MESSAGES/djangojs.po|mo files were updated.
  • It adds two app1 and app2 dirs under tests/regressiontests/views with their own French djangojs.po files
  • Don't forget to re-generate djangojs.mo files from all the djangojs.po involved.

Changed 5 years ago by ramiro

Patch with a regression test for this ticket. If/when applying don't forget to re-generate the .mo files

comment:6 Changed 5 years ago by kmtracey

  • milestone set to 1.2

This needs to be resolved before we can ship 1.2, right?

comment:7 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Changed 5 years ago by ramiro

Fixed the regression test (use javascript_quote() in the assertContain() call, fixed typo in app2's fr djangojs.po)

Changed 5 years ago by ramiro

Added a test to verify things work when both request and default locales are different from English

comment:8 in reply to: ↑ 1 Changed 5 years ago by ramiro

Replying to arkx:

I fixed this by setting a flag if English translation files could not be found during the catalog's construction and checking for that flag when at the last step the system is looking for active language's translation files and the language happens to be English. Then and only then is the catalog that had been set to LANGUAGE_CODE in previous step discarded. Note that nothing is discarded unless the English translation files are missing and English is the currently activated language choice, which should not happen if a browser requests the Finnish translation.

So far I've found that undoing the change to django/views/i18n.py:javascript_catalog() from r13069 makes the tests pass as someone suggested above, both the ones added to this ticket and even the ones added in that same commit. The full suite run successfully too.

arkx:

What I'd like to understand and make sure we are handling correctly is that en_catalog_missing flag. Currently we are setting it to True when at least one of the Python packages contained in the packages parameter has no English .mo file. This means the LANGUAGE_CODE translations will be discarded in such case. Is this correct?

comment:9 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [13250]) Fixed #13514 -- Corrected the process of loading multiple javascript translation catalogs. Thanks to jtiai for the report, to Ramiro Morales for working out the test case, and to Ramiro and Jannis for their help on the fix.

comment:10 Changed 5 years ago by russellm

(In [13253]) [1.1.X] Fixed #13514 -- Corrected the process of loading multiple javascript translation catalogs. Thanks to jtiai for the report, to Ramiro Morales for working out the test case, and to Ramiro and Jannis for their help on the fix.

Backport of r13250 from trunk.

comment:11 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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