From cf93b63ca11642bc891570f9b9eef950777b01f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 2 Jul 2023 18:27:22 +0100 Subject: [PATCH] Relax checks on character entry in numeric validators We can't perform the check for the invalid number in IsCharOk() because it's perfectly normal for the number to be temporarily invalid while it's being entered and, worse, sometimes this can't be avoided at all and the existing behaviour prevented the user from entering _anything_ into a control limited to the values between 10 and 20. Do ensure that the control still has a correct value after losing focus. See #12968, #23561. (combined cherry pick of c269932c4e8ed0c0e4a2554b928ae2b55f70ad8e, dccddcdd59b9a11564b809c516b418356db21718 and 1c160557132f76423dbc0acb3a01b1f64f016dd1 from master) --- docs/changes.txt | 1 + include/wx/valnum.h | 24 ++++++++++++++++++++---- src/common/valnum.cpp | 22 ++++++++++++---------- tests/validators/valnum.cpp | 20 +++++++++++++++++++- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 50eb4876dc..1ca467f0b9 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -260,6 +260,7 @@ All (GUI): - Fix return value of wxAuiToolBarItem::GetDisabledBitmap() (taler21, #23666). - Fix buffer overflow in wxHTMLDataObject::GetDataHere() (mcorino, #23660). - Fix crash when loading some mal-formed GIFs (PB, #23409). +- Relax checks on character entry in numeric validators (#12968, #23561). wxGTK: diff --git a/include/wx/valnum.h b/include/wx/valnum.h index 75041a1ce4..95fd1a72f2 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -101,8 +101,9 @@ private: // this function. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const = 0; - // NormalizeString the contents of the string if it's a valid number, return - // empty string otherwise. + // Return the canonical form of the number corresponding to the contents of + // the string: if the input string is invalid, return a string representing + // some valid value. virtual wxString NormalizeString(const wxString& s) const = 0; @@ -239,8 +240,23 @@ protected: virtual wxString NormalizeString(const wxString& s) const wxOVERRIDE { LongestValueType value; - return BaseValidator::FromString(s, &value) ? NormalizeValue(value) - : wxString(); + if ( !BaseValidator::FromString(s, &value) ) + { + // We don't have any valid number at all, just arbitrarily decide + // to return the minimum value. + value = static_cast(m_min); + } + else if ( !this->IsInRange(value) ) + { + // We do have a value, but it's out of range: clamp it to the + // closest limit. + if ( value > static_cast(m_max) ) + value = static_cast(m_max); + else + value = static_cast(m_min); + } + + return NormalizeValue(value); } virtual bool CanBeNegative() const wxOVERRIDE { return m_min < 0; } diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp index b2db43d95b..cb43d5844d 100644 --- a/src/common/valnum.cpp +++ b/src/common/valnum.cpp @@ -264,19 +264,19 @@ wxIntegerValidatorBase::FromString(const wxString& s, } bool -wxIntegerValidatorBase::IsCharOk(const wxString& val, int pos, wxChar ch) const +wxIntegerValidatorBase::IsCharOk(const wxString& WXUNUSED(val), + int WXUNUSED(pos), + wxChar ch) const { // We only accept digits here (remember that '-' is taken care of by the // base class already). if ( ch < '0' || ch > '9' ) return false; - // And the value after insertion needs to be in the defined range. - LongestValueType value; - if ( !FromString(GetValueAfterInsertingChar(val, pos, ch), &value) ) - return false; - - return IsInRange(value); + // Accept anything that looks like a number here, notably do _not_ call + // IsInRange() because this would prevent entering any digits in an + // initially empty control limited to the values between "10" and "20". + return true; } // ============================================================================ @@ -342,7 +342,8 @@ wxFloatingPointValidatorBase::IsCharOk(const wxString& val, if ( ch < '0' || ch > '9' ) return false; - // Check whether the value we'd obtain if we accepted this key is correct. + // Check whether the value we'd obtain if we accepted this key passes some + // basic checks. const wxString newval(GetValueAfterInsertingChar(val, pos, ch)); LongestValueType value; @@ -354,8 +355,9 @@ wxFloatingPointValidatorBase::IsCharOk(const wxString& val, if ( posSep != wxString::npos && newval.length() - posSep - 1 > m_precision ) return false; - // Finally check whether it is in the range. - return IsInRange(value); + // Note that we do _not_ check if it's in range here, see the comment in + // wxIntegerValidatorBase::IsCharOk(). + return true; } #endif // wxUSE_VALIDATORS && wxUSE_TEXTCTRL diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index a937a99797..24d0b204b5 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -343,9 +343,27 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") wxYield(); CHECK( text2->GetValue() == "9" ); + // Entering a value which is out of range is allowed. sim.Char('9'); wxYield(); - CHECK( text2->GetValue() == "9" ); + CHECK( text2->GetValue() == "99" ); + + // But it must be clamped to the valid range on focus loss. + m_text->SetFocus(); + wxYield(); + CHECK( text2->GetValue() == "10.000" ); + + // Repeat the test with a too small invalid value. + text2->Clear(); + text2->SetFocus(); + + sim.Text("-22"); + wxYield(); + CHECK( text2->GetValue() == "-22" ); + + m_text->SetFocus(); + wxYield(); + CHECK( text2->GetValue() == "-10.000" ); } #endif // wxUSE_UIACTIONSIMULATOR