Skip to content

Fixed #22106 -- Make django.views.i18n.javascript_catalog more flexible #2362

Closed
wants to merge 2 commits into from

3 participants

@timgraham timgraham changed the title from Patch for #22106 to Fixed #22106 -- Make django.views.i18n.javascript_catalog more flexible May 26, 2014
@timgraham
Django member

I know very little about javascript_catalog, but maybe if you could rebase your patch and add some documentation and release notes as described in our patch review checklist, that will help me.

@MoritzS
MoritzS commented Mar 14, 2015

I rebased it and added some documentation.

@timgraham
Django member

buildbot, test this please.

@timgraham
Django member

buildbot, test on selenium.

@timgraham
Django member

Test failure on Python 2:

Traceback (most recent call last):
  File "/mnt/jenkinsdata/workspace/pull-requests-selenium/database/sqlite3/python/python2.7/django/test/utils.py", line 182, in inner
    return test_func(*args, **kwargs)
  File "/mnt/jenkinsdata/workspace/pull-requests-selenium/database/sqlite3/python/python2.7/django/test/utils.py", line 182, in inner
    return test_func(*args, **kwargs)
  File "/mnt/jenkinsdata/workspace/pull-requests-selenium/database/sqlite3/python/python2.7/tests/view_tests/tests/test_i18n.py", line 279, in test_multiple_catalogs
    self.assertEqual(elem.text, "il faut traduire cette chaîne de caractères de app1")
AssertionError: u'il faut traduire cette cha\xeene de caract\xe8res de app1' != 'il faut traduire cette cha\xc3\xaene de caract\xc3\xa8res de app1'
@timgraham timgraham commented on an outdated diff Mar 20, 2015
django/views/i18n.py
- return fmt.replace(/%s/g, function(match){return String(obj.shift())});
- }
- };
-
-
- /* formatting library */
-
- django.formats = {{ formats_str }};
-
- django.get_format = function (format_type) {
- var value = django.formats[format_type];
- if (typeof(value) == 'undefined') {
- return format_type;
- } else {
+ if (!django.jsi18n_initialized) {
+ django.gettext = function (msgid) {
@timgraham
Django member
timgraham added a note Mar 20, 2015

I think we should drop the space after "function" through this file (can be a separate commit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Mar 20, 2015
docs/releases/1.9.txt
@@ -130,6 +130,9 @@ Internationalization
* The :func:`django.views.i18n.set_language` view now properly redirects to
:ref:`translated URLs <url-internationalization>`, when available.
+* The :func:`django.views.i18n.javascript_catalog` view now correctly works
@timgraham
Django member
timgraham added a note Mar 20, 2015

works correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Mar 20, 2015
docs/releases/1.9.txt
@@ -130,6 +130,9 @@ Internationalization
* The :func:`django.views.i18n.set_language` view now properly redirects to
:ref:`translated URLs <url-internationalization>`, when available.
+* The :func:`django.views.i18n.javascript_catalog` view now correctly works
+ if used multiple times with different configuration on the same page.
@timgraham
Django member
timgraham added a note Mar 20, 2015

configurations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Mar 20, 2015
docs/topics/i18n/translation.txt
@@ -943,6 +943,31 @@ different apps and this changes often and you don't want to pull in one big
catalog file. As a security measure, these values can only be either
``django.conf`` or any package from the :setting:`INSTALLED_APPS` setting.
+You can also split the catalogs in multiple urls and load them as you need in
@timgraham
Django member
timgraham added a note Mar 20, 2015

URLs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Mar 20, 2015
docs/topics/i18n/translation.txt
@@ -943,6 +943,31 @@ different apps and this changes often and you don't want to pull in one big
catalog file. As a security measure, these values can only be either
``django.conf`` or any package from the :setting:`INSTALLED_APPS` setting.
+You can also split the catalogs in multiple urls and load them as you need in
+your sites::
+
+ js_info_dict_app = {
+ 'packages': ('your.app.package',),
+ }
+
+ js_info_dict_other_app = {
+ 'packages': ('your.other.app.package',),
+ }
+
+ urlpatterns = [
+ url(r'^jsi18n/app$', javascript_catalog, js_info_dict_app),
@timgraham
Django member
timgraham added a note Mar 20, 2015

/$ (most URL examples use a trailing slash)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Mar 20, 2015
docs/topics/i18n/translation.txt
+
+ js_info_dict_app = {
+ 'packages': ('your.app.package',),
+ }
+
+ js_info_dict_other_app = {
+ 'packages': ('your.other.app.package',),
+ }
+
+ urlpatterns = [
+ url(r'^jsi18n/app$', javascript_catalog, js_info_dict_app),
+ url(r'^jsi18n/other_app$', javascript_catalog, js_info_dict_other_app),
+ ]
+
+If you use more than one ``javascript_catalog`` on a site and some of them
+define the same strings, the ones in the catalog that was loaded latest
@timgraham
Django member
timgraham added a note Mar 20, 2015

ones -> strings
latest -> last

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Mar 20, 2015
docs/topics/i18n/translation.txt
+ js_info_dict_other_app = {
+ 'packages': ('your.other.app.package',),
+ }
+
+ urlpatterns = [
+ url(r'^jsi18n/app$', javascript_catalog, js_info_dict_app),
+ url(r'^jsi18n/other_app$', javascript_catalog, js_info_dict_other_app),
+ ]
+
+If you use more than one ``javascript_catalog`` on a site and some of them
+define the same strings, the ones in the catalog that was loaded latest
+take precedence.
+
+.. versionchanged:: 1.9
+
+Before Django 1.9, the catalogs completely overwrote each other and you could
@timgraham
Django member
timgraham added a note Mar 20, 2015

indent 4 spaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Django member

Please bump the ticket to "ready for checkin" after making those updates, thanks!

@timgraham
Django member

buildbot, test on selenium.

@MoritzS
MoritzS commented Mar 20, 2015

There seems to be a problem with the build bot, the errors come from Java Exceptions.

@timgraham
Django member

buildbot, test on selenium.

@claudep claudep and 2 others commented on an outdated diff Mar 20, 2015
tests/view_tests/tests/test_i18n.py
@@ -270,6 +270,16 @@ def test_javascript_gettext(self):
elem = self.selenium.find_element_by_id("npgettext_plur")
self.assertEqual(elem.text, "455 Resultate")
+ @modify_settings(INSTALLED_APPS={'append': ['view_tests.app1', 'view_tests.app2']})
+ @override_settings(LANGUAGE_CODE='fr')
+ def test_multiple_catalogs(self):
+ self.selenium.get('%s%s' % (self.live_server_url, '/jsi18n_multi_catalogs/'))
+
+ elem = self.selenium.find_element_by_id('app1string')
+ self.assertEqual(elem.text.encode('utf8'), b'il faut traduire cette cha\xc3\xaene de caract\xc3\xa8res de app1')
@claudep
Django member
claudep added a note Mar 20, 2015

Why do you encode then test the bytestring, instead of testing the original unicode result?

@MoritzS
MoritzS added a note Mar 20, 2015

I think that's the most portable way for testing this with python 2 and 3.
Another way would be to test:

self.assertEqual(elem.text, u'il faut traduire cette chaîne de caractères de app1')

But I think, it's better to not depend on how selenium or the current python version interpret the string.

@timgraham
Django member
timgraham added a note Mar 20, 2015

You can add from __future__ import unicode_literals to that test file.

@MoritzS
MoritzS added a note Mar 20, 2015

I was not sure if that's ok.
So I'll just do that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
MoritzS added some commits Feb 23, 2014
@MoritzS MoritzS Fixed #22106 -- Make django.views.i18n.javascript_catalog more flexible 8c96263
@MoritzS MoritzS Removed spaces after 'function' in jsi18n
0e88f45
@timgraham
Django member

merged in 6bb2175, thanks!

@timgraham timgraham closed this Mar 20, 2015
@MoritzS MoritzS deleted the MoritzS:patch22106 branch Sep 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.