Opened 6 years ago

Closed 3 months ago

Last modified 3 months ago

#14213 closed defect (fixed)

Radio button groups don't work after deleting item from a menu under OS X

Reported by: shaurz Owned by: Artur Wieczorek <artwik@…>
Priority: normal Milestone:
Component: wxOSX Version: dev-latest
Keywords: wxMenu wxMenuItem remove radiobutton Cc: maartenbent@…
Blocked By: Blocking:
Patch: no

Description

When an item is deleted from a menu, radio button groups fail to work any more and it's possible to select multiple radio entries in the same group. See attached test case.

Attachments (2)

menu_radio_group_bug.py download (753 bytes) - added by shaurz 6 years ago.
Adjust-ranges-of-radio-button-groups.patch download (2.4 KB) - added by awi 4 years ago.
Adjust ranges of radio button groups.

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by shaurz

comment:1 Changed 6 years ago by vadz

  • Keywords menu radio added
  • Status changed from new to confirmed

I can indeed reproduce this in the menu sample under MSW with this patch:

  • samples/menu/menu.cpp

    diff --git a/samples/menu/menu.cpp b/samples/menu/menu.cpp
    index 3c8f97d..233cad5 100644
    a b void MyFrame::ShowContextMenu(const wxPoint& pos) 
    10831083    else // normal case, shift not pressed
    10841084    {
    10851085        menu.Append(Menu_Help_About, wxT("&About"));
     1086        menu.AppendRadioItem(Menu_Test_Radio1, "Radio 1");
     1087        menu.AppendRadioItem(Menu_Test_Radio2, "Radio 2");
     1088        menu.AppendRadioItem(Menu_Test_Radio3, "Radio 3");
     1089        menu.Delete(Menu_Help_About);
     1090        menu.Check(Menu_Test_Radio3, true);
     1091        PopupMenu(&menu, pos);
     1092        return;
     1093
    10861094        menu.Append(Menu_Popup_Submenu, wxT("&Submenu"), CreateDummyMenu(NULL));
    10871095        menu.Append(Menu_Popup_ToBeDeleted, wxT("To be &deleted"));
    10881096        menu.AppendCheckItem(Menu_Popup_ToBeChecked, wxT("To be &checked"));

Don't have time to debug how/why exactly is it broken right now though...

comment:2 Changed 4 years ago by awi

  • Keywords wxMenu wxMenuItem remove radiobutton added; menu radio removed
  • Patch set
  • Version changed from 2.9.3 to dev-latest

Problems with selecting radio buttons when any item is removed from the menu are caused by the fact that indices of the reamining items (inluding radio buttons) have been changed due to this operation but ranges of the radio buttons groups are not updated.
Patch fixing this issue is attached (attachment:ticket:14213:Adjust-ranges-of-radio-button-groups.patch).

Changed 4 years ago by awi

Adjust ranges of radio button groups.

comment:3 Changed 3 years ago by vadz

  • Component changed from GUI-all to wxOSX (any toolkit)
  • Summary changed from Radio button groups don't work after deleting item from a menu (Windows and OS X) to Radio button groups don't work after deleting item from a menu under OS X

Thanks a lot for fixing this!

Unfortunately the bug is almost certainly still present in wxOSX. We really should reuse the same code there, wxMenuRadioItemsData is not platform-specific at all and it "just" needs to be extracted to some include/wx/private/menuradio.h and be used in both ports...

comment:4 Changed 3 years ago by VZ

In 76650:

Fix bug with removing items from menus with radio buttons in wxMSW.

Update the indices of the radio groups after removing an item from the menu.

See #14213.

comment:5 Changed 3 years ago by VZ

In 76656:

Fix bug with removing items from menus with radio buttons in wxMSW.

Update the indices of the radio groups after removing an item from the menu.

See #14213.

comment:6 Changed 3 years ago by vadz

  • Patch unset

comment:7 Changed 22 months ago by vadz

  • Component changed from wxOSX (any toolkit) to wxOSX-Cocoa

Preparing to remove "wxOSX (any toolkit)" component: change all the bugs using to use "wxOSX-Cocoa" component instead.

comment:8 Changed 3 months ago by Artur Wieczorek <artwik@…>

In 4bc1c6fb7/git-wxWidgets:

Extract wxMenuRadioItemsData to a separate file

wxMenuRadioItemsData implementation is not MSW-specific and can be reused
on other platforms.

See #14213.

comment:9 Changed 3 months ago by Artur Wieczorek <artwik@…>

  • Owner set to Artur Wieczorek <artwik@…>
  • Resolution set to fixed
  • Status changed from confirmed to closed

In b6e4bdce3/git-wxWidgets:

Reimplement adding items to radio groups in wxMenu (wxOSX)

Rewrote wxOSX radio groups-related code reusing the code which works on
wxMSW and which seems to provide more rich functionality (supports adding
radio as well as no-radio items) and to have known bugs fixed.
The ranges of all radio groups are stored in wxMenu itself instead of
storing the information about the radio group an item belongs to in
the item itself - see 89511b4268.

Closes #14213.
Closes #17568.

comment:10 Changed 3 months ago by MaartenB

  • Cc maartenbent@… added

Changes from 4bc1c6fb7 result in a new warning in release builds:

../../src/msw/menu.cpp: In member function 'bool wxMenu::DoInsertOrAppend(wxMenuItem*, size_t)':
../../src/msw/menu.cpp:373:14: warning: unused variable 'groupWasSplit' [-Wunused-variable]
         bool groupWasSplit = m_radioData->UpdateOnInsertNonRadio(pos);

Please consider adding it inside the assert.

comment:11 Changed 3 months ago by Artur Wieczorek <artwik@…>

In e56cb112/git-wxWidgets:

Fix warning about unused variable in release build

Refactor the code to remove variable which is used only in the assertion.

See #14213.

Note: See TracTickets for help on using tickets.