Increment and decrement the value of the current focused field, in response to pressing the up and down arrow keys respectively.
Created attachment 407795 [details] Patch
Comment on attachment 407795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407795&action=review > Source/WebCore/html/shadow/DateTimeFieldElement.cpp:102 > + if (key == "Up") { > + keyboardEvent.setDefaultHandled(); > + stepUp(); > + return; > + } > + > + if (key == "Down") { > + keyboardEvent.setDefaultHandled(); > + stepDown(); > + return; > + } Code above calls setDefaultHandled after handling. This code and the code below calls it before handling. I suggest we consider a consistent pattern of doing setDefaultHandled last. > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:102 > + m_typeAheadBuffer.clear(); > + setValueAsInteger(newValue, DispatchInputAndChangeEvents); Would be nice to share this between up and down. I know it’s just two lines of code. > Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:82 > + int newValue = hasValue() ? m_selectedIndex - 1 : m_symbols.size() - 1; > + if (newValue < 0) > + newValue = m_symbols.size() - 1; Can m_symbols be empty? > Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:89 > + if (newValue == static_cast<int>(m_symbols.size())) Nicer to use >=
Created attachment 407882 [details] Patch for landing
(In reply to Darin Adler from comment #2) > Comment on attachment 407795 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407795&action=review > > > Source/WebCore/html/shadow/DateTimeFieldElement.cpp:102 > > + if (key == "Up") { > > + keyboardEvent.setDefaultHandled(); > > + stepUp(); > > + return; > > + } > > + > > + if (key == "Down") { > > + keyboardEvent.setDefaultHandled(); > > + stepDown(); > > + return; > > + } > > Code above calls setDefaultHandled after handling. This code and the code > below calls it before handling. I suggest we consider a consistent pattern > of doing setDefaultHandled last. Updated to call setDefaultHandled last. > > Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:102 > > + m_typeAheadBuffer.clear(); > > + setValueAsInteger(newValue, DispatchInputAndChangeEvents); > > Would be nice to share this between up and down. I know it’s just two lines > of code. Added a method: setValueAsIntegerByStepping. > > Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:82 > > + int newValue = hasValue() ? m_selectedIndex - 1 : m_symbols.size() - 1; > > + if (newValue < 0) > > + newValue = m_symbols.size() - 1; > > Can m_symbols be empty? It cannot. There is an assertion in the constructor and m_symbols is never modified. > > Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:89 > > + if (newValue == static_cast<int>(m_symbols.size())) > > Nicer to use >= Done.
Committed r266524: <https://trac.webkit.org/changeset/266524> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407882 [details].
<rdar://problem/68283770>