Bug 104985 – Seconds/Minutes field of date/time input UI should respect step attribute

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kunihiko Sakamoto]

Continued from https://bugs.webkit.org/show_bug.cgi?id=104628.
Make seconds/minutes fields of multiple-fields UI respect step attribute.

[Kunihiko Sakamoto]

Created attachment 179402 [details]
Patch

[Kent Tamura]

Comment on attachment 179402 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179402&action=review

The logic looks good. I have one comment.

> Source/WebCore/html/shadow/DateTimeEditElement.cpp:155
> +        int step, stepBase;
> +        getFieldStep(static_cast<int>(msPerMinute), static_cast<int>(msPerHour), step, stepBase);
> +        RefPtr<DateTimeNumericFieldElement> field = DateTimeMinuteFieldElement::create(document, m_editElement, step, stepBase);
>          m_editElement.addField(field);

I'd like to recommend to introduce DateTimeNumericFieldElement::Parameters, which contains step and stepBase.  If so, getFieldStep would be "DateTimeNumericFieldElement::Parameters createNumericFieldParameters(const Decimal&, const Decima&)", and field constructors would receive "const DateTimeNumericFieldElement::Parameters&".

[Kunihiko Sakamoto]

Created attachment 179406 [details]
Patch 2

[Kunihiko Sakamoto]

Comment on attachment 179402 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179402&action=review

>> Source/WebCore/html/shadow/DateTimeEditElement.cpp:155
>>          m_editElement.addField(field);
> 
> I'd like to recommend to introduce DateTimeNumericFieldElement::Parameters, which contains step and stepBase.  If so, getFieldStep would be "DateTimeNumericFieldElement::Parameters createNumericFieldParameters(const Decimal&, const Decima&)", and field constructors would receive "const DateTimeNumericFieldElement::Parameters&".

Thanks for the suggestion.
Done.

[Kent Tamura]

Comment on attachment 179406 [details]
Patch 2

Looks good!

[WebKit Review Bot]

Comment on attachment 179406 [details]
Patch 2

Clearing flags on attachment: 179406

Committed r137712: <http://trac.webkit.org/changeset/137712>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.