Spinners in toolbars broken on OSX

[Auria]

This little sample shows the issue, where theere should be a spinner there is jsut empty space.

Screenshot :  http://imageshack.us/photo/my-images/690/toolbaryb.png/

#include "wx/wx.h"
#include "wx/spinctrl.h"

class MyFrame : public wxFrame
{
public:
    MyFrame() : wxFrame(NULL, wxID_ANY,  wxT("Hello wxWidgets"), wxPoint(50,50), wxSize(800,600))
    {
        wxToolBar* tb = CreateToolBar(wxBORDER_NONE | wxTB_HORIZONTAL | wxTB_TEXT);

        wxButton* btn = new wxButton(tb, wxID_ANY, "BTN");
        tb->AddControl(btn, "btn");
        
        wxSpinCtrl* spinner = new wxSpinCtrl(tb, wxID_ANY);
        tb->AddControl(spinner, "spinner");
        tb->Realize();
    }
};

class MyApp: public wxApp
{
    wxFrame* m_frame;
public:
    
    bool OnInit()
    {
        m_frame = new MyFrame();
        m_frame->Show();
        return true;
    } 
    
};

IMPLEMENT_APP(MyApp)

I confirmed this is caused by r72413

[johnr]

Can you try the spin_generic.patch please?
It should also fix #14893 and remove the need for the work-arounds done for #12045 and #13142.

[Auria]

This patch does indeed appear to fix the issue, thanks!

[csomor]

Vadim, I cannot think of what could be wrong with this patch, it does fix the inheritance, so that things again just work, the GetParent() sibling construct seems to be there since a dozen of years ...

[vadz]

I agree that making the composite window components children of itself, instead of its parent, is the right thing to do, I didn't change this because it had been done like this from the beginning, not because I think it's the right thing to do.

However I believe that we still need to use wxCompositeWindow to make sure that various colours setting methods etc work correctly, i.e. are propagated to the children. John, could you please try if changing the base class to wxNavigationEnabled<wxCompositeWindow<wxSpinCtrlBase> > while keeping your other changes still fixes the problem?

And, also, does it fix #14893 too?

[johnr]

Yes the original patch did fix #14893 but...

The addition of wxNavigationEnabled<wxCompositeWindow<wxSpinCtrlBase> > continues to enable keyboard navigation, however, SpinCtrlDouble->SetFore/BackgroundColour( *wxRED ) results in no colour change in MSW.

Interestingly it crashes with wxSpinCtrlDouble in MSW 64 Win8. double value = 40.0000 but this is not using the latest trunk.

I haven't had time to check what is going on here or with the colour change stuff and I can't test in OSX at present as I have had 5 hardware failures in 4 machines in 5 days leaving me with a notebook and a Win8 64 box minus optical drive that I have resurrected. One of my Hackintosh box's graphic card slot with card in-situ appears to have been used as a latrine by a discerning cockroach.

I will look when able.

Message was "nptr != NULL"

Stack trace 1

msvcr110d.dll!_wcstod_l(const wchar_t * nptr, wchar_t * * endptr, localeinfo_struct * plocinfo) Line 71 C++

msvcr110d.dll!wcstod(const wchar_t * nptr, wchar_t * * endptr) Line 117 C++
appmain64d.exe!wxStrtod(const wchar_t * nptr, wchar_t * * endptr) Line 816 C++
appmain64d.exe!wxString::ToDouble(double * pVal) Line 1737 C++
appmain64d.exe!wxSpinCtrlDouble::DoTextToValue(const wxString & text, double * val) Line 677 C++
appmain64d.exe!wxSpinCtrlGenericBase::DoSetValue(double val) Line 541 C++
appmain64d.exe!wxSpinCtrlDouble::SetValue(double value) Line 383 C++

Stack trace 2

msvcr110d.dll!_invoke_watson(const wchar_t * pszExpression, const wchar_t * pszFunction, const wchar_t * pszFile, unsigned int nLine, unsigned int64 pReserved) Line 131 C++

msvcr110d.dll!_invalid_parameter(const wchar_t * pszExpression, const wchar_t * pszFunction, const wchar_t * pszFile, unsigned int nLine, unsigned int64 pReserved) Line 86 C++
msvcr110d.dll!_wcstod_l(const wchar_t * nptr, wchar_t * * endptr, localeinfo_struct * plocinfo) Line 71 C++
msvcr110d.dll!wcstod(const wchar_t * nptr, wchar_t * * endptr) Line 117 C++
appmain64d.exe!wxStrtod(const wchar_t * nptr, wchar_t * * endptr) Line 816 C++
appmain64d.exe!wxString::ToDouble(double * pVal) Line 1737 C++
appmain64d.exe!wxSpinCtrlDouble::DoTextToValue(const wxString & text, double * val) Line 677 C++
appmain64d.exe!wxSpinCtrlGenericBase::DoSetValue(double val) Line 541 C++
appmain64d.exe!wxSpinCtrlDouble::SetValue(double value) Line 383 C++

[vadz]

Ugh, I had a single hardware failure recently and this already costed me several hours of my time. I can't imagine what would I do if 4 of my machines broke down simultaneously (but it probably would have been something load and untranslatable). Good luck with fixing them!

I'll try to retest this under all platforms "soon". Of course, any help from others would still be appreciated.

[johnr]

Ok, back in business. I was trying hard to remain intrigued by the quantum packet event grouping rather than being p...ed off. It didn't work 100% but helped.

wxNavigationEnabled<wxCompositeWindow<wxSpinCtrlBase> >

GTK:
I don't have an installation.

OSX:
Key navigation and scrolling work.
Repetitive SetEnabled(false/true) works.
Repetitive Show(false/true) works.
Setting colours seems never to have worked on OSX or at least the composite window is not fully implemented there?
I tried SpinCtrl->SetBackgroundColour(...) using just the original compositewindow template. The call went to wxWindowMac::SetBackgroundColour(...) which set the colour for the composite window and its peer only with no change to the components.

MSW:
Key navigation and scrolling work.
Repetitive SetEnabled(false/true) works.
Repetitive Show(false/true) works.
compositewin.h / virtual bool SetBackgroundColour(...) is called and the colour is changed for all components including the parent composite window. Maybe this is how it should work if it is against a parent window and you want it to match that window's colour but it should inherit this background colour anyway and you might not want the textbox to change.

If you only want the textbox to change then it is not desirable as the chosen colour shows through the textctrl / spinbuttons spacing gap. It is not ideal but no worse than before.

[johnr]

Replying to johnr:

Setting colours seems never to have worked on OSX or at least the composite window is not fully implemented there?

I later noticed an error in my test code and I was totally wrong with the above comment.

If you only want the textbox to change then it is not desirable as the chosen colour shows through the textctrl / spinbuttons spacing gap. It is not ideal but no worse than before.

I have redone the patch to include wxCompositeWindow and also provided a SetBackgroundColour method for the base that sets only the wxTextCtrl component background so there are no odd colours showing through the between the components.

[VZ]

(In [73630]) Use generic spin control itself as parent for its children.

This fixes a problem with using wxSpinCtrlGeneric in toolbars under wxOSX,
using the toolbar itself (i.e. the parent of the spin control) as parent for
the children didn't work there and no windows were visible at all.

Also use wxNavigationEnabled as base class of wxSpinCtrlGeneric to fix
keyboard navigation.

And override SetBackgroundColour() to set it for the text control part of the
spin control only.

Closes #15016.