[webkit-reviews] review requested: [Bug 29004] [HTML5][Forms] simple implementation of date&time types of INPUT element : [Attachment 43216] Recognize date&time types as text fields (rev.8)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Nov 13 17:30:04 PST 2009
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 43216: Recognize date&time types as text fields (rev.8)
https://bugs.webkit.org/attachment.cgi?id=43216&action=review
------- Additional Comments from TAMURA, Kent <tkent at chromium.org>
Thank you for reviewing!
(In reply to comment #38)
> > + if (!t.isNull() && typeMap->contains(t))
> > + newType = typeMap->get(t);
> > else
> > newType = TEXT;
>
> It's not a good idiom for performance to do a contains followed by a separate
> get. Those are two completely separate hash table lookups. There are two
> alternatives:
>
> 1) Just call get without first checking contains. The result will be the
> default value, 0, which is TEXT
Done.
> I also think the comment is a little strange because there are tons of call
> sites for this and I don't think we want to add a comment like this at every
> one.
Removed the comment.
> > + types->add(HTMLInputElement::URL, "url");
> > + types->add(HTMLInputElement::WEEK, "week");
>
> These values are enum values, consecutive integers. This should be done with
an
> array, not a HashMap.
Changed it to AtomicString[].
More information about the webkit-reviews
mailing list