[Webkit-unassigned] [Bug 109555] INPUT_MULTIPLE_FIELDS_UI: Step-up/-down of hour field should respect min/max attributes

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 27 02:06:53 PST 2013


https://bugs.webkit.org/show_bug.cgi?id=109555





--- Comment #4 from Kunihiko Sakamoto <ksakamoto at chromium.org>  2013-02-27 02:09:15 PST ---
(From update of attachment 187813)
View in context: https://bugs.webkit.org/attachment.cgi?id=187813&action=review

>> Source/WebCore/html/shadow/DateTimeEditElement.cpp:161
>> +        RefPtr<DateTimeFieldElement> field = DateTimeHour11FieldElement::create(document, m_editElement, minimumHour23, maximumHour23, parameters);
> 
> How about introducing a subclass of DateTimeNumericFieldElement::Parameters to add minimum/maximum members?
> Repeating the same code four times looks not good.

Because minHour/maxHour are used in several places (e.g. am/pm field uses them too), I have made them data fields of DateTimeEditBuilder.
I think minimum and maximum should be included in DateTimeNumericFieldElement. I'd like to do it later.

>> Source/WebCore/html/shadow/DateTimeEditElement.cpp:163
>>          if (shouldHourFieldDisabled()) {
> 
> This patch doesn't disable the field in a case of minimumHour23 == maximumHour23.
> It's ok if you think it's out of scope.

Changed to disable Hour/AMPM fields when possible.

>> LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-stepup-stepdown-from-renderer.html:175
>> +shouldBeEqualToString('stepUp("15:00", 1, "12:00", "15:00")', '16:00');
> 
> We should have tests with
>  - max="12:00" 
>  - min="00:00"
>  - min is in the morning, and max is in the afternoon.  e.g. min="10:00" max="15:00"

Added.

>> LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-stepup-stepdown-from-renderer.html:182
>>  debug('Hours, 0-23');
> 
> I have a concern about test coverage. The test doesn't cover hour11 and hour24 cases.
> 
> It seems there are no locales with these hour formats in ICU and Windows. However we need support them because users can set these formats in OSX.
> Ideally, we had better introduce test-only function to set date/time format.  It would be something like window.internals.setDateTimeFormat(input, "kk:mm").

We added the pattern attribute to DateTimeEditElement in r143770; now we can test hour11/hour24.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list