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

diff --git a/include/wx/osx/menu.h b/include/wx/osx/menu.h
index 17f351efd6ff0c724c86a553213522b905a0bd8f..ceea341234cce4cc57b5d287dfe2402365f95783 100644 (file)
--- a/include/wx/osx/menu.h
+++ b/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)

diff --git a/include/wx/osx/menuitem.h b/include/wx/osx/menuitem.h
index e4f53c86e84cb79040b17759aca2a4b463631f41..31decc1e33b2e18747fcb64c4095f05ebce8c23d 100644 (file)
--- a/include/wx/osx/menuitem.h
+++ b/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() ;

diff --git a/src/osx/menu_osx.cpp b/src/osx/menu_osx.cpp
index 4ad6fa1fdfc2fe77dc739da4e2222684b692e3e6..8a795e67ce9d88d1e9e4036956e8589740bcfcab 100644 (file)
--- a/src/osx/menu_osx.cpp
+++ b/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;
+            }
         }
     }
 

diff --git a/src/osx/menuitem_osx.cpp b/src/osx/menuitem_osx.cpp
index 93ef5052867213a07960e6308f5bf7d1c922bb84..096d46c583b9971ab88d1b74f9a8f4cb4338e780 100644 (file)
--- a/src/osx/menuitem_osx.cpp
+++ b/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
 // ----------------------------------------------------------------------------