Opened 10 years ago

Closed 7 years ago

Last modified 5 years ago

#3594 closed (fixed)

Translation problem in javascript

Reported by: karsu Owned by: Jannis Leidel
Component: Internationalization Version: master
Severity: Keywords:
Cc: Jannis Leidel, 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 9 years ago.
Finally I fixed this for my own use.
js_i18n_test.zip (6.5 KB) - added by yishaibeeri 8 years ago.
minimal project to reproduce bug in admin
javascript_translation_r8521.patch (1.1 KB) - added by Manuel Saelices 8 years ago.
Patch updated to last version, r8521.
js_i18n_test.tgz (7.7 KB) - added by Manuel Saelices 8 years ago.
Updated the test project, to new admin app.
javascript_translation_with_test_r8521.patch (3.6 KB) - added by Manuel Saelices 8 years ago.
Patch updated with a regression test
jsi18n.patch (553 bytes) - added by angelolaub@… 8 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 Aku Kotkavuo 7 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 Manuel Saelices 7 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 Aku Kotkavuo 7 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 Morales 7 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 10 years ago by Gary Wilson <gary.wilson@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 10 years ago by Malcolm Tredinnick

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

Changed 9 years ago by Timo

Finally I fixed this for my own use.

comment:3 Changed 9 years ago by anonymous

Has patch: set
Needs tests: set

comment:4 Changed 9 years ago by anonymous

Needs tests: unset
Patch needs improvement: set

comment:5 Changed 9 years ago by anonymous

Needs tests: set
Patch needs improvement: unset

comment:6 Changed 9 years ago by Malcolm Tredinnick

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 9 years ago by Manuel Saelices

Owner: changed from nobody to Manuel Saelices
Status: newassigned

I'll try to fixed that and add tests

comment:8 Changed 9 years ago by Manuel Saelices

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 9 years ago by Manuel Saelices

Triage Stage: AcceptedReady 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 9 years ago by Russell Keith-Magee

Needs tests: set
Triage Stage: Ready for checkinAccepted

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 Changed 9 years ago by Manuel Saelices

Resolution: worksforme
Status: assignedclosed

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 8 years ago by yishaibeeri

Attachment: js_i18n_test.zip added

minimal project to reproduce bug in admin

comment:12 Changed 8 years ago by yishaibeeri

Resolution: worksforme
Status: closedreopened

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 8 years ago by Marc Fargas

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 8 years ago by Manuel Saelices

Patch updated to last version, r8521.

Changed 8 years ago by Manuel Saelices

Attachment: js_i18n_test.tgz added

Updated the test project, to new admin app.

Changed 8 years ago by Manuel Saelices

Patch updated with a regression test

comment:14 Changed 8 years ago by Manuel Saelices

Needs tests: unset

Changed 8 years ago by angelolaub@…

Attachment: jsi18n.patch added

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 7 years ago by Aku Kotkavuo

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 7 years ago by Aku Kotkavuo

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 7 years ago by Aku Kotkavuo

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 7 years ago by Aku Kotkavuo

Owner: changed from Manuel Saelices to Aku Kotkavuo
Status: reopenednew

comment:18 Changed 7 years ago by Jannis Leidel

Cc: Jannis Leidel added
Needs tests: set
Patch needs improvement: set

Checking for english

comment:19 Changed 7 years ago by Jannis Leidel

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 7 years ago by Aku Kotkavuo

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 7 years ago by Manuel Saelices

Attachment: jsi18n_11620.patch added

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

comment:21 Changed 7 years ago by Aku Kotkavuo

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 7 years ago by Manuel Saelices

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

Changed 7 years ago by Aku Kotkavuo

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

comment:23 Changed 7 years ago by Aku Kotkavuo

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 7 years ago by Aku Kotkavuo

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 7 years ago by Aku Kotkavuo

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 7 years ago by Robin

Cc: Robin added

Changed 7 years ago by Ramiro Morales

Attachment: 3594-r12362.diff added

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

comment:27 Changed 7 years ago by Ramiro Morales

milestone: 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 7 years ago by Jannis Leidel

Owner: changed from Aku Kotkavuo to Jannis Leidel
Status: newassigned

comment:29 Changed 7 years ago by Jannis Leidel

Resolution: fixed
Status: assignedclosed

(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 7 years ago by Jannis Leidel

(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 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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