Opened 8 years ago

Closed 5 years ago

Last modified 4 years ago

#3594 closed (fixed)

Translation problem in javascript

Reported by: karsu Owned by: jezdez
Component: Internationalization Version: master
Severity: Keywords:
Cc: jezdez, robin Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I have changed default language code in settings file to my own language. If I change language with "/i18n/setlang/" url everything works fine except javascript translation. Javascript translation problem exist only with english language. If i try to change language to english all javascript words is translated to default language (what is specified in settings file).

Attachments (10)

javascript_translation.patch (1.1 KB) - added by Timo 8 years ago.
Finally I fixed this for my own use.
js_i18n_test.zip (6.5 KB) - added by yishaibeeri 7 years ago.
minimal project to reproduce bug in admin
javascript_translation_r8521.patch (1.1 KB) - added by msaelices 7 years ago.
Patch updated to last version, r8521.
js_i18n_test.tgz (7.7 KB) - added by msaelices 7 years ago.
Updated the test project, to new admin app.
javascript_translation_with_test_r8521.patch (3.6 KB) - added by msaelices 7 years ago.
Patch updated with a regression test
jsi18n.patch (553 bytes) - added by angelolaub@… 6 years ago.
Fixed bug in the locale catalog assignment routine that caused it to always select the fallback language (misuse of dict.update())
javascript_translation_catalog.patch (1.7 KB) - added by arkx 5 years ago.
Fixed a case where translating from English but setting settings.LANGUAGE_CODE to something else would always choose that translation without the (unnecessary) English translation files.
jsi18n_11620.patch (4.8 KB) - added by msaelices 5 years ago.
Uploaded a working patch with english and not english fallback language and with regression test included. Works with [11620] revision
javascript_translation_test_case.patch (3.1 KB) - added by arkx 5 years ago.
Updated attached test case to assert that fallback languages other than English still work
3594-r12362.diff (5.6 KB) - added by ramiro 5 years ago.
Patch by arkx unified and updated to r12362, with fixes to tests and an optimization to the jsi18n view code

Download all attachments as: .zip

Change History (41)

comment:1 Changed 8 years ago by Gary Wilson <gary.wilson@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 8 years ago by mtredinnick

Needs confirmation that it's happening (and ideally even a small test app). Help requested in this django-i18n thread.

Changed 8 years ago by Timo

Finally I fixed this for my own use.

comment:3 Changed 8 years ago by anonymous

  • Has patch set
  • Needs tests set

comment:4 Changed 8 years ago by anonymous

  • Needs tests unset
  • Patch needs improvement set

comment:5 Changed 8 years ago by anonymous

  • Needs tests set
  • Patch needs improvement unset

comment:6 Changed 8 years ago by mtredinnick

In case anybody is looking at this, I am not happy with the patch here so far. It isn't correct to remove the fallback such as is being done there. We should be doing "preferred locale", then "default locale" (as set in settings) and then "en". So default_locale has to be mentioned somewhere, I would have thought.

comment:7 Changed 8 years ago by msaelices

  • Owner changed from nobody to msaelices
  • Status changed from new to assigned

I'll try to fixed that and add tests

comment:8 Changed 8 years ago by msaelices

  • Needs tests unset

I added test, but is attached in other ticket #5496. The reason is it was needed a new directory for testing django views.

comment:9 Changed 8 years ago by msaelices

  • Triage Stage changed from Accepted to Ready for checkin

In changeset [6370] there are many tests, including tests for [source:django/trunk/tests/regressiontests/views/tests/i18n.py jsi18n views]. I think this ticket is now ready for commit.

comment:10 Changed 8 years ago by russellm

  • Needs tests set
  • Triage Stage changed from Ready for checkin to Accepted

It's not ready for checkin until there is a test. The current tests pass; either we are missing a test, or the change isn't required.

comment:11 follow-up: Changed 7 years ago by msaelices

  • Resolution set to worksforme
  • Status changed from assigned to closed

There are no needs of this patch. It's work for me. I created exactly the same testing environment ticket author.

I do:

  • Created a project with settings.LANGUAGE_CODE to spanish 'es-es'
  • A /media/js/translate.js with a call to gettext.
  • A base.html template with include django jsi18n views and translate.js js file. Also contains a language selector.
  • Several locale/xx/LC_MESSAGES/djangojs.po with spanish, english and francais translations.

Default language is spanish, and first time I view web, all is in spanish. Then, if I change to english, all javascripts msgid are translated successfully to English.

I close this ticket because it works for me. Afterall, there is some tests of javascript translations that checks the generated javascript catalog.

Changed 7 years ago by yishaibeeri

minimal project to reproduce bug in admin

comment:12 follow-up: Changed 7 years ago by yishaibeeri

  • Resolution worksforme deleted
  • Status changed from closed to reopened

This still does not work - I've attached a minimal project that reproduces this on rev 7706.

  • Use the simple page at '/' to switch the current langauge
  • Look at the admin page for the test model - the date field has a couple of visible javascript (translatable) string, such as 'today' and 'now'.
  • If the default language in settings.py is English, all is well
  • If the default language is different, and you set the current language to English, the admin shows default language js strings instead of English ones.

The same behavior happens in custom app javascript translations, not just the admin.

Note that if you add translations to the django/conf/locale/en/LC_MESSAGE/djangojs.po file (e.g., put values in the msg_str parts, which are usually left empty for the en locale), this problem goes away.

comment:13 in reply to: ↑ 12 Changed 7 years ago by telenieko

Replying to yishaibeeri:

This still does not work - I've attached a minimal project that reproduces this on rev 7706.

Try to write a new test in tests/ that reproduces the bug. It's far easier to reproduce things and to check if patches solve it.

Changed 7 years ago by msaelices

Patch updated to last version, r8521.

Changed 7 years ago by msaelices

Updated the test project, to new admin app.

Changed 7 years ago by msaelices

Patch updated with a regression test

comment:14 Changed 7 years ago by msaelices

  • Needs tests unset

Changed 6 years ago by angelolaub@…

Fixed bug in the locale catalog assignment routine that caused it to always select the fallback language (misuse of dict.update())

comment:15 in reply to: ↑ 11 Changed 5 years ago by arkx

  • Patch needs improvement set

I can still reproduce this bug with Django version 1.0.2 final. My settings.LANGUAGE_CODE is set to fi, and everything works well when I'm using Finnish. However, when I switch to English, only translations in the django domain work: Javascript translations stop working due to a bug in javascript_catalog in django/views/i18n.py.

Replying to msaelices:

There are no needs of this patch. It's work for me. I created exactly the same testing environment ticket author.

I think this is the core of the issue. This works for you because you have created English translation files. I'm translating from English to Finnish, thus no English translation files exist. I suspect the original reporter and the people who created the earlier patches work in the same way: strings in templates and code are in English and all other languages are created through makemessages. This way is actually endorsed by Django's i18n documentation.

This is where the function javascript_catalog in views/i18n.py gets it wrong. In my case default_locale is 'fi' and 'locale' is 'en-us' in the following excerpts.

    # first load all english languages files for defaults
    for package in packages:
        p = importlib.import_module(package)
        path = os.path.join(os.path.dirname(p.__file__), 'locale')
        paths.append(path)
        try:
            catalog = gettext_module.translation(domain, path, ['en'])
            t.update(catalog._catalog)
        except IOError:
            # 'en' catalog was missing. This is harmless.
            pass

This part of the code tries to load English language files, which do not exist. It will raise the IOError and it will be passed on.

    # next load the settings.LANGUAGE_CODE translations if it isn't english
    if default_locale != 'en':
        for path in paths:
            try:
                catalog = gettext_module.translation(domain, path, [default_locale])
            except IOError:
                catalog = None
            if catalog is not None:
                #t.update(catalog._catalog)
                t = catalog._catalog

Now, because my settings.LANGUAGE_CODE and hence default_locale is set to 'fi', django will fetch and use the Finnish translation. At this point the Javascript catalog will hold Finnish translations.

    # last load the currently selected language, if it isn't identical to the default.
    if locale != default_locale:
        for path in paths:
            try:
                catalog = gettext_module.translation(domain, path, [locale])
            except IOError:
                catalog = None
            if catalog is not None:
                t.update(catalog._catalog)

This is where the code fails for me, as I still don't have any English translation files -- the code will fail to overwrite the Finnish translation files it fetched earlier.

There is a simple workaround for this bug if you can't wait for it to be fixed: create a translation file in djangojs domain for English language and fill in the blanks with the original strings. It's tiresome and feels unnecessary but it works.

As a side note, angelolaub's patch doesn't fix the issue for me. The earlier patches do (they remove the second part of the function), but they cause breakage in situations where users want the fallback language to be other than English.

Changed 5 years ago by arkx

Fixed a case where translating from English but setting settings.LANGUAGE_CODE to something else would always choose that translation without the (unnecessary) English translation files.

comment:16 Changed 5 years ago by arkx

  • Patch needs improvement unset

I've attempted to create a patch that fixes the issue without causing breakage in cases where people want the fallback language to be other than English.

comment:17 Changed 5 years ago by arkx

  • Owner changed from msaelices to arkx
  • Status changed from reopened to new

comment:18 Changed 5 years ago by jezdez

  • Cc jezdez added
  • Needs tests set
  • Patch needs improvement set

Checking for english

comment:19 Changed 5 years ago by jezdez

Oops,
checking for english as the originating language doesn't seem cover all cases, if I understand correctly. Please provide tests to be sure this will work.

comment:20 Changed 5 years ago by arkx

A note about the added test case: I removed previously existing locale/en files to show what's the problem. The previous test which used to use them did so for no good reason.

Current Django documentation does not encourage users to make en -> en translation files anyway, as they are unnecessary (unless you're trying to work around this very bug by providing no-op translation files).

Changed 5 years ago by msaelices

Uploaded a working patch with english and not english fallback language and with regression test included. Works with [11620] revision

comment:21 Changed 5 years ago by arkx

msaelices, that most certainly fixes the current problem by removing that part of the catalog forming algorithm completely, but if I understand your patch correctly, in turn we lose the ability to let users select their fallback language. I believe it is very important that we don't break that, as it would be strange if only Javascript translations didn't allow you to change it from English. Maybe the test case should be expanded a little to show the problem I mean here.

Also, didn't my earlier patch already fix this problem for you? At least for me it works already without the changes you did.

comment:22 Changed 5 years ago by msaelices

arkx, this patch doesn't break fallback language selection.

Changed 5 years ago by arkx

Updated attached test case to assert that fallback languages other than English still work

comment:23 Changed 5 years ago by arkx

  • Needs tests unset

msaelices, try your patch against my test case to see what the problem is. Your patch does break fallback language mechanism unless the fallback language happens to be English. My original patch doesn't break it and it fixes the bug discussed here, at least for me. Please try my original patch and report if it doesn't work for you.

comment:24 Changed 5 years ago by arkx

Also, if you're using patch to apply the patch: please manually delete tests/regressiontests/views/locale/en directory after applying the patch, as patch doesn't allow you to delete files.

comment:25 Changed 5 years ago by arkx

  • Patch needs improvement unset

Is this flag holding back the review? My patch really does fix this legitimate issue and it's backed up by a test case.

comment:26 Changed 5 years ago by robin

  • Cc robin added

Changed 5 years ago by ramiro

Patch by arkx unified and updated to r12362, with fixes to tests and an optimization to the jsi18n view code

comment:27 Changed 5 years ago by ramiro

  • milestone set to 1.2

In 3594-r12362.diff I've unified the patches javascript_translation_catalog.patch and javascript_translation_test_case.patch previously uploaded by aryx with the following changes:

  • Updated it to r12362.
  • Fixed added tests so they don't pollute other Django suite tests (they were causing problems in a syndication test run later) by segregating them in a separate unit test that preseves the value of settings.LANGUAGE_CODE.
  • Added a small optimization to a code branch.

Notes about mnually removing tests/regressiontests/views/locale/en directory if/when this patch is applied still apply.

comment:28 Changed 5 years ago by jezdez

  • Owner changed from arkx to jezdez
  • Status changed from new to assigned

comment:29 Changed 5 years ago by jezdez

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

(In [12384]) Fixed #3594 - Added ability to discard the language catalog in the JavaScript i18n view in case the selected language is English but no English translation catalog actual exists, e.g. due to being the language translated from. Thanks to msaelices, aryx and Ramiro Morales.

comment:30 Changed 5 years ago by jezdez

(In [12385]) [1.1.X] Fixed #3594 - Added ability to discard the language catalog in the JavaScript i18n view in case the selected language is English but no English translation catalog actual exists, e.g. due to being the language translated from. Thanks to msaelices, aryx and Ramiro Morales.

Backport of r12384.

Conflicts:

tests/regressiontests/views/tests/i18n.py

comment:31 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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