-
Notifications
You must be signed in to change notification settings - Fork 27.5k
fix(input): use Chromium's email validation regexp #5924
Conversation
This change uses the regexp from Chromium/Blink to validate emails, and corrects an error in the validation engine, which previously considered an invalid email to be valid. Additionally, the regexp was invalidating emails with capital letters, however this is not the behaviour recomended in the spec, or implemented in Chromium. Closes angular#5899
lgtm |
thanks for the reference to the blink code |
Thanks for the quick review, I'll merge |
I noticed a bug in chromium's regexp. It accepts a domain part that begins or ends with a dash. |
Should file a bug on Blink for this, I think it would be best if we kept our validation in sync with their own, IMO. It looks like Firefox will consider a domain like this invalid, so it seems reasonable |
On the one hand, it is good to match the browser. On the other hand, it is good to be correct. I noticed that chromium also accepts domain parts that begin with numbers. :-( This is the correct implementation of the domain part.
I'll see about filing a bug on Blink and see if we can get both hands to agree. |
Can't say that I like their bug submission form, but I submitted to chromium. |
I've filed a bug on Chromium/Blink https://code.google.com/p/chromium/issues/detail?id=338141, I think we can fix it pretty easily in both products. If you feel like getting some code into Chromium you should take a stab at fixing this, it's pretty trivial. ... Oh, you submitted one too! well then ;) |
@KevinBrogan I've got a patch being reviewed to fix this (https://codereview.chromium.org/132483006/), want to submit a patch updating the code in Angular? It probably won't be more than a week or two before it gets merged, so we should try to keep them in sync. |
This change uses the regexp from Chromium/Blink to validate emails, and corrects an error in the validation engine, which previously considered an invalid email to be valid. Additionally, the regexp was invalidating emails with capital letters, however this is not the behaviour recomended in the spec, or implemented in Chromium.
(The regexp can be seen in Blink at https://github.com/mirrors/blink/blob/9d8fc44892f633b1448d0daa638969ffac497bc4/Source/core/html/forms/EmailInputType.cpp#L45-L48)
Closes #5899