627657 - Allow domain name without dot in <input type=email>

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This just changed in the spec, shouldn't be hard to fix.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Since HTML forms are quite the focus for 2.0, I think this should block.

Comment 2

[Mounir Lamouri (:mounir)]

Should that block? Will a patch be taken at least?

Comment 3

[Mounir Lamouri (:mounir)]

(In reply to comment #1)
> Since HTML forms are quite the focus for 2.0, I think this should block.

Unfortunately, I don't think HTML5 Forms are the focus of this release.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yeah, don't think this blocks. But if a patch looks safe enough I suspect we'd take it.

But of course, if you can rather work on a blocker, that is time better spent.

Comment 5

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Jonas, I think the patch is pretty simple. I'm sure you want to r+a :)

Comment 6

[Mounir Lamouri (:mounir)]

CCing Olli in case of he has time to steal this review ;)

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 505804 [details] [diff] [review]
Patch v1

># HG changeset patch
># Parent 3639ce82bd7f4a370e90f997343830ca2a4363ee
># User Mounir Lamouri <[e-mail]>
># Date 1295626774 -3600
>
>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>--- a/content/html/content/src/nsHTMLInputElement.cpp
>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>@@ -4112,38 +4112,33 @@ nsHTMLInputElement::IsValidEmailAddress(
>     return PR_FALSE;
>   }
> 
>   // The domain name can't begin with a dot.
>   if (aValue[i] == '.') {
>     return PR_FALSE;
>   }
> 
>-  // The domain name must have at least one dot which can't follow another dot,
>-  // can't be the first nor the last domain name character.
>-  PRBool dotFound = PR_FALSE;
>-
>   // Parsing the domain name.
>   for (; i < length; ++i) {
>     PRUnichar c = aValue[i];
> 
>     if (c == '.') {
>-      dotFound = PR_TRUE;
>       // A dot can't follow a dot.
>       if (aValue[i-1] == '.') {
>         return PR_FALSE;
>       }
>     } else if (!(nsCRT::IsAsciiAlpha(c) || nsCRT::IsAsciiDigit(c) ||
>                  c == '-')) {
>       // The domain characters have to be in this list to be valid.
>       return PR_FALSE;
>     }
>   }
> 
>-  return dotFound;
>+  return PR_TRUE;
> }
> 
> //static
> PRBool
> nsHTMLInputElement::IsPatternMatching(nsAString& aValue, nsAString& aPattern,
>                                       nsIDocument* aDocument)
> {
>   NS_ASSERTION(aDocument, "aDocument should be a valid pointer (not null)");
>diff --git a/content/html/content/test/test_bug555559.html b/content/html/content/test/test_bug555559.html
>--- a/content/html/content/test/test_bug555559.html
>+++ b/content/html/content/test/test_bug555559.html
>@@ -36,31 +36,31 @@ function invalidEventHandler(e)
>   is(e.type, "invalid", "Invalid event type should be invalid");
>   gInvalid = true;
> }
> 
> function checkValidEmailAddress(element)
> {
>   gInvalid = false;
>   ok(!element.validity.typeMismatch,
>-     "Element should not suffer from type mismatch");
>+     "Element should not suffer from type mismatch (with value='"+element.value+"')");
>   ok(element.validity.valid, "Element should be valid");
>   ok(element.checkValidity(), "Element should be valid");
>   ok(!gInvalid, "The invalid event should not have been thrown");
>   is(element.validationMessage, '',
>      "Validation message should be the empty string");
>   is(window.getComputedStyle(element, null).getPropertyValue('background-color'),
>      "rgb(0, 255, 0)", ":valid pseudo-class should apply");
> }
> 
> function checkInvalidEmailAddress(element)
> {
>   gInvalid = false;
>   ok(element.validity.typeMismatch,
>-     "Element should suffer from type mismatch");
>+     "Element should suffer from type mismatch (with value='"+element.value+"')");
>   ok(!element.validity.valid, "Element should not be valid");
>   ok(!element.checkValidity(), "Element should not be valid");
>   ok(gInvalid, "The invalid event should have been thrown");
>   is(element.validationMessage, "Please enter an email address.",
>      "Validation message is not valid");
>   is(window.getComputedStyle(element, null).getPropertyValue('background-color'),
>      "rgb(255, 0, 0)", ":invalid pseudo-class should apply");
> }
>@@ -116,20 +116,20 @@ var illegalCharacters = "()<>[]:;@\, \t"
> for each (c in illegalCharacters)
> {
>   email.value = c + '@bar.com';
>   checkInvalidEmailAddress(email);
> }
> 
> // Some checks on the domain part of the address.
> email.value = 'foo@bar';
>-checkInvalidEmailAddress(email);
>+checkValidEmailAddress(email);
> 
> email.value = 'foo@b';
>-checkInvalidEmailAddress(email);
>+checkValidEmailAddress(email);
> 
> email.value = 'foo@';
> checkInvalidEmailAddress(email);
> 
> email.value = 'foo@bar.';
> checkInvalidEmailAddress(email);
> 
> email.value = '[e-mail]';
>@@ -222,17 +222,17 @@ checkInvalidEmailAddress(email);
> 
> email.value = '[e-mail];[e-mail]';
> checkInvalidEmailAddress(email);
> 
> email.value = '<[e-mail]>, <[e-mail]>';
> checkInvalidEmailAddress(email);
> 
> email.value = 'foo@bar, [e-mail]';
>-checkInvalidEmailAddress(email);
>+checkValidEmailAddress(email);
> 
> email.value = '[e-mail], foo';
> checkInvalidEmailAddress(email);
> 
> email.value = 'foo, [e-mail]';
> checkInvalidEmailAddress(email);
> 
> </script>

Comment 8

[Mounir Lamouri (:mounir)]

Push:
http://hg.mozilla.org/mozilla-central/rev/d341b2ece4e4

Comment 9

[Daniel C]

This seems like it will cause many more false positives than the false negatives it is trying to fix.

I don't think changes to the spec should be made to accommodate a testing purpose such as @localhost, or the purely theoretical idea that someone might use example@tld as their e-mail address.

example@hotmail is likely to be more common as a mistake.