Opened 5 years ago

Closed 5 years ago

Last modified 4 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: alexkoshelev 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 5 years ago.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by nullie

comment:1 Changed 5 years ago by alexkoshelev

  • Cc alexkoshelev added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by claudep

  • Component changed from Uncategorized to Internationalization
  • Has patch set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 5 years ago by claudep

  • milestone set to 1.3

comment:4 Changed 5 years ago by mk

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

#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 5 years ago by ramiro

  • Keywords blocker added

comment:7 Changed 5 years ago by russellm

  • 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 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

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

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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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