[webkit-reviews] review denied: [Bug 97997] [Forms] Multiple fields datetime/datetime-local input UI : [Attachment 166643] Patch 1

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 97997: [Forms] Multiple fields datetime/datetime-local input UI
https://bugs.webkit.org/show_bug.cgi?id=97997

Attachment 166643: Patch 1
https://bugs.webkit.org/attachment.cgi?id=166643&action=review

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


> Source/WebCore/ChangeLog:12
> +	   for multiple fields week input UI.

week -> datetime/datetime-local

> Source/WebCore/ChangeLog:18
> +	   Note: Actual outputs of four tests
> +	   - fast/forms/datetime/datetime-input-visible-string.html
> +	   - fast/forms/datetime/datetime-stepup-stepdown-from-renderer.html
> +	   - fast/forms/datetimelocal/datetimelocal-input-visible-string.html
> +	   -
fast/forms/datetimelocal/datetimelocal-stepup-stepdown-from-renderer.html

What do you mean by this paragraph?

> Source/WebCore/ChangeLog:33
> +	   (WebCore::DateTimeLocalInputType::formatDateTimeFieldsState): Added
to format numeric value to string value as specified in HTML5 specification.
> +	   (WebCore::DateTimeLocalInputType::setupLayoutParameters):  Added to
set layout of multiple fields.

These functions are very similar to DateTimeInputType functions.  Can you share
them?

> Source/WebCore/html/DateTimeInputType.cpp:148
> +    builder.append(layoutParameters.localizer.dateFormat());
> +    builder.append(' ');

Looks not correct.  AFAIK, some locales use ', ' for the delimiter, and some
locales need the reversed order.