Opened 6 years ago

Closed 6 years ago

Last modified 5 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: UI/UX:

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 6 years ago.

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by nullie

Attachment: 14824-patch.diff added

comment:1 Changed 6 years ago by Alexander Koshelev

Cc: Alexander Koshelev added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 6 years ago by Claude Paroz

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

comment:3 Changed 6 years ago by Claude Paroz

milestone: 1.3

comment:4 Changed 6 years ago by Matthias Kestenholz

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 Changed 6 years ago by Ramiro Morales

#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 Changed 6 years ago by Ramiro Morales

Keywords: blocker added

comment:7 Changed 6 years ago by Russell Keith-Magee

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 Changed 6 years ago by Russell Keith-Magee

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 Changed 6 years ago by Russell Keith-Magee

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 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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