[webkit-reviews] review denied: [Bug 97521] [Forms] Move multiple fields related functions to BaseDateAndTimeInputType from TimeInputType : [Attachment 165736] Patch 1

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 97521: [Forms] Move multiple fields related functions to
BaseDateAndTimeInputType from TimeInputType
https://bugs.webkit.org/show_bug.cgi?id=97521

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

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


The direction of the change looks good.

> Source/WebCore/html/BaseDateAndTimeInputType.h:74
> +class BaseMultipleFieldsDateAndTimeInputType : public
BaseDateAndTimeInputType {

This class should have its own source files because it is large.

> Source/WebCore/html/BaseDateAndTimeInputType.h:82
> +    class DateTimeEditControlOwnerImpl : public
DateTimeEditElement::EditControlOwner {

nit:
We may add DateTimeEditElement::EditControlOwner interface to
BaseMultipleFieldsDateAndTimeInputType class directly.
You can change so in another patch.

> Source/WebCore/html/TimeInputType.h:43
> +typedef BaseMultipleFieldsDateAndTimeInputType BaseTimeInputType;
> +#else
> +typedef BaseDateAndTimeInputType BaseTimeInputType;

This should be in BaseMultipleFieldsDateAndTimeInputType.h because this pattern
will be used in other date/time input types.
Of course we should rename BaseTimeInputType so that it doesn't have "Time".