RESOLVED FIXED 27457
Add <input type=email> validation support
https://bugs.webkit.org/show_bug.cgi?id=27457
Summary Add <input type=email> validation support
Peter Kasting
Reported 2009-07-20 12:29:07 PDT
See spec section 4.10.4.1.5. Basically, the only difference between an e-mail input and a normal text input is that at validation time the e-mail input can return true from validityState's typeMismatch() method if it doesn't contain a valid email address or list of addresses. The spec covers in detail what those consist of.
Attachments
Patch v1 (7.83 KB, patch)
2009-08-24 10:23 PDT, Michelangelo De Simone
no flags
Patch v1a (7.80 KB, patch)
2009-08-24 10:25 PDT, Michelangelo De Simone
no flags
Patch v1, Changelogs were missing (9.29 KB, patch)
2009-08-24 10:32 PDT, Michelangelo De Simone
eric: review-
Patch rev.2 (9.96 KB, patch)
2009-08-26 12:32 PDT, Michelangelo De Simone
no flags
Patch rev.2a (9.91 KB, patch)
2009-08-28 16:03 PDT, Michelangelo De Simone
no flags
Patch rev. 2b (10.51 KB, patch)
2009-09-15 08:37 PDT, Michelangelo De Simone
no flags
Peter Kasting
Comment 1 2009-07-21 10:24:47 PDT
Note: The underlying "URL" input type and associated parser changes were made on bug 25554, so this is only about validation.
Michelangelo De Simone
Comment 2 2009-08-21 06:53:51 PDT
AFAIK the fastest and most synthetic way to check for email validity is to make use of regexp. My intention is to divide this task into two separate patches: Part 1: regular validation check on type=email input value Part 2: same check when the "multiple" attribute is specified.
Michelangelo De Simone
Comment 3 2009-08-24 10:23:33 PDT
Created attachment 38484 [details] Patch v1
Michelangelo De Simone
Comment 4 2009-08-24 10:25:41 PDT
Created attachment 38485 [details] Patch v1a
Michelangelo De Simone
Comment 5 2009-08-24 10:32:10 PDT
Created attachment 38487 [details] Patch v1, Changelogs were missing
Eric Seidel (no email)
Comment 6 2009-08-24 13:36:51 PDT
Comment on attachment 38487 [details] Patch v1, Changelogs were missing I think the test results could be made much clearer by using a little pass/fail wrapper function: function testEmail(value, expectedValid) { i.value = "someone@somewhere.com"; shouldBe("v.typeMismatch", "false"); var didPass = v.typeMismatch == expectedValid; var validText = (expectedValue ? !didPass : didPass) ? "an invalid" : "a valid"; var resultText = "" + value + " is " + validText + " email address" if (didPass) testPassed(resultText); else testFailed(resultText); }
Eric Seidel (no email)
Comment 7 2009-08-25 10:43:17 PDT
Comment on attachment 38487 [details] Patch v1, Changelogs were missing Maybe we could also break: 31 #define RFC5322_DOTATOMTEXT "[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*" up into parts. It's kinda confusing to read as-is. r- for the tests being confusing.
Michelangelo De Simone
Comment 8 2009-08-26 12:32:00 PDT
Created attachment 38631 [details] Patch rev.2 Both Eric's comments have been addressed.
Peter Kasting
Comment 9 2009-08-28 15:23:44 PDT
Comment on attachment 38631 [details] Patch rev.2 > +bool ValidityState::isValidEmailAddress(String& email) > +{ > + if (email.isEmpty()) > + return false; > + > + DEFINE_STATIC_LOCAL(String, dotAtomText, (RFC5322_ATEXT)); > + dotAtomText.append("+(?:\\."); > + dotAtomText.append(RFC5322_ATEXT); > + dotAtomText.append("+)*"); Wait a minute, won't this append these on every call, making the regex longer and longer? What about just #define RFC5322_ATEXT "[a-z0-9!#$%&'*+/=?^_`{|}~-]" #define RFC5322_DOTATOMTEXT RFC5322_ATEXT "+(?:\\." RFC5322_ATEXT "+)*" ... DEFINE_STATIC_LOCAL(AtomicString, emailPattern, RFC5322_DOTATOMTEXT "@" RFC5322_DOTATOMTEXT);
Michelangelo De Simone
Comment 10 2009-08-28 16:03:11 PDT
Created attachment 38759 [details] Patch rev.2a Corrected, thanks to Peter.
Peter Kasting
Comment 11 2009-08-31 00:02:25 PDT
Note that Ian has just changed the spec as to what the acceptance criteria are.
Michelangelo De Simone
Comment 12 2009-08-31 10:52:29 PDT
Comment on attachment 38759 [details] Patch rev.2a Canceling r? due to specs change (4.10.4.1.5); a valid email address is in format 1*( atext / "." ) "@" ldh-str 1*( "." ldh-str )
Michelangelo De Simone
Comment 13 2009-09-02 16:12:28 PDT
(In reply to comment #12) > Canceling r? due to specs change (4.10.4.1.5); a valid email address is in > format 1*( atext / "." ) "@" ldh-str 1*( "." ldh-str ) I'm gonna publish a patch that makes use of the following regexp to match Ian's modifications to specs: [a-z0-9!#$%&'*+/=?^_`{|}~-](?.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)* @ [a-z0-9-]+(?.[a-z0-9-])* It's obviously quite "verbose" but, for the sake of clearness...
Kent Tamura
Comment 14 2009-09-10 18:34:46 PDT
(In reply to comment #13) > (In reply to comment #12) > > > Canceling r? due to specs change (4.10.4.1.5); a valid email address is in > > format 1*( atext / "." ) "@" ldh-str 1*( "." ldh-str ) > > I'm gonna publish a patch that makes use of the following regexp to match Ian's > modifications to specs: > > [a-z0-9!#$%&'*+/=?^_`{|}~-](?.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)* @ > [a-z0-9-]+(?.[a-z0-9-])* > > It's obviously quite "verbose" but, for the sake of clearness... It should be [a-z0-9!#$%&'*+/=?^_`{|}~-.]+@[a-z0-9-]+(?\.[a-z0-9-])+
Michelangelo De Simone
Comment 15 2009-09-14 07:09:12 PDT
(In reply to comment #14) > It should be > [a-z0-9!#$%&'*+/=?^_`{|}~-.]+@[a-z0-9-]+(?\.[a-z0-9-])+ This seems slightly incorrect.:) 1*( atext / "." ) = [a-z0-9!#$%&'*+/=?^_`{|}~.-]+ ldh-str 1*( "." ldh-str ) = [a-z0-9-]+(.[a-z0-9-]+)* [a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(.[a-z0-9-]+)*
Kent Tamura
Comment 16 2009-09-14 08:35:33 PDT
(In reply to comment #15) > 1*( atext / "." ) = [a-z0-9!#$%&'*+/=?^_`{|}~.-]+ > ldh-str 1*( "." ldh-str ) = [a-z0-9-]+(.[a-z0-9-]+)* > > [a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(.[a-z0-9-]+)* The last '*' should be '+'. We need at least 1 dot in a domain part.
Michelangelo De Simone
Comment 17 2009-09-15 08:37:28 PDT
Created attachment 39602 [details] Patch rev. 2b
Adam Barth
Comment 18 2009-10-13 13:34:25 PDT
Comment on attachment 39602 [details] Patch rev. 2b This looks good. I'm not 100% sure the regexp is right, but we can tweak that later if we're not matching HTML5.
WebKit Commit Bot
Comment 19 2009-10-13 13:53:20 PDT
Comment on attachment 39602 [details] Patch rev. 2b Clearing flags on attachment: 39602 Committed r49508: <http://trac.webkit.org/changeset/49508>
WebKit Commit Bot
Comment 20 2009-10-13 13:53:26 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.