Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#11907 closed (fixed)

EmailField should run strip()

Reported by: whatcould Owned by: SmileyChris
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: UI/UX:

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 6 years ago.
Patch for #11907
11907.2.diff (1.4 KB) - added by djansoft 6 years ago.
Fix with test.
11907.3.diff (1.3 KB) - added by SmileyChris 6 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 7 years ago by mk

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #6362

comment:2 Changed 6 years ago by whatcould

  • Resolution duplicate deleted
  • Status changed from closed to reopened

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

  • milestone set to 1.2
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 6 years ago by krisneuharth

  • Owner changed from nobody to krisneuharth
  • Status changed from reopened to new

Changed 6 years ago by krisneuharth

Patch for #11907

comment:5 Changed 6 years ago by krisneuharth

  • Has patch set
  • Needs documentation set
  • Needs tests set

comment:6 Changed 6 years ago by krisneuharth

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

comment:7 Changed 6 years ago by djansoft

  • Owner changed from nobody to djansoft
  • Status changed from new to assigned

comment:8 Changed 6 years ago by djansoft

  • Has patch set
  • Needs documentation set
  • Needs tests set

Changed 6 years ago by djansoft

Fix with test.

comment:9 Changed 6 years ago by djansoft

  • Needs documentation unset
  • Needs tests unset
  • Resolution set to fixed
  • Status changed from assigned to closed

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

comment:10 Changed 6 years ago by Alex

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:11 Changed 6 years ago by ubernostrum

  • milestone 1.2 deleted

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

comment:12 Changed 6 years ago by whatcould

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 Changed 6 years ago by ubernostrum

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 Changed 6 years ago by ubernostrum

  • Resolution set to duplicate
  • Status changed from reopened to closed

comment:15 Changed 6 years ago by whatcould

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 Changed 6 years ago by SmileyChris

  • Resolution duplicate deleted
  • Status changed from closed to reopened

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.

Changed 6 years ago by SmileyChris

comment:17 Changed 6 years ago by SmileyChris

  • Owner changed from djansoft to SmileyChris
  • Status changed from reopened to new
  • Triage Stage changed from Accepted to Ready for checkin

comment:18 Changed 6 years ago by adrian

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

(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 Changed 6 years ago by SmileyChris

(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