Opened 6 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 6 years ago.
chrome debugger window when the problem occurs

Download all attachments as: .zip

Change History (13)

comment:1 by Jeremy Moffitt, 6 years ago

Description: modified (diff)

comment:2 by Claude Paroz, 6 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, 6 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, 6 years ago

Description: modified (diff)
Triage Stage: UnreviewedAccepted

Tentatively accepting for investigation.

comment:5 by Jeremy Moffitt, 6 years ago

I've got the unit tests running, though I'm not 100% clear on how this section gets tested given that its part of a javascript template within django (defined in django/views/i18n.py) . I do see that jsi18n-mocks.test.js defines:

    django.ngettext = function(singular, plural, count) {
        return (count === 1) ? singular : plural;
    };

the tests under tests/i18n/tests.py are testing the functions from utils/translation (where trans_real.py and trans_null.py both define ngettext). I'll continue investigating to see if I can pin it down further.

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

Version 0, edited 6 years ago by Jeremy Moffitt (next)

comment:6 by Jeremy Moffitt, 6 years ago

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

by Jeremy Moffitt, 6 years ago

chrome debugger window when the problem occurs

comment:7 by Jeremy Moffitt, 6 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, 6 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