[Webkit-unassigned] [Bug 106212] INPUT_MULTIPLE_FIELDS_UI: Step-up/-down of month/day field should respect min/max attributes
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 7 21:30:00 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=106212
--- Comment #5 from Kunihiko Sakamoto <ksakamoto at chromium.org> 2013-01-07 21:31:54 PST ---
(From update of attachment 181487)
Thanks for the review.
All done, please take another look.
View in context: https://bugs.webkit.org/attachment.cgi?id=181487&action=review
>> Source/WebCore/html/shadow/DateTimeEditElement.cpp:221
>> + RefPtr<DateTimeFieldElement> field;
>
> There are a lot of duplicated code in the FieldTypeMonth case and the FieldTypeMonthStandAlone case. We had better merge them.
> e.g. ...::create(document, m_editElement, fieldType == DateTimeFormat::FieldTypeMonth ? m_parameters.locale.shortMonthLabels() : m_parameters.locale.shortStandAloneMonthLabels(), minMonth, maxMonth);
Done.
>> Source/WebCore/html/shadow/DateTimeFieldElements.h:64
>> + // DateTimeNumericFieldElement functions.
>
> functions -> function
Done.
>> Source/WebCore/html/shadow/DateTimeFieldElements.h:137
>> + // DateTimeNumericFieldElement functions.
>
> Ditto.
Done.
>> Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.h:77
>> + const int m_maxIndex;
>
> should not be abbreviations.
> m_minIndex -> m_minimumIndex
> m_maxIndex -> m_maximumIndex
Done.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-readonly-subfield.html:49
>> +shouldBeFalse('isReadOnlyField(createDateInput("2012-12-01", "2013-12-31", ""), pseudoMonth)');
>
> should add out-of-range value case such as createDateInput("2012-12-01", "2012-12-31", "2012-11-30").
Done.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-readonly-subfield.html:59
>> +shouldBeFalse('isReadOnlyField(createDateInput("2012-12-17", "2013-12-17", ""), pseudoDay)');
>
> Ditto.
Done.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-stepup-stepdown-from-renderer.html:11
>> + internals.settings.setLangAttributeAwareFormControlUIEnabled(true);
>
> Looks unnecessary.
Removed.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-stepup-stepdown-from-renderer.html:20
>> +{
>
> Style is inconsistent. You put "{" on the same line as "function" in other functions.
Done.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-stepup-stepdown-from-renderer.html:22
>> + return;
>
> You had better warn the lack of eventSender at the beginning of the test rather than ignoring eventSender.keyDown.
> We sometimes load a layout test with a real browser, and if the test doesn't work on the browser, we need to investigate the reason.
Done.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-stepup-stepdown-from-renderer.html:27
>> + input.value = input.min = input.max = null;
>
> Is this needed?
Removed.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-stepup-stepdown-from-renderer.html:58
>> +shouldBeEqualToString('test("2000-05-01", null, null, ["delete", "downArrow"])', '2000-12-01');
>
> They look duplications of date-multiple-fields-keyboard-events.html.
> We should remove them from this test, or merge everything to date-multiple-fields-keyboard-events.html.
Ah, I see.
Removed the test cases with min=max=null from this file.
>> LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-stepup-stepdown-from-renderer.html:116
>> +document.body.removeChild(input);
>
> input.remove() is enough.
Done.
>> LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-readonly-subfield-expected.txt:24
>> +PASS isReadOnlyField(createMonthInput("2012-12", "2013-12", ""), pseudoMonth) is false
>
> should add out-of-range month value cases.
Done.
--
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