Typing zero into certain fields for multiple field input will turn it into 1. Typing zero in 12 hour field will set 12 can move to the next field so you can't type 09 to enter 9 o clock.
Created attachment 180285 [details] Patch
Comment on attachment 180285 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180285&action=review > Source/WebCore/ChangeLog:13 > + Could you write new behavior what this patch does? > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:136 > + if (charCode < '0' || charCode > '9') We want to handle localized digit. We have been using convertFromLocalizedNumber(). > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:-169 > - m_lastDigitCharTime = 0; Can we remove this reset timer statement? Please try "1" + Backspace + "7". It should be "17". > Source/WebCore/html/shadow/DateTimeNumericFieldElement.h:99 > + mutable StringBuilder m_typeAheadBuffer; It seems we don't need to have "mutable" for m_typeAheadBuffer.
Comment on attachment 180285 [details] Patch Attachment 180285 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15411869 New failing tests: fast/forms/month-multiple-fields/month-multiple-fields-keyboard-events.html fast/forms/time-multiple-fields/time-multiple-fields-keyboard-events.html fast/forms/week-multiple-fields/week-multiple-fields-ax-value-changed-notification.html fast/forms/month-multiple-fields/month-multiple-fields-ax-value-changed-notification.html fast/forms/time-multiple-fields/time-multiple-fields-ax-value-changed-notification.html fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-keyboard-events.html fast/forms/date-multiple-fields/date-multiple-fields-keyboard-events.html fast/forms/week-multiple-fields/week-multiple-fields-keyboard-events.html
Created attachment 180300 [details] Patch
(In reply to comment #2) > (From update of attachment 180285 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=180285&action=review > > > Source/WebCore/ChangeLog:13 > > + > > Could you write new behavior what this patch does? Done. > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:136 > > + if (charCode < '0' || charCode > '9') > > We want to handle localized digit. We have been using convertFromLocalizedNumber(). Done. > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:-169 > > - m_lastDigitCharTime = 0; > > Can we remove this reset timer statement? Please try "1" + Backspace + "7". It should be "17". Fixed. > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.h:99 > > + mutable StringBuilder m_typeAheadBuffer; > > It seems we don't need to have "mutable" for m_typeAheadBuffer. StringBuilder::toString is not a const and DateTimeNumericFieldElement::visibleValue() is const
(In reply to comment #5) > (In reply to comment #2) > > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.h:99 > > > + mutable StringBuilder m_typeAheadBuffer; > > > > It seems we don't need to have "mutable" for m_typeAheadBuffer. > StringBuilder::toString is not a const and DateTimeNumericFieldElement::visibleValue() is const If so, this function should not be "const" and name could be visualValueAndResetTypeAhead(). If we have StringBuilder::toStringWithoutReseting() or so, we can split this function into visbleValue() and resetTypeAhead().
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #2) > > > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.h:99 > > > > + mutable StringBuilder m_typeAheadBuffer; > > > > > > It seems we don't need to have "mutable" for m_typeAheadBuffer. > > StringBuilder::toString is not a const and DateTimeNumericFieldElement::visibleValue() is const > > If so, this function should not be "const" and name could be visualValueAndResetTypeAhead(). > > If we have StringBuilder::toStringWithoutReseting() or so, we can split this function into visbleValue() and resetTypeAhead(). visibleValue() isn't reseting the type ahead buffer. I think StringBuilder is just reallocating a new buffer when we call toString().
Comment on attachment 180300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180300&action=review > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:113 > + int value = typeAheadValue(); > + m_typeAheadBuffer.clear(); > + if (value >= 0) > + setValueAsInteger(value, DispatchEvent); If the field is for 1-12h and m_typeAheadBuffer is "0", this code will set "0". I don't think it's a correct behavior. Dispatching events at this timing looks dangerous. But I think it's ok because an Event object for this 'blur' event has a RefPtr for this. > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:225 > + if (m_typeAheadBuffer.length() > 0) { > + bool ok = false; > + return m_typeAheadBuffer.toString().toInt(&ok); .length() never be negative. It should be if (m_typeAheadBuffer.length()) The variable 'ok' is not read. It should be omitted. return m_typeAheadBuffer.toString().toInt();
(In reply to comment #8) > (From update of attachment 180300 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=180300&action=review > > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:113 > > + int value = typeAheadValue(); > > + m_typeAheadBuffer.clear(); > > + if (value >= 0) > > + setValueAsInteger(value, DispatchEvent); > > If the field is for 1-12h and m_typeAheadBuffer is "0", this code will set "0". I don't think it's a correct behavior. This was my intension. Typing "0" shows "00" but doesn't set the value. When blur occurs, setValueAsInteger(0) is called which will set "12" for a 1-12h field and "01" for a month field. > Dispatching events at this timing looks dangerous. But I think it's ok because an Event object for this 'blur' event has a RefPtr for this. I'm not sure but I think its ok. > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:225 > > + if (m_typeAheadBuffer.length() > 0) { > > + bool ok = false; > > + return m_typeAheadBuffer.toString().toInt(&ok); > > .length() never be negative. It should be > if (m_typeAheadBuffer.length()) Fixed. > The variable 'ok' is not read. It should be omitted. > return m_typeAheadBuffer.toString().toInt(); Fixed.
Created attachment 180467 [details] Patch
Comment on attachment 180300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180300&action=review >>> Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:113 >>> + setValueAsInteger(value, DispatchEvent); >> >> If the field is for 1-12h and m_typeAheadBuffer is "0", this code will set "0". I don't think it's a correct behavior. >> >> Dispatching events at this timing looks dangerous. But I think it's ok because an Event object for this 'blur' event has a RefPtr for this. > > This was my intension. Typing "0" shows "00" but doesn't set the value. When blur occurs, setValueAsInteger(0) is called which will set "12" for a 1-12h field and "01" for a month field. ok, this behavior is not the main issue of this bug and is compatible with the current behavior. Let's discuss this later.
Comment on attachment 180467 [details] Patch ok
Comment on attachment 180467 [details] Patch Clearing flags on attachment: 180467 Committed r138365: <http://trac.webkit.org/changeset/138365>
All reviewed patches have been landed. Closing bug.