Opened 10 months ago

Last modified 10 months ago

#22654 new Bug

DecimalField and DECIMAL_SEPARATOR (in admin)

Reported by: matija@… Owned by:
Component: Forms Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

DecimalField with following settings:

LANGUAGE_CODE = 'hr-hr'
TIME_ZONE = 'CET'
USE_I18N = True
USE_L10N = False
USE_TZ = False
DATE_FORMAT = 'd.m.Y.'
DATE_INPUT_FORMATS = ('%d.%m.%Y.', '%d.%m.%Y', '%Y-%m-%d')
DECIMAL_SEPARATOR = ','

Correctly shows Decimal field in admin, but raises validation error.

As most of our customers do not set their preferences to Croatian, and we need to show Croatian locale for all users. So with settings above, things sdhould work.

Tried in 1.6 and 1.7.

In order to reproduce, all you need is a single model with a DecimalField, registered in Admin. And the above settings. Input any non-integer value and press save_and_continue_editing. If you encounter no errors, press it again. Now it will show an error.

I added a new ticked because related ones were closed years ago.

Change History (11)

comment:1 Changed 10 months ago by sabinemaennel

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to sabinemaennel
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 10 months ago by sabinemaennel

I think this is clearly an error. I tested it with a decimal field. The settings DECIMAL_SEPARATOR = ',' is taken into account when the field is shown in the admin, but when validating the field only input with a . as delimiter is accepted. The field validation needs to be adapted to the delimiter given in the settings as well. I tested this with the new 1.7 Django.

comment:3 Changed 10 months ago by sabinemaennel

  • Component changed from Uncategorized to Forms
  • Owner sabinemaennel deleted
  • Status changed from assigned to new
  • Triage Stage changed from Unreviewed to Someday/Maybe
  • Type changed from Uncategorized to Bug

This is actually an HTML5 Problem. The admin form for the input of a number uses the following HTML5 statement:
<input id="id_decimal_number" name="..." step="0.01" type="number" />.
step= in HTML5 can only be defined that way with a .as delimiter. Then the browser will show it on the screen according to local browser settings. Mine showed it as a dot, but I read on stackoverflow, that some other users see it with a comma.http://stackoverflow.com/questions/6178332/force-decimal-point-instead-of-comma-in-html5-number-input-client-side.

I think HTML5 will develop further and this behaviour can be corrected via a Javascript-Library-Use in the meantime. I am not sure what to do with this bug. I would probably place it outside the influence of Django even though I can see that it is annoying. Django decided to use HTML5 for its forms and this is due to the actual state of HTML5. I feel Django is not responsible to correct HTML5 behaviour, but I would really like to know the opinion of some more experienced Django contributers on this. So I am handing the ticket back.

comment:4 Changed 10 months ago by matija@…

ACTUALLY this is DJANGO problem. Remove type="number". The ValidationError on Django side is the issue, not browser side.

comment:5 Changed 10 months ago by claudep

Could you please give us a little more information: what is displayed exactly in your widget (dot or comma), what value is actually posted to the request, what validation error message do you get ?

comment:6 Changed 10 months ago by anonymous

Sure. With settings above, everything works as excpected, comma is shown, but when I try to save, ValidationError is shown.
Error is, of course, in croatian, but it comes back to this:

invalid': _('Enter a number.'), https://github.com/django/django/blob/master/django/forms/fields.py

If L10N is set tu True, everything actually works. The problem is in this line. When self.localize is set to true, this works fine, but this should be run when localization is off, but DECIMAL_SEPARATOR is anything other than '.'. Would anything break if this "if" is simple removed?

if self.localize:

value = formats.sanitize_separators(value)

comment:7 Changed 10 months ago by claudep

So do you really get a comma in the widget value, even when L10N is off? Is that dot-to-comma conversion done by the browser itself (AFAIK Django is sending a dot in the HTML source)?

comment:8 Changed 10 months ago by matija@…

Ok, I got it.

This is definitely a design problem. Date/Datetime settings work regardless of L10N, and Decimal-related only when L10N is set. (sanitize_separators checks L10N first).

These two things are closely related and should follow the same logic.

IMHO the key question is should Dates AND Dacimals (and other L10N-related stuff) follow DATE_FORMATS, DECIMAL_SEPARATOR and other L10N related setting when L10N is False. The answer should be Yes or No for all of them.

Yes, I do get a comma in the output. However, on re-saving, errors are shown. If I replace it with a dot, it works, and comma is shown in response.

comment:9 Changed 10 months ago by anonymous

Sorry, no comma with the default widget. However, with TextInput, it is there.

This design problem still exists, at least docs should be updated indicating what works when. This definitely does not work as one may expect.

However, there SHOULD be a way to work with a fixed locale other than en-us ;)

comment:10 Changed 10 months ago by claudep

  • Triage Stage changed from Someday/Maybe to Accepted

I think we should be as much as flexible as possible on the input side of things (potentially removing the is_localize/USE_L10N tests).
However, it's a bit tricky when you get ambiguous inputs like 24,637...

comment:11 Changed 10 months ago by matija

Well, it is clear that nothing should be done quickly. The whole thing seems a bit messy. I did test many different configs and mostly got unexpected results, either in Decimal or Date.

Id say this is 1.8 issue.

My deadline is approaching and I already spent too much time on this, right now I have a middleware forcing locale I want and L10N turned on and formats ... works fine.

Ill do an update when I find time to re-test from scratch (weekend perhaps).

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