diff --git a/docs/changes.txt b/docs/changes.txt index 3afa22464b..bb933010e2 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -199,6 +199,7 @@ wxMSW: - Fix handling wxClipboard data when wxUSE_OLE == 0. - Fix caching of wxFONTSTYLE_SLANT fonts in wxTheFontList. - Fix wxTextCtrl::XYToPosition() and PositionToXY(). +- Fix updating radio groups when non-radio item is inserted to wxMenu. wxOSX: diff --git a/src/msw/menu.cpp b/src/msw/menu.cpp index c7790aeacd..e93cbbe184 100644 --- a/src/msw/menu.cpp +++ b/src/msw/menu.cpp @@ -102,11 +102,13 @@ public: } // Take into account the new radio item about to be added at the given - // position. - // + // position. The are two cases to handle: + // - If item precedes the range, the range indices have to be updated. + // - If item falls inside the range, this range is extended to include + // the item. // Returns true if this item starts a new radio group, false if it extends // an existing one. - bool UpdateOnInsert(int pos) + bool UpdateOnInsertRadio(int pos) { bool inExistingGroup = false; @@ -124,6 +126,8 @@ public: } else if ( pos <= r.end + 1 ) { + wxASSERT_MSG( !inExistingGroup, + wxS("Item already inserted inside another range") ); // 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. @@ -146,6 +150,58 @@ public: return true; } + // Take into account the new non-radio item about to be added at the given + // position. The are two cases to handle: + // - If item precedes the range, the range indices have to be updated. + // - If item falls inside the range, this range has to be split into + // two new ranges. + // Returns true if existing group has been split into two subgroups. + bool UpdateOnInsertNonRadio(int pos) + { + bool wasSplit = false; + Range newRange; + + 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 or just at its start, + // update its indices. + r.start++; + r.end++; + } + else if ( pos <= r.end ) + { + wxASSERT_MSG( !wasSplit, + wxS("Item already inserted inside another range") ); + // Item is inserted inside this range in which case + // it breaks the range into two parts: one ending before + // the item and one started after it. + + // The new range after the item has to be stored and added to the list + // after finishing the iteration through the ranges. + newRange.start = pos + 1; // start after the item + newRange.end = r.end + 1; // inherits current end "moved up" by one item + wasSplit = true; + // Current range ends just before the item position. + r.end = pos - 1; + } + //else: Item is inserted after this range, nothing to do for it. + } + + if ( !wasSplit ) + return false; + + // Add a split range to the list. + m_ranges.push_back(newRange); + + return true; + } + // Update the ranges of the existing radio groups after removing the menu // item at the given position. // @@ -498,20 +554,27 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos) pos = GetMenuItemCount() - 1; } - // Update radio groups data if we're inserting a new radio item. + // Update radio groups data if we're inserting a new menu item. + // Inserting radio and non-radio item has a different impact + // on radio groups so we have to handle each case separately. + // (Inserting a radio item in the middle of exisiting group extends + // the group, but inserting non-radio item breaks it into two subgroups.) // - // 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) ) + if ( m_radioData->UpdateOnInsertRadio(pos) ) checkInitially = true; } + else if ( m_radioData ) + { + bool groupWasSplit = m_radioData->UpdateOnInsertNonRadio(pos); + wxASSERT_MSG( !groupWasSplit, + wxS("Inserting non-radio item inside a radio group?") ); + } // Also handle the case of check menu items that had been checked before // being attached to the menu: we don't need to actually call Check() on