Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#14824 closed (fixed)

django.utils.get_format_modules doesn't respect USE_L10N

Reported by: nullie Owned by: nobody
Component: Internationalization Version: 1.2
Severity: Keywords: blocker
Cc: Alexander Koshelev Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

http://code.djangoproject.com/changeset/13898 introduced this error.

This was used to return empty list if localization isn't enabled or language isn't supported:

if not check_for_language(get_language()) or not settings.USE_L10N:
   return []

Changeset incorrectly reversed it into:

if check_for_language(lang) or settings.USE_L10N:
   ... return list of modules ...

This makes it return list of localized settings modules even if L10N is disabled, which breaks admin date input for some locales with enabled I18N and disabled L10N, specifically Russian, because javascript inserts localized dates and view parses non-localized dates (it handles USE_L10N setting correctly).

Attachments (1)

14824-patch.diff (576 bytes ) - added by nullie 13 years ago.

Download all attachments as: .zip

Change History (11)

by nullie, 13 years ago

Attachment: 14824-patch.diff added

comment:1 by Alexander Koshelev, 13 years ago

Cc: Alexander Koshelev added

comment:2 by Claude Paroz, 13 years ago

Component: UncategorizedInternationalization
Has patch: set
Needs tests: set
Triage Stage: UnreviewedAccepted

comment:3 by Claude Paroz, 13 years ago

milestone: 1.3

comment:4 by Matthias Kestenholz, 13 years ago

This patch causes quite a few test failures in regressiontests.i18n. I think the tests do not test the correct behavior though. The docs give the impression that USE_L10N is a global master switch to generally enable or disable localization, but the tests say differently. Template content inside {% localize %} blocks should always be localized it seems, even if USE_L10N is False. I'd argue that this is broken behavior.

tl;dr: I think the patch is correct (it fixes the Django administration app, too). The current testsuite tests broken behavior.

comment:5 by Ramiro Morales, 13 years ago

#15218 describes a series of steps to reproduce a use case in the admin app that is broken because of this issue, was closed as a duplicate of this ticket.

comment:6 by Ramiro Morales, 13 years ago

Keywords: blocker added

comment:7 by Russell Keith-Magee, 13 years ago

Patch needs improvement: set

@mk -- I disagree. If that interpretation were to be followed, there is would be use case for the localize template tag or filter.

The patch certainly appears to be in the right place; but we can satisfy both requirements by eliminating the USE_L10N check entirely. This means that L10N settings will be loaded when USE_L10N is False, but it doesn't mean those formats will actually be used. get_formats already pays attention to USE_L10N; the other problem is the js18n catalog, which isn't being computed correctly.

comment:8 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: newclosed

In [15404]:

Fixed #14824 -- Corrected the handling of formats when USE_L10N is disabled. Thanks to nullie for the report and initial patch, and to idle for the separate report with helpful debug info.

comment:9 by Russell Keith-Magee, 13 years ago

In [15405]:

[1.2.X] Fixed #14824 -- Corrected the handling of formats when USE_L10N is disabled. Thanks to nullie for the report and initial patch, and to idle for the separate report with helpful debug info.

Backport of r15404 from trunk.

comment:10 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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