Use Localizer::monthFormat to construct input[type=month] UI
Created attachment 169568 [details] Patch
Created attachment 169574 [details] Patch 2 more rebaseline
Comment on attachment 169568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169568&action=review > Source/WebCore/html/shadow/DateTimeFieldElements.cpp:399 > + dateTimeFieldsState.setMonth(hasValue() ? valueAsInteger() + 1 : DateTimeFieldsState::emptyValue); We might want an ASSERT that checks that the value is less than symbolsSize(). > Source/WebCore/html/shadow/DateTimeFieldElements.cpp:420 > + const unsigned value = dateTimeFieldsState.month() - 1; > + if (value < symbolsSize()) { > + setValueAsInteger(value); > + return; > + } > + > + setEmptyValue(dateForReadOnlyField); Nit: I would slightly prefer: if (value >= symbolsSize()) { setEmptyValue(dateForReadOnlyField); return; } setValueAsInteger(value); Just like the above 'if (!dateTimeFieldsState.hasMonth())' check, we can put an error case first.
Created attachment 169583 [details] Patch 3 follow reviewer's comments
Comment on attachment 169568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169568&action=review >> Source/WebCore/html/shadow/DateTimeFieldElements.cpp:399 >> + dateTimeFieldsState.setMonth(hasValue() ? valueAsInteger() + 1 : DateTimeFieldsState::emptyValue); > > We might want an ASSERT that checks that the value is less than symbolsSize(). Done. >> Source/WebCore/html/shadow/DateTimeFieldElements.cpp:420 >> + setEmptyValue(dateForReadOnlyField); > > Nit: I would slightly prefer: > > if (value >= symbolsSize()) { > setEmptyValue(dateForReadOnlyField); > return; > } > setValueAsInteger(value); > > Just like the above 'if (!dateTimeFieldsState.hasMonth())' check, we can put an error case first. Done.
Comment on attachment 169583 [details] Patch 3 Looks OK
Comment on attachment 169583 [details] Patch 3 Thanks!
Comment on attachment 169583 [details] Patch 3 Clearing flags on attachment: 169583 Committed r131898: <http://trac.webkit.org/changeset/131898>
All reviewed patches have been landed. Closing bug.