Bug 104985

Summary:

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

Product:

WebKit

Reporter:

Kunihiko Sakamoto <ksakamoto>

Component:

Forms

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ojan.autocc, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

All

Attachments:

Description	Flags
Patch	none
Patch 2	none

Kunihiko Sakamoto

Reported 2012-12-13 18:25:01 PST

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

Attachments
Patch (22.12 KB, patch) 2012-12-13 19:15 PST, Kunihiko Sakamoto	no flags	Details Formatted Diff Diff
Patch 2 (27.87 KB, patch) 2012-12-13 20:12 PST, Kunihiko Sakamoto	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Kunihiko Sakamoto

Comment 1 2012-12-13 19:15:36 PST

Created attachment 179402 [details] Patch

Kent Tamura

Comment 2 2012-12-13 19:35:29 PST

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

Comment 3 2012-12-13 20:12:06 PST

Created attachment 179406 [details] Patch 2

Kunihiko Sakamoto

Comment 4 2012-12-13 20:12:44 PST

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 5 2012-12-13 20:15:54 PST

Comment on attachment 179406 [details] Patch 2 Looks good!

WebKit Review Bot

Comment 6 2012-12-13 20:57:24 PST

Comment on attachment 179406 [details] Patch 2 Clearing flags on attachment: 179406 Committed r137712: <http://trac.webkit.org/changeset/137712>

WebKit Review Bot

Comment 7 2012-12-13 20:57:28 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.