Fixed #18231 -- Made JavaScript i18n not pollute global JS namespace.

Also, use Django templating for the dynamic generated JS code and use
more idiomatic coding techniques.

Thanks Matthew Tretter for the report and the patch.

AUTHORS

@@ -558,6 +558,7 @@ answer newbie questions, and generally made Django that much better:
     Tom Tobin
     Joe Topjian <http://joe.terrarum.net/geek/code/python/django/>
     torne-django@wolfpuppy.org.uk
+    Matthew Tretter <m@tthewwithanm.com>
     Jeff Triplett <jeff.triplett@gmail.com>
     tstromberg@google.com
     Makoto Tsuyuki <mtsuyuki@gmail.com>

django/contrib/admin/static/admin/js/jquery.init.js

@@ -3,6 +3,5 @@
  * namespace (i.e. this preserves pre-existing values for both window.$ and
  * window.jQuery).
  */
-var django = {
+var django = django || {};
-    "jQuery": jQuery.noConflict(true)
+django.jQuery = jQuery.noConflict(true);
-};

django/views/i18n.py

@@ -1,11 +1,12 @@
+import json
 import os
 import gettext as gettext_module
 
 from django import http
 from django.conf import settings
+from django.template import Context, Template
 from django.utils import importlib
 from django.utils.translation import check_for_language, activate, to_locale, get_language
-from django.utils.text import javascript_quote
 from django.utils.encoding import smart_text
 from django.utils.formats import get_format_modules, get_format
 from django.utils._os import upath
@@ -38,6 +39,7 @@ def set_language(request):
                 response.set_cookie(settings.LANGUAGE_COOKIE_NAME, lang_code)
     return response
 
+
 def get_formats():
     """
     Returns all formats strings required for i18n to work
@@ -53,120 +55,142 @@ def get_formats():
     for module in [settings] + get_format_modules(reverse=True):
         for attr in FORMAT_SETTINGS:
             result[attr] = get_format(attr)
-    src = []
+    formats = {}
     for k, v in result.items():
         if isinstance(v, (six.string_types, int)):
-            src.append("formats['%s'] = '%s';\n" % (javascript_quote(k), javascript_quote(smart_text(v))))
+            formats[k] = smart_text(v)
         elif isinstance(v, (tuple, list)):
-            v = [javascript_quote(smart_text(value)) for value in v]
+            formats[k] = [smart_text(value) for value in v]
-            src.append("formats['%s'] = ['%s'];\n" % (javascript_quote(k), "', '".join(v)))
+    return formats
-    return ''.join(src)
 
-NullSource = """
-/* gettext identity library */
 
-function gettext(msgid) { return msgid; }
+js_catalog_template = r"""
-function ngettext(singular, plural, count) { return (count == 1) ? singular : plural; }
+{% autoescape off %}
-function gettext_noop(msgid) { return msgid; }
+(function (globals) {
-function pgettext(context, msgid) { return msgid; }
-function npgettext(context, singular, plural, count) { return (count == 1) ? singular : plural; }
-"""
 
-LibHead = """
+  var django = globals.django || (globals.django = {});
-/* gettext library */
 
-var catalog = new Array();
+  {% if plural %}
-"""
+  django.pluralidx = function (n) {
+    var v={{ plural }};
+    if (typeof(v) == 'boolean') {
+      return v ? 1 : 0;
+    } else {
+      return v;
+    }
+  };
+  {% else %}
+  django.pluralidx = function (count) { return (count == 1) ? 0 : 1; };
+  {% endif %}
 
-LibFoot = """
+  {% if catalog_str %}
+  /* gettext library */
 
-function gettext(msgid) {
+  django.catalog = {{ catalog_str }};
-  var value = catalog[msgid];
-  if (typeof(value) == 'undefined') {
-    return msgid;
-  } else {
-    return (typeof(value) == 'string') ? value : value[0];
-  }
-}
 
-function ngettext(singular, plural, count) {
+  django.gettext = function (msgid) {
-  value = catalog[singular];
+    var value = django.catalog[msgid];
-  if (typeof(value) == 'undefined') {
+    if (typeof(value) == 'undefined') {
-    return (count == 1) ? singular : plural;
+      return msgid;
-  } else {
+    } else {
-    return value[pluralidx(count)];
+      return (typeof(value) == 'string') ? value : value[0];
-  }
+    }
-}
+  };
 
-function gettext_noop(msgid) { return msgid; }
+  django.ngettext = function (singular, plural, count) {
+    value = django.catalog[singular];
+    if (typeof(value) == 'undefined') {
+      return (count == 1) ? singular : plural;
+    } else {
+      return value[django.pluralidx(count)];
+    }
+  };
 
-function pgettext(context, msgid) {
+  django.gettext_noop = function (msgid) { return msgid; };
-  var value = gettext(context + '\\x04' + msgid);
-  if (value.indexOf('\\x04') != -1) {
-    value = msgid;
-  }
-  return value;
-}
 
-function npgettext(context, singular, plural, count) {
+  django.pgettext = function (context, msgid) {
-  var value = ngettext(context + '\\x04' + singular, context + '\\x04' + plural, count);
+    var value = django.gettext(context + '\x04' + msgid);
-  if (value.indexOf('\\x04') != -1) {
+    if (value.indexOf('\x04') != -1) {
-    value = ngettext(singular, plural, count);
+      value = msgid;
-  }
+    }
-  return value;
+    return value;
-}
+  };
-"""
 
-LibFormatHead = """
+  django.npgettext = function (context, singular, plural, count) {
-/* formatting library */
+    var value = django.ngettext(context + '\x04' + singular, context + '\x04' + plural, count);
+    if (value.indexOf('\x04') != -1) {
+      value = django.ngettext(singular, plural, count);
+    }
+    return value;
+  };
+  {% else %}
+  /* gettext identity library */
 
-var formats = new Array();
+  django.gettext = function (msgid) { return msgid; };
+  django.ngettext = function (singular, plural, count) { return (count == 1) ? singular : plural; };
+  django.gettext_noop = function (msgid) { return msgid; };
+  django.pgettext = function (context, msgid) { return msgid; };
+  django.npgettext = function (context, singular, plural, count) { return (count == 1) ? singular : plural; };
+  {% endif %}
 
-"""
+  django.interpolate = function (fmt, obj, named) {
+    if (named) {
+      return fmt.replace(/%\(\w+\)s/g, function(match){return String(obj[match.slice(2,-2)])});
+    } else {
+      return fmt.replace(/%s/g, function(match){return String(obj.shift())});
+    }
+  };
 
-LibFormatFoot = """
+
-function get_format(format_type) {
+  /* formatting library */
-    var value = formats[format_type];
+
+  django.formats = {{ formats_str }};
+
+  django.get_format = function (format_type) {
+    var value = django.formats[format_type];
     if (typeof(value) == 'undefined') {
       return format_type;
     } else {
       return value;
     }
-}
+  };
+
+  /* add to global namespace */
+  globals.pluralidx = django.pluralidx;
+  globals.gettext = django.gettext;
+  globals.ngettext = django.ngettext;
+  globals.gettext_noop = django.gettext_noop;
+  globals.pgettext = django.pgettext;
+  globals.npgettext = django.npgettext;
+  globals.interpolate = django.interpolate;
+  globals.get_format = django.get_format;
+
+}(this));
+{% endautoescape %}
 """
 
-SimplePlural = """
-function pluralidx(count) { return (count == 1) ? 0 : 1; }
-"""
 
-InterPolate = r"""
+def render_javascript_catalog(catalog=None, plural=None):
-function interpolate(fmt, obj, named) {
+    template = Template(js_catalog_template)
-  if (named) {
+    indent = lambda s: s.replace('\n', '\n  ')
-    return fmt.replace(/%\(\w+\)s/g, function(match){return String(obj[match.slice(2,-2)])});
+    context = Context({
-  } else {
+        'catalog_str': indent(json.dumps(
-    return fmt.replace(/%s/g, function(match){return String(obj.shift())});
+            catalog, sort_keys=True, indent=2)) if catalog else None,
-  }
+        'formats_str': indent(json.dumps(
-}
+            get_formats(), sort_keys=True, indent=2)),
-"""
+        'plural': plural,
+    })
+
+    return http.HttpResponse(template.render(context), 'text/javascript')
 
-PluralIdx = r"""
-function pluralidx(n) {
-  var v=%s;
-  if (typeof(v) == 'boolean') {
-    return v ? 1 : 0;
-  } else {
-    return v;
-  }
-}
-"""
 
 def null_javascript_catalog(request, domain=None, packages=None):
     """
     Returns "identity" versions of the JavaScript i18n functions -- i.e.,
     versions that don't actually do anything.
     """
-    src = [NullSource, InterPolate, LibFormatHead, get_formats(), LibFormatFoot]
+    return render_javascript_catalog()
-    return http.HttpResponse(''.join(src), 'text/javascript')
+
 
 def javascript_catalog(request, domain='djangojs', packages=None):
     """
@@ -243,42 +267,32 @@ def javascript_catalog(request, domain='djangojs', packages=None):
                     locale_t.update(catalog._catalog)
             if locale_t:
                 t = locale_t
-    src = [LibHead]
     plural = None
     if '' in t:
         for l in t[''].split('\n'):
             if l.startswith('Plural-Forms:'):
-                plural = l.split(':',1)[1].strip()
+                plural = l.split(':', 1)[1].strip()
     if plural is not None:
         # this should actually be a compiled function of a typical plural-form:
         # Plural-Forms: nplurals=3; plural=n%10==1 && n%100!=11 ? 0 : n%10>=2 && n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2;
-        plural = [el.strip() for el in plural.split(';') if el.strip().startswith('plural=')][0].split('=',1)[1]
+        plural = [el.strip() for el in plural.split(';') if el.strip().startswith('plural=')][0].split('=', 1)[1]
-        src.append(PluralIdx % plural)
+
-    else:
-        src.append(SimplePlural)
-    csrc = []
     pdict = {}
+    maxcnts = {}
+    catalog = {}
     for k, v in t.items():
         if k == '':
             continue
         if isinstance(k, six.string_types):
-            csrc.append("catalog['%s'] = '%s';\n" % (javascript_quote(k), javascript_quote(v)))
+            catalog[k] = v
         elif isinstance(k, tuple):
-            if k[0] not in pdict:
+            msgid = k[0]
-                pdict[k[0]] = k[1]
+            cnt = k[1]
-            else:
+            maxcnts[msgid] = max(cnt, maxcnts.get(msgid, 0))
-                pdict[k[0]] = max(k[1], pdict[k[0]])
+            pdict.setdefault(msgid, {})[cnt] = v
-            csrc.append("catalog['%s'][%d] = '%s';\n" % (javascript_quote(k[0]), k[1], javascript_quote(v)))
         else:
             raise TypeError(k)
-    csrc.sort()
     for k, v in pdict.items():
-        src.append("catalog['%s'] = [%s];\n" % (javascript_quote(k), ','.join(["''"]*(v+1))))
+        catalog[k] = [v.get(i, '') for i in range(maxcnts[msgid] + 1)]
-    src.extend(csrc)
+
-    src.append(LibFoot)
+    return render_javascript_catalog(catalog, plural)
-    src.append(InterPolate)
-    src.append(LibFormatHead)
-    src.append(get_formats())
-    src.append(LibFormatFoot)
-    src = ''.join(src)
-    return http.HttpResponse(src, 'text/javascript')

tests/view_tests/tests/test_i18n.py

@@ -61,13 +61,13 @@ class I18NTests(TestCase):
                 else:
                     trans_txt = catalog.ugettext('this is to be translated')
                 response = self.client.get('/views/jsi18n/')
-                # in response content must to be a line like that:
+                # response content must include a line like:
-                # catalog['this is to be translated'] = 'same_that_trans_txt'
+                # "this is to be translated": <value of trans_txt Python variable>
                 # javascript_quote is used to be able to check unicode strings
                 self.assertContains(response, javascript_quote(trans_txt), 1)
                 if lang_code == 'fr':
                     # Message with context (msgctxt)
-                    self.assertContains(response, "['month name\x04May'] = 'mai';", 1)
+                    self.assertContains(response, r'"month name\u0004May": "mai"', 1)
 
 
 class JsI18NTests(TestCase):