Opened 15 years ago

Closed 13 years ago

Last modified 13 years ago

#11907 closed (fixed)

EmailField should run strip()

Reported by: whatcould Owned by: Chris Beaven
Component: Forms Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The EmailField should run strip() to remove opening and trailing spaces before the validation is run. Copying and pasting an email can easily pick up a trailing space (I did it myself today).

The code would be something like this, from forms.fields.EmailField. (My first python, sorry if it's ugly, I'm a rubyist.)

  def clean(self, value):
      value = value.strip()
      value = super(RegexField, self).clean(value)
      return value

Attachments (3)

11907.diff (535 bytes ) - added by krisneuharth 14 years ago.
Patch for #11907
11907.2.diff (1.4 KB ) - added by Daniel Gonzalez Gasull 14 years ago.
Fix with test.
11907.3.diff (1.3 KB ) - added by Chris Beaven 13 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 by Matthias Kestenholz, 14 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #6362

comment:2 by whatcould, 14 years ago

Resolution: duplicate
Status: closedreopened

This is clearly not a duplicate of #6362. Email is a specific kind of data -- #6362 is the general case. (Which, of course, should also be changed, but that's not the point here.)

Emails can not have spaces, so the email. Nothing to do with stripping spaces for other field types.

If the argument against #6362 is that sometimes you might want spaces in the data, that argument is moot here, because you would never want spaces in a user-entered email address. The examples people brought up in #6362, appropriately enough, are of users entering emails (with spaces) and being confused/frustrated.

comment:3 by Russell Keith-Magee, 14 years ago

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:4 by krisneuharth, 14 years ago

Owner: changed from nobody to krisneuharth
Status: reopenednew

by krisneuharth, 14 years ago

Attachment: 11907.diff added

Patch for #11907

comment:5 by krisneuharth, 14 years ago

Has patch: set
Needs documentation: set
Needs tests: set

comment:6 by krisneuharth, 14 years ago

Has patch: unset
Needs documentation: unset
Needs tests: unset
Owner: changed from krisneuharth to nobody

comment:7 by Daniel Gonzalez Gasull, 14 years ago

Owner: changed from nobody to Daniel Gonzalez Gasull
Status: newassigned

comment:8 by Daniel Gonzalez Gasull, 14 years ago

Has patch: set
Needs documentation: set
Needs tests: set

by Daniel Gonzalez Gasull, 14 years ago

Attachment: 11907.2.diff added

Fix with test.

comment:9 by Daniel Gonzalez Gasull, 14 years ago

Needs documentation: unset
Needs tests: unset
Resolution: fixed
Status: assignedclosed

Added fix based on krisneuharth's diff. Added test.

comment:10 by Alex Gaynor, 14 years ago

Resolution: fixed
Status: closedreopened

No patch was committed to Django, tickets are not fixed when a patch is uploaded.

comment:11 by James Bennett, 14 years ago

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:12 by whatcould, 14 years ago

This sounds a lot like a bug to me, James, not a feature. Emails are emails, and should be formatted as such. Why do you call the thing "email field" anyway if you let any old spaces in?

I'd wager users who try to enter emails, include an incidental space, and then can't log in -- they think it's a bug.

comment:13 by James Bennett, 14 years ago

Well... actually it's part of (and thus now being closed as a duplicate of) #6362, which asks for whitespace stripping to be applied before validating data in all text-based field types. And that's definitely a feature request, so once again there's no way for this to make it into 1.2. The best way to advance this is to hold on for a bit while 1.2 gets out the door, then lobby for it to get into 1.3.

comment:14 by James Bennett, 14 years ago

Resolution: duplicate
Status: reopenedclosed

comment:15 by whatcould, 14 years ago

ubernostrum, if you see my comment above (02/04/10 00:25:26) I explained why this should be marked as a separate bug, instead of being thrown into the 2-year-old black hole that is that other bug. Someone else apparently agreed at the time.

This is all no skin off my back (I was consulting on a django project that's finished) -- I was just trying to follow through on this bug. Wish it hadn't felt like pushing on a string.

-whatcould (fellow joyeur)

comment:16 by Chris Beaven, 13 years ago

Resolution: duplicate
Status: closedreopened

This is in the same basket as #5714 (also originally marked as a dupe of #6362).

Malcolm reopened that one, so I'm going to reopen this one.

by Chris Beaven, 13 years ago

Attachment: 11907.3.diff added

comment:17 by Chris Beaven, 13 years ago

Owner: changed from Daniel Gonzalez Gasull to Chris Beaven
Status: reopenednew
Triage Stage: AcceptedReady for checkin

comment:18 by Adrian Holovaty, 13 years ago

Resolution: fixed
Status: newclosed

(In [13997]) Fixed #11907 -- EmailField now runs strip() on its input. This means mistakenly including leading or trailing spaces will not cause a validation error, and clean() will remove those spaces. Thanks, krisneuharth, djansoft and SmileyChris

comment:19 by Chris Beaven, 13 years ago

(In [14124]) [1.2.X] Fixed #11907 -- EmailField now runs strip() on its input. This means mistakenly including leading or trailing spaces will not cause a validation error, and clean() will remove those spaces. Thanks, krisneuharth and djansoft. Backport of [13997].

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