Get rid of wxMenu::m_startRadioGroup in wxOSX.

This code was probably copied from wxUniv but was wrong as we can't rely on
the items being always inserted in order. This commit on its own fixes
removing the first radio group menu item but it also makes possible to
properly implement the insertion of new items in the middle of an existing
radio group which couldn't be done with m_startRadioGroup approach at all.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@74544 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/osx/menu.h

@@ -33,8 +33,6 @@ public:
 
     virtual ~wxMenu();
 
-    virtual void Attach(wxMenuBarBase *menubar) ;
-
     virtual void Break();
 
     virtual void SetTitle(const wxString& title);
@@ -80,9 +78,6 @@ 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();
-
     // Common part of HandleMenu{Opened,Closed}().
     void DoHandleMenuOpenedOrClosed(wxEventType evtType);
 
@@ -96,9 +91,6 @@ private:
     // don't trigger native events
     bool m_noEventsMode;
 
-    // the position of the first item in the current radio group or -1
-    int m_startRadioGroup;
-
     wxMenuImpl* m_peer;
 
     DECLARE_DYNAMIC_CLASS(wxMenu)

include/wx/osx/menuitem.h

@@ -46,6 +46,9 @@ public:
     virtual void SetBitmap(const wxBitmap& bitmap) ;
     virtual const wxBitmap& GetBitmap() const { return m_bitmap; }
 
+
+    // Implementation only from now on.
+
     // update the os specific representation
     void UpdateItemBitmap() ;
     void UpdateItemText() ;
@@ -56,6 +59,19 @@ public:
     void SetRadioGroupStart(int start);
     void SetRadioGroupEnd(int end);
 
+    // return true if this is the starting item of a radio group
+    bool IsRadioGroupStart() const;
+
+    // get the start of the radio group this item belongs to: should not be
+    // called for the starting radio group item itself because it doesn't have
+    // this information
+    int GetRadioGroupStart() const;
+
+    // get the end of the radio group this item belongs to: should be only
+    // called for the starting radio group item, i.e. if IsRadioGroupStart() is
+    // true
+    int GetRadioGroupEnd() const;
+
     wxMenuItemImpl* GetPeer() { return m_peer; }
 private:
     void UncheckRadio() ;

src/osx/menu_osx.cpp

@@ -57,7 +57,6 @@ static const int idMenuTitle = -3;
 void wxMenu::Init()
 {
     m_doBreak = false;
-    m_startRadioGroup = -1;
     m_allowRearrange = true;
     m_noEventsMode = false;
 
@@ -89,13 +88,6 @@ void wxMenu::Break()
     // not available on the mac platform
 }
 
-void wxMenu::Attach(wxMenuBarBase *menubar)
-{
-    wxMenuBase::Attach(menubar);
-
-    EndRadioGroup();
-}
-
 void wxMenu::SetAllowRearrange( bool allow )
 {
     m_allowRearrange = allow;
@@ -141,30 +133,21 @@ 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 )
+    if ( item->IsRadio() )
     {
         int count = GetMenuItemCount();
 
-        if ( m_startRadioGroup == -1 )
+        if ( !count || !(*GetMenuItems().rbegin())->IsRadio() )
         {
             // start a new radio group
-            m_startRadioGroup = count;
-
-            // for now it has just one element
             item->SetAsRadioGroupStart();
-            item->SetRadioGroupEnd(m_startRadioGroup);
+            item->SetRadioGroupEnd(count);
 
             // ensure that we have a checked item in the radio group
             check = true;
@@ -172,8 +155,13 @@ wxMenuItem* wxMenu::DoAppend(wxMenuItem *item)
         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);
+            wxMenuItem* const last = *GetMenuItems().rbegin();
+            const int groupStart = last->IsRadioGroupStart()
+                                    ? count
+                                    : last->GetRadioGroupStart();
+
+            item->SetRadioGroupStart(groupStart);
+            wxMenuItemList::compatibility_iterator node = GetMenuItems().Item(groupStart);
 
             if ( node )
             {
@@ -185,10 +173,6 @@ wxMenuItem* wxMenu::DoAppend(wxMenuItem *item)
             }
         }
     }
-    else // not a radio item
-    {
-        EndRadioGroup();
-    }
 
     if ( !wxMenuBase::DoAppend(item) || !DoInsertOrAppend(item) )
         return NULL;
@@ -210,14 +194,33 @@ wxMenuItem* wxMenu::DoInsert(size_t pos, wxMenuItem *item)
 
 wxMenuItem *wxMenu::DoRemove(wxMenuItem *item)
 {
-    if ( m_startRadioGroup != -1 )
+    if ( item->IsRadio() )
     {
         // Check if we're removing the item starting the radio group
-        if ( GetMenuItems().Item(m_startRadioGroup)->GetData() == item )
+        if ( item->IsRadioGroupStart() )
         {
-            // Yes, we do, so reset its index as the next item added shouldn't
-            // count as part of the same radio group anyhow.
-            m_startRadioGroup = -1;
+            // Yes, we do, update the next radio group item, if any, to be the
+            // start one now.
+            const int endGroup = item->GetRadioGroupEnd();
+
+            wxMenuItemList::compatibility_iterator
+                node = GetMenuItems().Item(endGroup);
+            wxASSERT_MSG( node, wxS("Should have valid radio group end") );
+
+            while ( node->GetData() != item )
+            {
+                const wxMenuItemList::compatibility_iterator
+                    prevNode = node->GetPrevious();
+                wxMenuItem* const prevItem = prevNode->GetData();
+                if ( prevItem == item )
+                {
+                    prevItem->SetAsRadioGroupStart();
+                    prevItem->SetRadioGroupEnd(endGroup);
+                    break;
+                }
+
+                node = prevNode;
+            }
         }
     }
 

src/osx/menuitem_osx.cpp

@@ -239,6 +239,27 @@ void wxMenuItem::SetRadioGroupEnd(int end)
     m_radioGroup.end = end;
 }
 
+bool wxMenuItem::IsRadioGroupStart() const
+{
+    return m_isRadioGroupStart;
+}
+
+int wxMenuItem::GetRadioGroupStart() const
+{
+    wxASSERT_MSG( !m_isRadioGroupStart,
+                  wxS("shouldn't be called for the first radio item") );
+
+    return m_radioGroup.start;
+}
+
+int wxMenuItem::GetRadioGroupEnd() const
+{
+    wxASSERT_MSG( m_isRadioGroupStart,
+                  wxS("shouldn't be called for the first radio item") );
+
+    return m_radioGroup.end;
+}
+
 // ----------------------------------------------------------------------------
 // wxMenuItemBase
 // ----------------------------------------------------------------------------