#14894 closed defect (worksforme)

wxSpinCtrl ignores constructors initialize value when outside of default range (wxMSW)

Reported by: joim Owned by: vadz
Priority: normal Milestone: 3.0.0
Component: wxMSW Version: 2.9.4
Keywords: Cc: barney.wrightson@…
Blocked By: Blocking:
Patch: no

Description

I tried to create a wxSpinCtrl by passing new min- and max-values and and a new initial value directly to the constructor (this is the way wxGetNumberFromUser () does it when creating its spin control).

But the initial value will be cut to 100 even if given max-value is bigger.

Operating system is Windows7.

The following line code shows the effect:

m_spinctrl = new wxSpinCtrl(this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 16896L, 0, 300, 150);

Maybe this could be solved by changing the order of the commands

SetValue(initial);

SetRange(min, max);

in the function Create () in file src/msw/spinctrl.cpp?

Attachments (1)

initValueSpinCtrl.patch download (1.7 KB) - added by joim 20 months ago.
Patch for initial value bug with SpinCtrl

Download all attachments as: .zip

Change History (11)

comment:1 Changed 21 months ago by joim

  • Component changed from GUI-all to wxMSW
  • Version set to 2.9.4

comment:2 Changed 20 months ago by vadz

  • Status changed from new to confirmed

I do see the problem with just

  • samples/minimal/minimal.cpp

    diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
    index a78e462..8d1a5e5 100644
    a b bool MyApp::OnInit() 
    141141// main frame 
    142142// ---------------------------------------------------------------------------- 
    143143 
     144#include "wx/spinctrl.h" 
     145 
    144146// frame constructor 
    145147MyFrame::MyFrame(const wxString& title) 
    146148       : wxFrame(NULL, wxID_ANY, title) 
    bool MyApp::OnInit() 
    167169    SetMenuBar(menuBar); 
    168170#endif // wxUSE_MENUS 
    169171 
     172    new wxSpinCtrl(this, wxID_ANY, "", wxPoint(5, 5), wxDefaultSize, 
     173                   wxSP_ARROW_KEYS, 0, 300, 150); 
     174    new wxStaticText(this, wxID_ANY, ""); 
     175 
    170176#if wxUSE_STATUSBAR 
    171177    // create a status bar just for fun (by default with 1 pane only) 
    172178    CreateStatusBar(2); 

The challenge is fixing it without breaking something else. It would be great if you could add a (currently failing) test for this to tests/controls/spinctrltest.cpp, then apply your proposed fix and verify that it doesn't break all the other tests by running test_gui.exe -t SpinCtrl. Could you please try to do it and submit a patch with your changes?

Changed 20 months ago by joim

Patch for initial value bug with SpinCtrl

comment:3 Changed 20 months ago by joim

  • Patch set

I added a new test which failed with the current version.

I then switched the order of the SetValue () and SetRange () function calls. This seems to solve the problem, all test cases are OK now.

I also commented out the line "m_oldValue = initial;" which seems not to be necessary in my opinion, because the m_oldValue will be set in SetValue () function which is called one line before. Even worse, after switching the order this command could be wrong because m_oldValue will not be cut to min/max values now any more.

comment:4 Changed 20 months ago by vadz

  • Owner set to vadz
  • Status changed from confirmed to accepted

Thanks! Everything you say seems to be totally right so I'll apply the patch soon.

comment:5 Changed 20 months ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [73394]) Fix setting initial wxSpinCtrl value outside 0..100 range in wxMSW.

Set the range before setting the initial value when creating wxSpinCtrl, as
otherwise the value was wrongly limited to the default 0..100 range instead of
the one really specified.

Closes #14894.

comment:6 Changed 15 months ago by bmw

  • Resolution fixed deleted
  • Status changed from closed to reopened

While this fixes the issue of not being able to est an initial value outside of the default 0..100 range, it introduces a different issue. SetRange calls NormalizeValue() which will not be operating on the initial value, as it hasn't been set yet. This can lead to wxEVT_COMMAND_SPINCTRL_UPDATED events being sent before construction of the control is finished, which would not be expected. This can cause problems as clients may already have handlers registered that expect a member pointer to the control to be valid.

It ends up being a chicken and egg problem with the ordering of the SetValue and SetRange calls, which IMHO stems from the fact that wxSpinButtonBase doesn't allow the min/max to be set in the constructor. SetValue references m_min/m_max and SetRange references the current value.

If both need to be called to get everything set up right, then I believe the easiest fix is to revert the patch from this ticket, and simply set m_min = min and m_max = max (or call wxSpinButtonBase::SetRange() ) before calling either SetValue() or SetRange() and then all should be well.

Barney

comment:7 Changed 15 months ago by bmw

whoops I mean "set" not "est" an initial value

comment:8 Changed 15 months ago by vadz

  • Milestone set to 3.0
  • Patch unset

Could you please [HowtoSubmitPatches make a patch] to the test suite reproducing the problem? Please see the changes to the tests/control/spinctrltest.cpp file in r72341 and r71892 for some of the existing tests checking for similar bugs.

Of course, if you can make a patch with the changes to fix the problem, it would be very welcome too, but being able to reproduce it would be a very useful first step already.

TIA!

comment:9 Changed 14 months ago by vadz

  • Status changed from reopened to infoneeded_new

Sorry, I just spent some time on this myself and I can't reproduce the problem so without having some way to provoke the generation of these events from the ctor, I can't really do anything about this.

Could you please check if you can reproduce the bug by changing SpinCtrlTestCase::NoEventsInCtor() in source:wxWidgets/trunk/tests/controls/spinctrltest.cpp#L103 ?

TIA!

comment:10 Changed 12 months ago by bmw

  • Cc barney.wrightson@… added
  • Resolution set to worksforme
  • Status changed from infoneeded_new to closed

Hi Vadim,

Sorry for the slow reply - I forgot to add myself to th CC list.

At the time I applied this patch to a pristine 2.9.4 to test it, which I now realise is why I was getting the issue. The combination of this patch and a previous change (72341 - Don't generate events from wxSpinCtrl::SetRange() in wxMSW) seems to make it work as expected.

Sorry about the noise.

Thanks,
Barney

Note: See TracTickets for help on using tickets.