Fix updating radio groups when non-radio item is inserted to wxMenu (MSW)

When wxMenu contains radio groups and a new item (radio and non-radio) is
being added to the menu, data describing exisiting groups have to
be updated accordingly. Because adding radio and non-radio items has
a different impact on the groups, adding non-radio items has to be handled
separately. (The main difference between adding radio and non-radio item is
that when a radio item is inserted inside the group this group is extended,
but for non-radio item the group is split into two subgroups.)

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:
 

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