[webkit-reviews] review requested: [Bug 29004] [HTML5][Forms] simple implementation of date&time types of INPUT element : [Attachment 42918] Patch part 1: recognize date&time types as text fields (rev.6)

[bugzilla-daemon at webkit.org]

TAMURA, Kent <tkent at chromium.org> has asked  for review:
Bug 29004: [HTML5][Forms] simple implementation of date&time types of INPUT
element
https://bugs.webkit.org/show_bug.cgi?id=29004

Attachment 42918: Patch part 1: recognize date&time types as text fields
(rev.6)
https://bugs.webkit.org/attachment.cgi?id=42918&action=review

------- Additional Comments from TAMURA, Kent <tkent at chromium.org>
(In reply to comment #31)
> It seems your test case only needs to test the casing once.  once it's
> confirmed that button and BUTTON both map to BUTTON, you don't really need to

> check CHECKBOX, COLOR, EMail, etc.  Just the lowercase versions should be
> enough and might make the test less confusing.

Done.

> I suspect the dom spec says nothing about the implementation, but it does say

> that input.type has to be lowercase, right?  Would be more clear to say
> something like:
> // input.type must return a lower case value according to DOM3.

Done.

> I dont understand this comment:
>   // We don't use formControlTypes->get(inputType()) because this function
>  542	   // tries to return a reference to a temporary AtomicString object in

> that
>  543	   // case.

I updated that comment.  I hope it get clearer.
If we write "return formControlTypes->get(inputType());", gcc in Xcode warns
it:
WebKit/WebCore/html/HTMLInputElement.cpp:547: warning: returning reference to
temporary


> This comment isn't clear:
>  509	   // Needs to be lowercase according to DOM spec

Improved.