To share code among date time related input types, date, datetime, month, time and week, we should move multiple fields related functions to BaseDateAndTimeInputType class from TimeInputType class.
Created attachment 165736 [details] Patch 1
Comment on attachment 165736 [details] Patch 1 Could you review this patch? Thanks in advance. Bug 97299 has an implementation of multiple fields "month" input UI.
Comment on attachment 165736 [details] Patch 1 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".
Created attachment 165916 [details] Patch 2
Comment on attachment 165916 [details] Patch 2 Could you review this patch? Thanks in advance. = Changes since the last review = * Move BaseMultipleFiledsDateAndTimeInputType class to own file.
Comment on attachment 165916 [details] Patch 2 Attachment 165916 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14036636
Comment on attachment 165916 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=165916&action=review > Source/WebCore/ChangeLog:9 > + This patch introduces new class BaseMultipleFieldsDateAndTimeInputType > + for sharing code among multiple fields date/time input UI. nit: extra space before 'for'
Created attachment 165922 [details] Patch 3
Comment on attachment 165922 [details] Patch 3 Could you review this patch? Thanks in advance. = Changes since the last review = * Make BaseMultipleFieldsDateAndTimeInputType::formatDateTimeFieldsState() and setupLayoutParameters() pure virtual ** Fix compilation error and no need to have default functions * Update ChangeLog
Comment on attachment 165922 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=165922&action=review > Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:234 > + TextFieldInputType::updateInnerTextValue(); Why did you add the TextFieldInputType::updateInnerTextValue() call? It looks unnecessary and confusing.
Created attachment 165931 [details] Patch 4
Comment on attachment 165931 [details] Patch 4 Could you review this patch? Thanks in advance. = Changes since the last review = * Remove call of TextFieldInputType::updateInnerTextValue() in BaseMultipleFieldsDateAndTimeInputType::updateInnerTextValue()
Comment on attachment 165931 [details] Patch 4 ok
Comment on attachment 165931 [details] Patch 4 Clearing flags on attachment: 165931 Committed r129729: <http://trac.webkit.org/changeset/129729>
All reviewed patches have been landed. Closing bug.