Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#29825 closed Bug (fixed)

ngettext returns invalid result if msgstr is also a valid msgid in the same catalog

Reported by: Jeremy Moffitt Owned by: nobody
Component: Internationalization Version: 2.1
Severity: Normal Keywords: ngettext localization
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

When ngettext is called with a msgid and that msgid has a msgstr that is also a valid msgid in the same catalog, the return value is not the msgstr, but instead is a single character from the msgstr. The problem seems to be that the msgstr is passed back into ngettext and since it has a valid value in the catalog, the code breaks into the else block and returns the 0 index character...

    django.ngettext = function(singular, plural, count) {
      var value = django.catalog[singular];
      if (typeof(value) == 'undefined') {
        return (count == 1) ? singular : plural;
      } else {
        return value[django.pluralidx(count)];
      }
    };

The example in OpenStack Horizon (see link below) is that the French bundle contains the following:

msgid "Image"
msgstr "Image"

This should return "Image" ... instead it returns "I" ... the result of django.catalog["Image"] being "Image" , which then breaks into the else block of the following if statement, resulting in a return value of "Image"[0] ... or the capital letter "I". For languages where the msgstr does not match a valid key in the file, this problem does not occur.

see also openstack bug: https://bugs.launchpad.net/horizon/+bug/1778189

Attachments (1)

django_ngettext_horizon_debugger.png (228.1 KB ) - added by Jeremy Moffitt 5 years ago.
chrome debugger window when the problem occurs

Download all attachments as: .zip

Change History (13)

comment:1 by Jeremy Moffitt, 5 years ago

Description: modified (diff)

comment:2 by Claude Paroz, 5 years ago

Component: UncategorizedInternationalization
Type: UncategorizedBug

Jeremy, would you be able to write a failing test? (probably in i18n.tests.TranslationTests)?

in reply to:  2 comment:3 by Jeremy Moffitt, 5 years ago

Replying to Claude Paroz:

Jeremy, would you be able to write a failing test? (probably in i18n.tests.TranslationTests)?

I can try, I'm not particularly familiar with the Django test suite. I ran into this debugging the Horizon bug in OpenStack and have no experience using Django outside of Horizon.

comment:4 by Tim Graham, 5 years ago

Description: modified (diff)
Triage Stage: UnreviewedAccepted

Tentatively accepting for investigation.

comment:5 by Jeremy Moffitt, 5 years ago

*edit* found the view tests in test_i18n.py , attempting to replicate the scenario there

When the problem is occurring in Horizon, the initial call into the django.ngettext (as observed via the debugger) passes in ("Image", "Images", undefined)

Last edited 5 years ago by Jeremy Moffitt (previous) (diff)

comment:6 by Jeremy Moffitt, 5 years ago

Pull request with similar behavior uploaded at https://github.com/django/django/pull/10506

by Jeremy Moffitt, 5 years ago

chrome debugger window when the problem occurs

comment:7 by Jeremy Moffitt, 5 years ago

The following passes the unit test, but I'm not clear if it will have other repercussions I'm overlooking:

diff --git a/django/views/i18n.py b/django/views/i18n.py
index e121d3038a..82978dace2 100644
--- a/django/views/i18n.py
+++ b/django/views/i18n.py
@@ -114,6 +114,8 @@ js_catalog_template = r"""
       var value = django.catalog[singular];
       if (typeof(value) == 'undefined') {
         return (count == 1) ? singular : plural;
+      } if (typeof(count) == 'undefined') {
+        return singular;
       } else {
         return value[django.pluralidx(count)];
       }

comment:8 by Tim Graham, 5 years ago

Has patch: set

comment:9 by Claude Paroz, 5 years ago

Patch needs improvement: set

comment:10 by Tim Graham, 5 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

PR from Claude looks fine to me if it works for the reporter.

comment:11 by Claude Paroz <claude@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 16454ac3:

Fixed #29825 -- Fixed JS ngettext if the string is a non-plural msgid in the catalog.

comment:12 by Tim Graham <timograham@…>, 5 years ago

In caaa0114:

[2.2.x] Fixed #29825 -- Fixed JS ngettext if the string is a non-plural msgid in the catalog.

Backport of 16454ac35f6a24a04b23a9340b0d62c33edbc1ea from master.

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