Amebis/wxWidgets
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rewrote wxMSW radio menu items code to support not only appending them.

Browse Source 
Previously the radio menu items could only be appended to a menu in wxMSW,
inserting them (either in an existing radio group or to start a new one) not
only didn't work but could even result in crashes because invalid iterators in
the menu items list could be used.

Fix this by storing the ranges of all radio groups in wxMenu itself instead of
storing the information about the radio group an item belongs to in the item
itself and by updating this data whenever a new radio item is inserted. Also
get rid of the notion of "current radio group" in wxMenu which doesn't really
make any sense.

Finally add a unit test checking that inserting radio items works as expected.

Closes #13200.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@67720 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
This commit is contained in:
Vadim Zeitlin
2011-05-10 08:50:38 +00:00
parent a6ca624a27
commit 89511b4268
5 changed files with 194 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
include/wx/msw/menu.h
View File

@@ -25,6 +25,7 @@ class WXDLLIMPEXP_FWD_CORE wxFrame;
class WXDLLIMPEXP_FWD_CORE wxToolBar;
#endif
class wxMenuRadioItemsData;
// Not using a combined wxToolBar/wxMenuBar? then use
// a commandbar in WinCE .NET to implement the
@@ -63,13 +64,16 @@ public:
// implementation only from now on
// -------------------------------
virtual void Attach(wxMenuBarBase *menubar);
bool MSWCommand(WXUINT param, WXWORD id);
// get the native menu handle
WXHMENU GetHMenu() const { return m_hMenu; }
// Return the start and end position of the radio group to which the item
// at the given position belongs. Returns false if there is no radio group
// containing this position.
bool MSWGetRadioGroupRange(int pos, int *start, int *end) const;
#if wxUSE_ACCEL
// called by wxMenuBar to build its accel table from the accels of all menus
bool HasAccels() const { return !m_accels.empty(); }
@@ -122,15 +126,17 @@ private:
// common part of Append/Insert (behaves as Append is pos == (size_t)-1)
bool DoInsertOrAppend(wxMenuItem *item, size_t pos = (size_t)-1);
// terminate the current radio group, if any
void EndRadioGroup();
// This variable contains the description of the radio item groups and
// allows to find whether an item at the given position is part of the
// group and also where its group starts and ends.
//
// It is initially NULL and only allocated if we have any radio items.
wxMenuRadioItemsData *m_radioData;
// if true, insert a breal before appending the next item
bool m_doBreak;
// the position of the first item in the current radio group or -1
int m_startRadioGroup;
// the menu handle of this menu
WXHMENU m_hMenu;

17
include/wx/msw/menuitem.h
View File

@@ -61,11 +61,6 @@ public:
// Win32 API
WXWPARAM GetMSWId() const;
// mark item as belonging to the given radio group
void SetAsRadioGroupStart();
void SetRadioGroupStart(int start);
void SetRadioGroupEnd(int end);
#if WXWIN_COMPATIBILITY_2_8
// compatibility only, don't use in new code
wxDEPRECATED(
@@ -130,18 +125,6 @@ private:
// common part of all ctors
void Init();
// the positions of the first and last items of the radio group this item
// belongs to or -1: start is the radio group start and is valid for all
// but first radio group items (m_isRadioGroupStart == false), end is valid
// only for the first one
union
{
int start;
int end;
} m_radioGroup;
// does this item start a radio group?
bool m_isRadioGroupStart;
#if wxUSE_OWNER_DRAWN
// item bitmaps

191
src/msw/menu.cpp
View File

@@ -41,6 +41,7 @@
#endif
#include "wx/scopedarray.h"
#include "wx/vector.h"
#include "wx/msw/private.h"
#include "wx/msw/wrapcctl.h" // include <commctrl.h> "properly"
@@ -85,9 +86,102 @@
static const int idMenuTitle = wxID_NONE;
// ----------------------------------------------------------------------------
// private functions
// private helper classes and functions
// ----------------------------------------------------------------------------
// Contains the data about the radio items groups in the given menu.
class wxMenuRadioItemsData
{
public:
wxMenuRadioItemsData() { }
// Default copy ctor, assignment operator and dtor are all ok.
// Find the start and end of the group containing the given position or
// return false if it's not inside any range.
bool GetGroupRange(int pos, int *start, int *end) const
{
// We use a simple linear search here because there are not that many
// items in a menu and hence even fewer radio items ranges anyhow, so
// normally there is no need to do anything fancy (like keeping the
// array sorted and using binary search).
for ( Ranges::const_iterator it = m_ranges.begin();
it != m_ranges.end();
++it )
{
const Range& r = *it;
if ( r.start <= pos && pos <= r.end )
{
if ( start )
*start = r.start;
if ( end )
*end = r.end;
return true;
}
}
return false;
}
// Take into account the new radio item about to be added at the given
// position.
//
// Returns true if this item starts a new radio group, false if it extends
// an existing one.
bool UpdateOnInsert(int pos)
{
bool inExistingGroup = false;
for ( Ranges::iterator it = m_ranges.begin();
it != m_ranges.end();
++it )
{
Range& r = *it;
if ( pos < r.start )
{
// Item is inserted before this range, update its indices.
r.start++;
r.end++;
}
else if ( pos <= r.end + 1 )
{
// Item is inserted in the middle of this range or immediately
// after it in which case it extends this range so make it span
// one more item in any case.
r.end++;
inExistingGroup = true;
}
//else: Item is inserted after this range, nothing to do for it.
}
if ( inExistingGroup )
return false;
// Make a new range for the group this item will belong to.
Range r;
r.start = pos;
r.end = pos;
m_ranges.push_back(r);
return true;
}
private:
// Contains the inclusive positions of the range start and end.
struct Range
{
int start;
int end;
};
typedef wxVector<Range> Ranges;
Ranges m_ranges;
};
namespace
{
@@ -168,8 +262,8 @@ inline bool IsGreaterThanStdSize(const wxBitmap& bmp)
// Construct a menu with optional title (then use append)
void wxMenu::Init()
{
m_radioData = NULL;
m_doBreak = false;
m_startRadioGroup = -1;
#if wxUSE_OWNER_DRAWN
m_ownerDrawn = false;
@@ -211,6 +305,8 @@ wxMenu::~wxMenu()
// delete accels
WX_CLEAR_ARRAY(m_accels);
#endif // wxUSE_ACCEL
delete m_radioData;
}
void wxMenu::Break()
@@ -219,13 +315,6 @@ void wxMenu::Break()
m_doBreak = true;
}
void wxMenu::Attach(wxMenuBarBase *menubar)
{
wxMenuBase::Attach(menubar);
EndRadioGroup();
}
#if wxUSE_ACCEL
int wxMenu::FindAccel(int id) const
@@ -348,6 +437,11 @@ HBITMAP GetHBitmapForMenu(wxMenuItem *pItem, bool checked = true)
} // anonymous namespace
bool wxMenu::MSWGetRadioGroupRange(int pos, int *start, int *end) const
{
return m_radioData && m_radioData->GetGroupRange(pos, start, end);
}
// append a new item or submenu to the menu
bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
{
@@ -397,6 +491,21 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
pos = GetMenuItemCount() - 1;
}
// Update radio groups data if we're inserting a new radio item.
//
// NB: If we supported inserting non-radio items in the middle of existing
// radio groups to break them into two subgroups, we'd need to update
// m_radioData in this case too but currently this is not supported.
bool checkInitially = false;
if ( pItem->GetKind() == wxITEM_RADIO )
{
if ( !m_radioData )
m_radioData = new wxMenuRadioItemsData;
if ( m_radioData->UpdateOnInsert(pos) )
checkInitially = true;
}
// adjust position to account for the title of a popup menu, if any
if ( !GetMenuBar() && !m_title.empty() )
pos += 2; // for the title itself and its separator
@@ -600,6 +709,10 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
}
// Check the item if it should be initially checked.
if ( checkInitially )
pItem->Check(true);
// if we just appended the title, highlight it
if ( id == (UINT_PTR)idMenuTitle )
{
@@ -616,67 +729,9 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
return true;
}
void wxMenu::EndRadioGroup()
{
// we're not inside a radio group any longer
m_startRadioGroup = -1;
}
wxMenuItem* wxMenu::DoAppend(wxMenuItem *item)
{
wxCHECK_MSG( item, NULL, wxT("NULL item in wxMenu::DoAppend") );
bool check = false;
if ( item->GetKind() == wxITEM_RADIO )
{
int count = GetMenuItemCount();
if ( m_startRadioGroup == -1 )
{
// start a new radio group
m_startRadioGroup = count;
// for now it has just one element
item->SetAsRadioGroupStart();
item->SetRadioGroupEnd(m_startRadioGroup);
// ensure that we have a checked item in the radio group
check = true;
}
else // extend the current radio group
{
// we need to update its end item
item->SetRadioGroupStart(m_startRadioGroup);
wxMenuItemList::compatibility_iterator node = GetMenuItems().Item(m_startRadioGroup);
if ( node )
{
node->GetData()->SetRadioGroupEnd(count);
}
else
{
wxFAIL_MSG( wxT("where is the radio group start item?") );
}
}
}
else // not a radio item
{
EndRadioGroup();
}
if ( !wxMenuBase::DoAppend(item) || !DoInsertOrAppend(item) )
{
return NULL;
}
if ( check )
{
// check the item initially
item->Check(true);
}
return item;
return wxMenuBase::DoAppend(item) && DoInsertOrAppend(item) ? item : NULL;
}
wxMenuItem* wxMenu::DoInsert(size_t pos, wxMenuItem *item)

40
src/msw/menuitem.cpp
View File

@@ -494,9 +494,6 @@ wxMenuItem::wxMenuItem(wxMenu *parentMenu,
void wxMenuItem::Init()
{
m_radioGroup.start = -1;
m_isRadioGroupStart = false;
#if wxUSE_OWNER_DRAWN
// when the color is not valid, wxOwnerDraw takes the default ones.
@@ -557,30 +554,6 @@ bool wxMenuItem::IsChecked() const
return (flag & MF_CHECKED) != 0;
}
// radio group stuff
// -----------------
void wxMenuItem::SetAsRadioGroupStart()
{
m_isRadioGroupStart = true;
}
void wxMenuItem::SetRadioGroupStart(int start)
{
wxASSERT_MSG( !m_isRadioGroupStart,
wxT("should only be called for the next radio items") );
m_radioGroup.start = start;
}
void wxMenuItem::SetRadioGroupEnd(int end)
{
wxASSERT_MSG( m_isRadioGroupStart,
wxT("should only be called for the first radio item") );
m_radioGroup.end = end;
}
// change item state
// -----------------
@@ -634,17 +607,10 @@ void wxMenuItem::Check(bool check)
int start,
end;
if ( m_isRadioGroupStart )
if ( !m_parentMenu->MSWGetRadioGroupRange(pos, &start, &end) )
{
// we already have all information we need
start = pos;
end = m_radioGroup.end;
}
else // next radio group item
{
// get the radio group end from the start item
start = m_radioGroup.start;
end = items.Item(start)->GetData()->m_radioGroup.end;
wxFAIL_MSG( wxT("Menu radio item not part of radio group?") );
return;
}
#ifdef __WIN32__

55
tests/menu/menu.cpp
View File

@@ -84,6 +84,7 @@ private:
CPPUNIT_TEST( FindInMenu );
CPPUNIT_TEST( Count );
CPPUNIT_TEST( Labels );
CPPUNIT_TEST( RadioItems );
CPPUNIT_TEST_SUITE_END();
void CreateFrame();
@@ -92,6 +93,7 @@ private:
void FindInMenu();
void Count();
void Labels();
void RadioItems();
wxFrame* m_frame;
@@ -304,3 +306,56 @@ void MenuTestCase::Labels()
CPPUNIT_ASSERT_EQUAL( "Foo", itemFoo->GetItemLabelText() );
CPPUNIT_ASSERT_EQUAL( "Foo", wxMenuItem::GetLabelText("&Foo\tCtrl-F") );
}
void MenuTestCase::RadioItems()
{
wxMenuBar * const bar = m_frame->GetMenuBar();
wxMenu * const menu = new wxMenu;
bar->Append(menu, "&Radio");
// Adding consecutive radio items creates a radio group.
menu->AppendRadioItem(MenuTestCase_First, "Radio 0");
menu->AppendRadioItem(MenuTestCase_First + 1, "Radio 1");
// First item of a radio group is checked by default.
CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First) );
// Checking the second one make the first one unchecked however.
menu->Check(MenuTestCase_First + 1, true);
CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First) );
CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 1) );
// Adding more radio items after a separator creates another radio group...
menu->AppendSeparator();
menu->AppendRadioItem(MenuTestCase_First + 2, "Radio 2");
menu->AppendRadioItem(MenuTestCase_First + 3, "Radio 3");
menu->AppendRadioItem(MenuTestCase_First + 4, "Radio 4");
// ... which is independent from the first one.
CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 2) );
menu->Check(MenuTestCase_First + 3, true);
CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 3) );
CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 2) );
CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 1) );
// Insert an item in the middle of an existing radio group.
menu->InsertRadioItem(4, MenuTestCase_First + 5, "Radio 5");
CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 3) );
menu->Check( MenuTestCase_First + 5, true );
CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 3) );
// Prepend a couple of items before the first group.
menu->PrependRadioItem(MenuTestCase_First + 6, "Radio 6");
menu->PrependRadioItem(MenuTestCase_First + 7, "Radio 7");
menu->Check(MenuTestCase_First + 7, true);
CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 1) );
// Check that the last radio group still works as expected.
menu->Check(MenuTestCase_First + 4, true);
CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 5) );
}