From 35fa1f93bc3d01defe9892cb852e5d51b0259259 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 23 Apr 2021 16:25:39 +0100 Subject: [PATCH 1/4] Reset wxSpinCtrl value to GetMin() if text string is invalid Previously, wxSpinCtrl (using native control) and wxSpinCtrlDouble (using the generic implementation) behaved differently in this case, with the former changing its value but the latter keeping the last valid value instead. Make them behave the same by resetting the value in both cases and document this behaviour. --- docs/changes.txt | 3 +++ interface/wx/spinctrl.h | 8 ++++---- src/generic/spinctlg.cpp | 2 ++ tests/controls/spinctrldbltest.cpp | 2 +- tests/controls/spinctrltest.cpp | 1 + 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 5d2d9d12cd..b5242e5681 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -114,6 +114,9 @@ Changes in behaviour not resulting in compilation errors - wxChoice::GetString() now consistently asserts when passed an invalid index. +- wxSpinCtrlDouble now always resets its value to GetMin() if an invalid text + string is passed to its SetValue() after its creation. + Changes in behaviour which may result in build errors ----------------------------------------------------- diff --git a/interface/wx/spinctrl.h b/interface/wx/spinctrl.h index 55353ef86d..6371aeec7c 100644 --- a/interface/wx/spinctrl.h +++ b/interface/wx/spinctrl.h @@ -197,10 +197,10 @@ public: Sets the value of the spin control. It is recommended to use the overload taking an integer value instead. - The behaviour of this function when @a text doesn't represent a valid - number currently differs between the platforms, however passing an - empty string does clear the text part contents, without affecting the - value returned by GetValue(), under all of them. + If @a text doesn't represent a valid number, it may not be shown in the + text part of the control at all (only empty string is guaranteed to be + supported under all platforms) and the numeric value will be changed to + GetMin(). Notice that, unlike wxTextCtrl::SetValue(), but like most of the other setter methods in wxWidgets, calling this method does not generate any diff --git a/src/generic/spinctlg.cpp b/src/generic/spinctlg.cpp index aa99410c35..7255045e24 100644 --- a/src/generic/spinctlg.cpp +++ b/src/generic/spinctlg.cpp @@ -522,6 +522,8 @@ void wxSpinCtrlGenericBase::SetValue(const wxString& text) } else // not a number at all or out of range { + m_value = m_min; + m_textCtrl->ChangeValue(text); m_textCtrl->SelectAll(); } diff --git a/tests/controls/spinctrldbltest.cpp b/tests/controls/spinctrldbltest.cpp index a4fe967942..a028dbe977 100644 --- a/tests/controls/spinctrldbltest.cpp +++ b/tests/controls/spinctrldbltest.cpp @@ -179,7 +179,7 @@ TEST_CASE_METHOD(SpinCtrlDoubleTestCase, m_spin->SetValue(""); CHECK( m_spin->GetTextValue() == "" ); - CHECK( m_spin->GetValue() == 57.3 ); + CHECK( m_spin->GetValue() == 0 ); } #if wxUSE_UIACTIONSIMULATOR diff --git a/tests/controls/spinctrltest.cpp b/tests/controls/spinctrltest.cpp index ec46aada54..a6624066e2 100644 --- a/tests/controls/spinctrltest.cpp +++ b/tests/controls/spinctrltest.cpp @@ -278,6 +278,7 @@ TEST_CASE_METHOD(SpinCtrlTestCase2, "SpinCtrl::Value", "[spinctrl]") m_spin->SetValue(""); CHECK( m_spin->GetTextValue() == "" ); + CHECK( m_spin->GetValue() == 0 ); } TEST_CASE_METHOD(SpinCtrlTestCase2, "SpinCtrl::Base", "[spinctrl]") From 8fc2d4400421232c5520cda92c79f832d2673d62 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 23 Apr 2021 22:30:59 +0100 Subject: [PATCH 2/4] Don't generate events from wxMSW wxSpinCtrl::SetValue(wxString) The function was documented to not generate the events, but actually did generate wxEVT_TEXT ones, even if it didn't generate wxEVT_SPINCTRL. This was inconsistent with wxGTK and generic wxSpinCtrlDouble used under MSW, so change this to avoid the unwanted events. --- docs/changes.txt | 5 ++++- src/msw/spinctrl.cpp | 9 +++++---- tests/controls/spinctrldbltest.cpp | 6 ++++++ tests/controls/spinctrltest.cpp | 6 ++++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index b5242e5681..e8ad00371c 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -115,7 +115,10 @@ Changes in behaviour not resulting in compilation errors - wxChoice::GetString() now consistently asserts when passed an invalid index. - wxSpinCtrlDouble now always resets its value to GetMin() if an invalid text - string is passed to its SetValue() after its creation. + string is passed to its SetValue(wxString) overload after its creation. + +- wxSpinCtrl::SetValue(wxString) overload doesn't generate any events with + wxMSW, which was already the documented behaviour. Changes in behaviour which may result in build errors diff --git a/src/msw/spinctrl.cpp b/src/msw/spinctrl.cpp index 9cad282bcc..e41003091f 100644 --- a/src/msw/spinctrl.cpp +++ b/src/msw/spinctrl.cpp @@ -359,12 +359,9 @@ bool wxSpinCtrl::Create(wxWindow *parent, SetRange(min, max); SetValue(initial); - // Also set the text part of the control if it was specified independently - // but don't generate an event for this, it would be unexpected. - m_blockEvent = true; + // Also set the text part of the control if it was specified independently. if ( !value.empty() ) SetValue(value); - m_blockEvent = false; // Finally deal with the size: notice that this can only be done now both // windows are created and the text one is set up as buddy because @@ -449,10 +446,14 @@ wxString wxSpinCtrl::GetTextValue() const void wxSpinCtrl::SetValue(const wxString& text) { + m_blockEvent = true; + if ( !::SetWindowText(GetBuddyHwnd(), text.c_str()) ) { wxLogLastError(wxT("SetWindowText(buddy)")); } + + m_blockEvent = false; } void wxSpinCtrl::SetValue(int val) diff --git a/tests/controls/spinctrldbltest.cpp b/tests/controls/spinctrldbltest.cpp index a028dbe977..1e834b1915 100644 --- a/tests/controls/spinctrldbltest.cpp +++ b/tests/controls/spinctrldbltest.cpp @@ -177,9 +177,15 @@ TEST_CASE_METHOD(SpinCtrlDoubleTestCase, CHECK( m_spin->GetTextValue() == "57.30" ); CHECK( m_spin->GetValue() == 57.3 ); + CHECK( updatedSpin.GetCount() == 0 ); + CHECK( updatedText.GetCount() == 0 ); + m_spin->SetValue(""); CHECK( m_spin->GetTextValue() == "" ); CHECK( m_spin->GetValue() == 0 ); + + CHECK( updatedSpin.GetCount() == 0 ); + CHECK( updatedText.GetCount() == 0 ); } #if wxUSE_UIACTIONSIMULATOR diff --git a/tests/controls/spinctrltest.cpp b/tests/controls/spinctrltest.cpp index a6624066e2..fa2662deee 100644 --- a/tests/controls/spinctrltest.cpp +++ b/tests/controls/spinctrltest.cpp @@ -276,9 +276,15 @@ TEST_CASE_METHOD(SpinCtrlTestCase2, "SpinCtrl::Value", "[spinctrl]") CHECK( m_spin->GetTextValue() == "57" ); CHECK( m_spin->GetValue() == 57 ); + CHECK(updatedSpin.GetCount() == 0); + CHECK(updatedText.GetCount() == 0); + m_spin->SetValue(""); CHECK( m_spin->GetTextValue() == "" ); CHECK( m_spin->GetValue() == 0 ); + + CHECK(updatedSpin.GetCount() == 0); + CHECK(updatedText.GetCount() == 0); } TEST_CASE_METHOD(SpinCtrlTestCase2, "SpinCtrl::Base", "[spinctrl]") From 06f368be54b9b092990517b9e0a6f98e1925a0fb Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 23 Apr 2021 22:37:15 +0100 Subject: [PATCH 3/4] Use wxON_BLOCK_EXIT_SET() in wxMSW wxSpinCtrl code Ensure that m_blockEvent flag is always reset on the scope exit. No real changes. --- src/msw/spinctrl.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/msw/spinctrl.cpp b/src/msw/spinctrl.cpp index e41003091f..2fd2525a67 100644 --- a/src/msw/spinctrl.cpp +++ b/src/msw/spinctrl.cpp @@ -37,6 +37,8 @@ #include "wx/msw/private.h" #include "wx/msw/private/winstyle.h" +#include "wx/scopeguard.h" + #if wxUSE_TOOLTIPS #include "wx/tooltip.h" #endif // wxUSE_TOOLTIPS @@ -447,18 +449,18 @@ wxString wxSpinCtrl::GetTextValue() const void wxSpinCtrl::SetValue(const wxString& text) { m_blockEvent = true; + wxON_BLOCK_EXIT_SET(m_blockEvent, false); if ( !::SetWindowText(GetBuddyHwnd(), text.c_str()) ) { wxLogLastError(wxT("SetWindowText(buddy)")); } - - m_blockEvent = false; } void wxSpinCtrl::SetValue(int val) { m_blockEvent = true; + wxON_BLOCK_EXIT_SET(m_blockEvent, false); wxSpinButton::SetValue(val); @@ -487,8 +489,6 @@ void wxSpinCtrl::SetValue(int val) } m_oldValue = GetValue(); - - m_blockEvent = false; } int wxSpinCtrl::GetValue() const From fb88a3f3eb1c0d2729a34f0aad36fc800e2b2f95 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 23 Apr 2021 23:43:49 +0200 Subject: [PATCH 4/4] Don't use last valid value in wxGTK wxSpinCtrl with text override This partially undoes the recent changes to wxGTK wxSpinCtrl and reverts to the previous behaviour, which was actually compatible with wxMSW, and returns the minimum value when the text of the control is set to an invalid string. --- src/gtk/spinctrl.cpp | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/src/gtk/spinctrl.cpp b/src/gtk/spinctrl.cpp index b9f72eda28..637fb60592 100644 --- a/src/gtk/spinctrl.cpp +++ b/src/gtk/spinctrl.cpp @@ -130,13 +130,9 @@ private: class wxSpinCtrlGTKTextOverride { public: - wxSpinCtrlGTKTextOverride() - : m_value(0.0) - { - } - + // Text value used instead of the text representation of the actual numeric + // value. Notice that this string may be empty. wxString m_text; - double m_value; }; //----------------------------------------------------------------------------- @@ -160,19 +156,8 @@ wxSpinCtrlGTKBase::~wxSpinCtrlGTKBase() void wxSpinCtrlGTKBase::GTKSetTextOverride(const wxString& text) { if ( !m_textOverride ) - { - // Remember the original numeric value, that we're going to keep using - // it while this override is valid, and do it before initializing - // m_textOverride as our own "input" handler called from GTKGetValue() - // would use it if it were non-null. - const double value = GTKGetValue(); - m_textOverride = new wxSpinCtrlGTKTextOverride(); - m_textOverride->m_value = value; - } - //else: No need to change the value, it stays the same anyhow. - // Update the text in any case. m_textOverride->m_text = text; } @@ -258,10 +243,8 @@ bool wxSpinCtrlGTKBase::Create(wxWindow *parent, wxWindowID id, double wxSpinCtrlGTKBase::DoGetValue() const { // While using a text override, the text value is fixed by the program and - // shouldn't be used, just return the numeric value we had had before, as - // the text override is reset whenever it changes, so it must not have - // changed yet. - return m_textOverride ? m_textOverride->m_value : GTKGetValue(); + // shouldn't be used, just return the minimum value (which is 0 by default). + return m_textOverride ? DoGetMin() : GTKGetValue(); } double wxSpinCtrlGTKBase::GTKGetValue() const @@ -512,7 +495,7 @@ wxSpinCtrlGTKBase::GTKInputResult wxSpinCtrlGTKBase::GTKInput(double* value) con { if ( m_textOverride ) { - *value = m_textOverride->m_value; + *value = DoGetMin(); return GTKInput_Converted; }