[Webkit-unassigned] [Bug 55987] input type=email and multiple is not compliant

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=55987


Kentaro Hara <haraken at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #98685|1                           |0
        is obsolete|                            |




--- Comment #12 from Kentaro Hara <haraken at google.com>  2011-06-27 21:42:17 PST ---
(From update of attachment 98685)
View in context: https://bugs.webkit.org/attachment.cgi?id=98685&action=review

>> LayoutTests/fast/forms/ValidityState-typeMismatch-email-expected.txt:6
>> +PASS "something at something.com" is a correct valid email address. It was sanitized to "something at something.com".
> 
> The test result readability is not good.
> How about
>  - Remove 'correct' and 'incorrect'
>    We show 'PASS' or 'FAIL'.  It's enough.
>  - Show sanitized value only if the expected sanitized value is not identical to the input value or a sanitization test fails.

Done.

>> LayoutTests/fast/forms/resources/ValidityState-typeMismatch-email.js:21
>>  // VALID
> 
> It's good to print such headings.  You can
>   debug('Valid single addresses:');
> or something.

Done.

>> LayoutTests/fast/forms/resources/ValidityState-typeMismatch-email.js:22
>> +emailCheck("something at something.com", "something at something.com", false, false);
> 
> Boolean parameters are not good for readability.
> I'd like to do:
> 
> var expectValid = false;
> var expectInvalid = true;
> var multiple = true;
> 
> emailCheck("something at something.com", "something at something.com", expectValid); // We can omit the trailing false parameter.
> emailCheck(" a at p.com,b at p.com", "a at p.com,b at p.com", expectInvalid, multiple);

Done.

>> Source/JavaScriptCore/wtf/ASCIICType.h:147
>> +
> 
> These functions are HTML-specific, and not useful for JavaScriptCode.  So I think they should be in WebCore/html/parser/HTMLParserIdioms.{cpp,h}.

Moved them.

>> Source/JavaScriptCore/wtf/text/StringImpl.cpp:351
>> +    // skip white space from start
> 
> Usually comments should be sentences.  "// Skip white space from start."
> Anyway, this comment seems not helpful.  So remove this comment please.

I removed this method since I found that stripLeadingAndTrailingHTMLSpaces() is doing the same thing.

>> Source/JavaScriptCore/wtf/text/StringImpl.h:294
>> +    PassRefPtr<StringImpl> stripLineBreaks();
> 
> ditto. Move them to HTMLParserIdioms.

Done.

>> Source/WebCore/html/EmailInputType.cpp:97
>> +
> 
> It's ok to remove this function.

Done.

>> Source/WebCore/html/EmailInputType.cpp:111
>> +    return strippedValue;
> 
> You had better use wtf/textStirngBuilder.

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.