#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)
Change History (11)
by , 14 years ago
Attachment: | 14824-patch.diff added |
---|
comment:1 by , 14 years ago
Cc: | added |
---|
comment:2 by , 14 years ago
Component: | Uncategorized → Internationalization |
---|---|
Has patch: | set |
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 14 years ago
milestone: | → 1.3 |
---|
comment:4 by , 14 years ago
comment:5 by , 14 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 , 14 years ago
Keywords: | blocker added |
---|
comment:7 by , 14 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.
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.