EmailInputType::convertEmailAddressToASCII() should not try to convert non-email addresses.

EmailInputType::convertEmailAddressToASCII() should not try to convert non-email addresses.

It seems ICU produces broken strings if an input string is not in host name
format. EmailInputType::convertEmailAddressToASCII() should return the input
string as is the resultant string is not valid.

BUG=551551
Committed: https://crrev.com/453bf5082bba44aba93b428387e0e112c4db0dd3
Cr-Commit-Position: refs/heads/master@{#358779}

[tkent, 2015-11-09 05:43:53]

tkent@chromium.org changed reviewers:
+ keishi@chromium.org

[tkent, 2015-11-09 05:43:54]

keishi, would you review this please?

[keishi, 2015-11-09 05:53:40]

LGTM

https://codereview.chromium.org/1408833006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/forms/EmailInputType.cpp (right):

https://codereview.chromium.org/1408833006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/forms/EmailInputType.cpp:69: if
(host.find('@') != kNotFound || host.find(isASCIISpace) != kNotFound)
nit: maybe we should be rejecting if there are multiple "@"

[tkent, 2015-11-09 05:56:36]

https://codereview.chromium.org/1408833006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/forms/EmailInputType.cpp (right):

https://codereview.chromium.org/1408833006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/forms/EmailInputType.cpp:69: if
(host.find('@') != kNotFound || host.find(isASCIISpace) != kNotFound)
On 2015/11/09 at 05:53:40, keishi wrote:
> nit: maybe we should be rejecting if there are multiple "@"

This rejects one or more "@"s.

[keishi, 2015-11-09 06:02:08]

https://codereview.chromium.org/1408833006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/forms/EmailInputType.cpp (right):

https://codereview.chromium.org/1408833006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/forms/EmailInputType.cpp:69: if
(host.find('@') != kNotFound || host.find(isASCIISpace) != kNotFound)
On 2015/11/09 05:56:35, tkent wrote:
> On 2015/11/09 at 05:53:40, keishi wrote:
> > nit: maybe we should be rejecting if there are multiple "@"
> 
> This rejects one or more "@"s.

Oh right, because its searching host.

[tkent, 2015-11-09 23:45:10]

Description was changed from

==========
EmailInputType::convertEmailAddressToASCII() should not try to convert non-email
addresses.

It seems ICU produces broken strings if an input string is not in host name
format. EmailInputType::convertEmailAddressToASCII() rejects obviously wrong
strings to avoid it.

BUG=551551
==========

to

==========
EmailInputType::convertEmailAddressToASCII() should not try to convert non-email
addresses.

It seems ICU produces broken strings if an input string is not in host name
format. EmailInputType::convertEmailAddressToASCII() should return the input
string as is the resultant string is not valid.

BUG=551551
==========

[tkent, 2015-11-09 23:47:56]

I changed the approach a little bit.  convertEmailAdressToASCII() uses
isValidEmailAddress() in Patch Set 2.

[tkent, 2015-11-10 01:02:56]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-11-10 01:02:57]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org
Link to the patchset: https://codereview.chromium.org/1408833006/#ps20001
(title: "Use isValidEmailAddress() instead of ad-hoc check")

[commit-bot: I haz the power, 2015-11-10 01:08:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1408833006/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1408833006/20001

[commit-bot: I haz the power, 2015-11-10 03:25:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-10 03:26:01]

Patchset 2 (id:??) landed as
https://crrev.com/453bf5082bba44aba93b428387e0e112c4db0dd3
Cr-Commit-Position: refs/heads/master@{#358779}