[webkit-reviews] review denied: [Bug 59752] Allow Localized Date Strings for Date Input Fields : [Attachment 91632] [PATCH] Add LocalizedDate Allow Formatting + Parsing

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 29 15:58:52 PDT 2011


Kent Tamura <tkent at chromium.org> has denied Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 59752: Allow Localized Date Strings for Date Input Fields
https://bugs.webkit.org/show_bug.cgi?id=59752

Attachment 91632: [PATCH] Add LocalizedDate Allow Formatting + Parsing
https://bugs.webkit.org/attachment.cgi?id=91632&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91632&action=review

> Source/WebCore/ChangeLog:27
> +	   (WebCore::BaseDateAndTimeInputType::serialize):
> +	   (WebCore::BaseDateAndTimeInputType::serializeWithComponents):
> +	   serializing for a double value may be different for different
> +	   types. The month type assumes it is monthes instead of msecs
> +	   like the others. So provide a more general serialization
> +	   function that will always serialize a string correctly for
> +	   the current type but from a DateComponents object. Useful
> +	   when we create our own DateComponents object from the msec
> +	   value returned from a LocalizedDate implementation.

I don't think this change is needed. 
BaseDateAndTimeInputType::serialize(double) already handle the month type
correctly.

> Source/WebCore/html/BaseDateAndTimeInputType.cpp:208
> +    if (!date.setMillisecondsSinceEpochForDateTime(parsedValue))

This is wrong.	It sets DateTime type to the DateComponents object.  I think
serialize(parsedValue) is enough.


More information about the webkit-reviews mailing list