Opened 12 years ago

Closed 11 years ago

#18761 closed Cleanup/optimization (fixed)

Strip fields' values consistently

Reported by: Vlastimil Zíma Owned by: Deni Bertovic
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Currently, some fields strips values during validation and some do not. The state does not even clearly represents whether whitespace is valid part of field's value.

Field Strips Should strip
CharField no no
IntegerField yes yes
FloatField yes yes
DecimalField yes yes
BaseTemporalField yes yes
RegexField no no
EmailField yes yes
URLField no yes
BooleanField no ?
NullBooleanField no ?
IPAddressField no yes
GenericIPAddressField no yes
SlugField no yes

Comments:

  • Spaces are not allowed in URLField, IPAddressField, GenericIPAddressField and SlugField
  • Fairly unclear are boolean fields. IMHO if value to validate is string of whitespace characters, it should validate to None/False as when represented by TextInput user will see empty field.

I have not check localised form fields but I expect more fields that can be safely trimmed.

Maybe a good long-term solution would be list of filters (like validators) which would perform transformation to python object.

Attachments (1)

ticket_18761.diff (1.3 KB ) - added by Lebedev Ilya 11 years ago.
Added stripping for URLField, IPAddressField, GenericIPAddressField, SlugField. I have two questions: is it all that should be done? Why no tests are required?

Download all attachments as: .zip

Change History (11)

comment:1 by Stephen Burrows, 12 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

I've actually gotten this as a feature request on projects I've worked on in the wild... I assumed that django just wasn't stripping any field values of whitespace. But it is, either implicitly or explicity. It would be good to be consistent. So it would make sense (to me) to strip whitespace on !URLField, !IPAddressField, !GenericIPAddressField, and SlugField - maybe leave the BooleanFields for later/never, since the case isn't clear.

comment:2 by Lebedev Ilya, 11 years ago

Owner: changed from nobody to Lebedev Ilya

comment:3 by Lebedev Ilya, 11 years ago

Status: newassigned

by Lebedev Ilya, 11 years ago

Attachment: ticket_18761.diff added

Added stripping for URLField, IPAddressField, GenericIPAddressField, SlugField. I have two questions: is it all that should be done? Why no tests are required?

comment:4 by Lebedev Ilya, 11 years ago

Has patch: set

comment:5 by Claude Paroz, 11 years ago

Needs tests: set

Note that this has been fixed for IPAddressField/GenericIPAddressField in #17751.

comment:6 by Sasha Romijn, 11 years ago

Owner: changed from Lebedev Ilya to Sasha Romijn

comment:7 by Sasha Romijn, 11 years ago

Owner: Sasha Romijn removed
Status: assignednew

comment:8 by Deni Bertovic, 11 years ago

Owner: set to Deni Bertovic
Status: newassigned

comment:9 by Aymeric Augustin, 11 years ago

Needs tests: unset

comment:10 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 7d050e8e9ce0e08aefe035a7e203c764fbfb544b:

Merge pull request #1113 from denibertovic/master

Fixed #18761 -- Added whitespace stripping to URLField and SlugField.

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