Opened 5 years ago

Closed 8 months ago

Last modified 8 months ago

#11821 closed defect (fixed)

wxToolBar::Realize() hangs in a vertical toolbar with only controls in it

Reported by: timhirrel Owned by:
Priority: low Milestone:
Component: wxMSW Version: dev-latest
Keywords: wxToolBar vertical control AddControl Cc:
Blocked By: Blocking:
Patch: yes

Description

class TAutoEntry : public wxToolBar {

wxCheckBox *PromptCheckBox;
wxRadioBox *AutoEntryRadioBox;
wxTextCtrl *DiamEdit, *RoughEdit, *PrefixEdit;
wxStaticText *diamText, *roughText, *prefixText;
wxButton *UndoBtn;
......

TAutoEntry::TAutoEntry (TMapFrame *winparam) :
wxToolBar (winparam, -1) {

SetWindowStyle(wxNO_BORDER | wxTB_VERTICAL | wxTB_TEXT);
}

void TAutoEntry::InitAutoEntry () {

PromptCheckBox = new wxCheckBox(this, idPromptBox, _T("Prompt IDs"));
AddControl(PromptCheckBox);
diamText = new wxStaticText (this, idDiamText, _T("Diameter"));
AddControl(diamText);
....

void TMapFrame::OnMenuAuto (wxCommandEvent &event) {

if (AutoEntry) {

AutoEntry = new TAutoEntry(this);
AutoEntry->InitAutoEntry();
SetToolBar(AutoEntry);
AutoEntry->Realize();
}

When compiled and run under MSWXP, the program locks up upon the 2nd call to wxToolBar::AddControl, regardless of the order of the calls. With only one call, the program runs ok, with either call. When compiled and run under Ubuntu, the program runs ok with multiple calls.

Attachments (3)

test11821_minimal.diff download (704 bytes) - added by catalin 21 months ago.
toolbar-vert-control.patch download (3.2 KB) - added by awi 8 months ago.
Patch fixing issue with controls added to vertical toolbar.
toolbar-vert-control-vz.patch download (3.8 KB) - added by vadz 8 months ago.
VZ version of the patch reducing code duplication

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by vadz

  • Component changed from wxGTK to wxMSW
  • Status changed from new to infoneeded_new

Please provide a patch to the toolbar sample reproducing the problem.

TIA!

Changed 21 months ago by catalin

comment:2 Changed 21 months ago by catalin

  • Status changed from infoneeded_new to new

In 2.9-svn the problem is a bit different than the description: a Vertical toolbar with more than one controls enters an infinite loop in Realize(). So all AddControl() calls return normally.
Test patch attached.

I've found How to Create Vertical Toolbars (Windows) which mention TBSTATE_WRAP button style, if it's relevant in this case.

comment:3 Changed 21 months ago by vadz

  • Priority changed from normal to low
  • Status changed from new to confirmed
  • Summary changed from wxToolBar AddControl, 2nd call in MSW locks up to wxToolBar::Realize() hangs in a vertical toolbar with only controls in it
  • Version changed from 2.8.x to 2.9-svn

It looks like the real problem is that we can't send TB_SETROWS to the empty toolbar as it enters into an infinite loop. The fix would be to maintain the number of buttons really added to it, i.e. excluding all the controls in the vertical case as we explicitly avoid adding them then.

If we add another item, the program doesn't hang but still looks wrong because the controls are not added to the toolbar but still shown. We should probably hide them if we don't add them...

The slightly updated patch I used for testing this:

  • samples/minimal/minimal.cpp

    diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
    index a78e462..0f13133 100644
    a b  
    3030    #include "wx/wx.h" 
    3131#endif 
    3232 
     33#include "wx/artprov.h" 
     34 
    3335// ---------------------------------------------------------------------------- 
    3436// resources 
    3537// ---------------------------------------------------------------------------- 
    bool MyApp::OnInit() 
    167169    SetMenuBar(menuBar); 
    168170#endif // wxUSE_MENUS 
    169171 
     172    wxToolBar* tbar = CreateToolBar(wxTB_VERTICAL); 
     173    tbar->AddTool(wxID_NEW, "New", wxArtProvider::GetBitmap("wxART_NEW")); 
     174    tbar->AddControl(new wxStaticText(tbar, wxID_HIGHEST + 1, _T("Txt 1"))); 
     175    tbar->AddControl(new wxStaticText(tbar, wxID_HIGHEST + 2, _T("Txt 2"))); 
     176    tbar->Realize(); 
     177 
    170178#if wxUSE_STATUSBAR 
    171179    // create a status bar just for fun (by default with 1 pane only) 
    172180    CreateStatusBar(2); 

comment:4 Changed 8 months ago by awi

  • Keywords vertical control added
  • Patch set
  • Version changed from stable-latest to dev-latest

It seems that controls appear on vertical toolbar because however they are not added to the toolbar and not displayed as toolbar tools but they exist and are displayed as if they were "independent" wxWidgets controls. If there are many such controls then they are overwritten and this is why the last one is visible.
Generally, to fix the issue it is necessary to hide all wxWidgets controls which are not added to the vertical toolbar.
Patch attached.

Another (more demanding) test case below:

--- wxWidgets-trunk\samples\minimal\minimal.cpp	2013-12-29
+++ wxWidgets-work\samples\minimal\minimal.cpp	2014-03-07
@@ -140,6 +140,7 @@
 // ----------------------------------------------------------------------------
 // main frame
 // ----------------------------------------------------------------------------
+#include "wx/artprov.h"
 
 // frame constructor
 MyFrame::MyFrame(const wxString& title)
@@ -166,7 +167,13 @@
     // ... and attach this menu bar to the frame
     SetMenuBar(menuBar);
 #endif // wxUSE_MENUS
-
+    wxToolBar* tbar = CreateToolBar(wxTB_VERTICAL);
+ 	tbar->AddControl(new wxStaticText(tbar, wxID_HIGHEST + 1, _T("Txt 1")));
+ 	tbar->AddTool(wxID_NEW, "New", wxArtProvider::GetBitmap("wxART_NEW"));
+ 	tbar->AddControl(new wxStaticText(tbar, wxID_HIGHEST + 2, _T("Txt 2")));
+    tbar->AddStretchableSpace();
+ 	tbar->AddTool(wxID_OPEN, "Delete", wxArtProvider::GetBitmap("wxART_DELETE"));
+ 	tbar->Realize();
 #if wxUSE_STATUSBAR
     // create a status bar just for fun (by default with 1 pane only)
     CreateStatusBar(2);

Changed 8 months ago by awi

Patch fixing issue with controls added to vertical toolbar.

comment:5 Changed 8 months ago by awi

Test case again:

  • \samples\minimal\minimal.cpp

    old new  
    140140// ---------------------------------------------------------------------------- 
    141141// main frame 
    142142// ---------------------------------------------------------------------------- 
     143#include "wx/artprov.h" 
    143144 
    144145// frame constructor 
    145146MyFrame::MyFrame(const wxString& title) 
     
    166167    // ... and attach this menu bar to the frame 
    167168    SetMenuBar(menuBar); 
    168169#endif // wxUSE_MENUS 
    169  
     170    wxToolBar* tbar = CreateToolBar(wxTB_VERTICAL); 
     171        tbar->AddControl(new wxStaticText(tbar, wxID_HIGHEST + 1, _T("Txt 1"))); 
     172        tbar->AddTool(wxID_NEW, "New", wxArtProvider::GetBitmap("wxART_NEW")); 
     173        tbar->AddControl(new wxStaticText(tbar, wxID_HIGHEST + 2, _T("Txt 2"))); 
     174    tbar->AddStretchableSpace(); 
     175        tbar->AddTool(wxID_OPEN, "Delete", wxArtProvider::GetBitmap("wxART_DELETE")); 
     176        tbar->Realize(); 
    170177#if wxUSE_STATUSBAR 
    171178    // create a status bar just for fun (by default with 1 pane only) 
    172179    CreateStatusBar(2); 

comment:6 Changed 8 months ago by vadz

Thanks for fixing this! But I wonder if we could avoid wxGetTBItemRect() duplication with my version of the patch. Do you see anything wrong with it? TIA!

Changed 8 months ago by vadz

VZ version of the patch reducing code duplication

comment:7 Changed 8 months ago by awi

You are right, in this case it's better to have one universal function then two specialized.
It looks fine.

comment:8 Changed 8 months ago by VZ

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

(In [76098]) Fix handling of controls in vertical toolbars in wxMSW.

Not adding the controls to vertical toolbar is not enough, we also need to
hide them to prevent them from being shown as independent floating windows.
And we also need to add separators instead of the controls themselves to keep
the indices the same as in the horizontal case.

Closes #11821.

comment:9 Changed 8 months ago by VZ

(In [76099]) Fix handling of controls in vertical toolbars in wxMSW.

Not adding the controls to vertical toolbar is not enough, we also need to
hide them to prevent them from being shown as independent floating windows.
And we also need to add separators instead of the controls themselves to keep
the indices the same as in the horizontal case.

Closes #11821.

Note: See TracTickets for help on using tickets.