diff --git a/android/tests_backend/app.py b/android/tests_backend/app.py index 9085b27903..ee581f5b75 100644 --- a/android/tests_backend/app.py +++ b/android/tests_backend/app.py @@ -12,6 +12,8 @@ class AppProbe(BaseProbe, DialogsMixin): + supports_application_menu_command_native_items = False + supports_status_icon_command_native_items = False supports_key = False supports_dark_mode = True edit_menu_noop_enabled = False diff --git a/changes/4580.bugfix.md b/changes/4580.bugfix.md new file mode 100644 index 0000000000..9f9babeb7e --- /dev/null +++ b/changes/4580.bugfix.md @@ -0,0 +1 @@ +Native menu and toolbar command instances are now released when their containers are rebuilt or closed. diff --git a/cocoa/src/toga_cocoa/window.py b/cocoa/src/toga_cocoa/window.py index 0f44e4a85b..a8a8c174a9 100644 --- a/cocoa/src/toga_cocoa/window.py +++ b/cocoa/src/toga_cocoa/window.py @@ -176,6 +176,7 @@ def toolbar_itemForItemIdentifier_willBeInsertedIntoToolbar_( native.setImage(item.icon._impl._as_size(32)) item._impl.native.add(native) + self.impl.toolbar_items[native] = item native.setTarget_(self) native.setAction_(SEL("onToolbarButtonPress:")) @@ -544,11 +545,16 @@ def __init__(self, interface, title, position, size): # By default, no toolbar self._toolbar_items = {} + self.toolbar_items = {} self.native_toolbar = None def __del__(self): self.purge_toolbar() + def close(self): + self.purge_toolbar() + super().close() + def create_menus(self): # macOS doesn't have window-level menus pass @@ -577,20 +583,7 @@ def create_toolbar(self): self.interface.content.refresh() def purge_toolbar(self): - while self._toolbar_items: - dead_items = [] - _, cmd = self._toolbar_items.popitem() - # The command might have toolbar representations on multiple window - # toolbars, and may have other representations (at the very least, a menu - # item). Only clean up the representation pointing at *this* window. Do this - # in 2 passes so that we're not modifying the set of native objects while - # iterating over it. - for item_native in cmd._impl.native: - if ( - isinstance(item_native, NSToolbarItem) - and item_native.target == self.native - ): - dead_items.append(item_native) - - for item_native in dead_items: - cmd._impl.native.remove(item_native) + for item_native, cmd in self.toolbar_items.items(): + cmd._impl.native.remove(item_native) + self.toolbar_items = {} + self._toolbar_items = {} diff --git a/cocoa/tests_backend/app.py b/cocoa/tests_backend/app.py index cb03b62454..0bf3d95640 100644 --- a/cocoa/tests_backend/app.py +++ b/cocoa/tests_backend/app.py @@ -20,6 +20,8 @@ class AppProbe(BaseProbe, DialogsMixin): + supports_application_menu_command_native_items = True + supports_status_icon_command_native_items = True supports_key = True supports_key_mod3 = True supports_current_window_assignment = True diff --git a/cocoa/tests_backend/window.py b/cocoa/tests_backend/window.py index c121ac5a77..1bcc1431c6 100644 --- a/cocoa/tests_backend/window.py +++ b/cocoa/tests_backend/window.py @@ -10,6 +10,7 @@ class WindowProbe(BaseProbe, DialogsMixin): + supports_command_items = True supports_closable = True supports_minimizable = True supports_move_while_hidden = True @@ -126,6 +127,13 @@ def instantaneous_state(self): def has_toolbar(self): return self.native.toolbar is not None + def command_items(self, command): + return { + item + for item, item_command in self.impl.toolbar_items.items() + if item_command is command + } + def assert_is_toolbar_separator(self, index, section=False): # macOS doesn't display separators, so there's nothing to assert. pass diff --git a/gtk/src/toga_gtk/app.py b/gtk/src/toga_gtk/app.py index b98ef0f033..e96ece88ee 100644 --- a/gtk/src/toga_gtk/app.py +++ b/gtk/src/toga_gtk/app.py @@ -53,6 +53,7 @@ def __init__(self, interface): flags=Gio.ApplicationFlags.DEFAULT_FLAGS, ) self.native_about_dialog = None + self._menu_items = {} # Connect the GTK signal that will cause app startup to occur self.native.connect("startup", self.gtk_startup) @@ -121,7 +122,10 @@ def create_menus(self): # application level, and are automatically added to any ApplicationWindow. # (or to the top of the screen if the GTK theme requires) - # Only create the menu if the menu item index has been created. + for action, cmd in self._menu_items.items(): + self.native.remove_action(action.get_name()) + cmd._impl.native.remove(action) + self._menu_items = {} self._menu_groups = {} @@ -138,7 +142,7 @@ def create_menus(self): action = Gio.SimpleAction.new(cmd_id, None) action.connect("activate", cmd._impl.gtk_activate) - cmd._impl.native.append(action) + cmd._impl.native.add(action) cmd._impl.set_enabled(cmd.enabled) self._menu_items[action] = cmd self.native.add_action(action) diff --git a/gtk/src/toga_gtk/command.py b/gtk/src/toga_gtk/command.py index d7e9d01416..c13e4e28a3 100644 --- a/gtk/src/toga_gtk/command.py +++ b/gtk/src/toga_gtk/command.py @@ -4,7 +4,7 @@ class Command: - """Command `native` property is a list of native widgets associated with the + """Command `native` property is a set of native widgets associated with the command. Native widgets can be both Gtk.ToolButton and Gio.SimpleAction. @@ -12,7 +12,7 @@ class Command: def __init__(self, interface): self.interface = interface - self.native = [] + self.native = set() @classmethod def standard(self, app, id): diff --git a/gtk/src/toga_gtk/window.py b/gtk/src/toga_gtk/window.py index 8abb03bfc4..5a8d0b4a77 100644 --- a/gtk/src/toga_gtk/window.py +++ b/gtk/src/toga_gtk/window.py @@ -516,24 +516,32 @@ def create_menus(self): # GTK menus are handled at the app level pass - def create_toolbar(self): - if GTK_VERSION < (4, 0, 0): # pragma: no-cover-if-gtk4 - # If there's an existing toolbar, hide it until we know we need it. - self.layout.remove(self.native_toolbar) - - # Deregister any toolbar buttons from their commands, and remove them - # from the toolbar - for cmd, item_impl in self.toolbar_items.items(): + def purge_toolbar(self): + if GTK_VERSION < (4, 0, 0): # pragma: no-cover-if-gtk4 # pragma: no branch + for item_impl, cmd in self.toolbar_items.items(): self.native_toolbar.remove(item_impl) cmd._impl.native.remove(item_impl) - # Remove any toolbar separators for sep in self.toolbar_separators: self.native_toolbar.remove(sep) - # Create the new toolbar items self.toolbar_items = {} self.toolbar_separators = set() + + def close(self): + self.purge_toolbar() + super().close() + + def create_toolbar(self): + if GTK_VERSION < (4, 0, 0): # pragma: no-cover-if-gtk4 + # If there's an existing toolbar, hide it until we know we need it. + self.layout.remove(self.native_toolbar) + + # Deregister any toolbar buttons from their commands, and remove them + # from the toolbar. + self.purge_toolbar() + + # Create the new toolbar items prev_group = None for cmd in self.interface.toolbar: if isinstance(cmd, Separator): @@ -561,8 +569,8 @@ def create_toolbar(self): if cmd.tooltip: item_impl.set_tooltip_text(cmd.tooltip) item_impl.connect("clicked", cmd._impl.gtk_clicked) - cmd._impl.native.append(item_impl) - self.toolbar_items[cmd] = item_impl + cmd._impl.native.add(item_impl) + self.toolbar_items[item_impl] = cmd self.native_toolbar.insert(item_impl, -1) diff --git a/gtk/tests_backend/app.py b/gtk/tests_backend/app.py index 3d5a3ec69e..315941de2d 100644 --- a/gtk/tests_backend/app.py +++ b/gtk/tests_backend/app.py @@ -13,6 +13,8 @@ class AppProbe(BaseProbe, DialogsMixin): + supports_application_menu_command_native_items = True + supports_status_icon_command_native_items = False supports_key = True supports_key_mod3 = True # Gtk 3.24.41 ships with Ubuntu 24.04 where present() works on Wayland diff --git a/gtk/tests_backend/window.py b/gtk/tests_backend/window.py index 591951e7e0..52ea6fd457 100644 --- a/gtk/tests_backend/window.py +++ b/gtk/tests_backend/window.py @@ -11,6 +11,7 @@ class WindowProbe(BaseProbe, DialogsMixin): + supports_command_items = GTK_VERSION < (4, 0) # GTK defers a lot of window behavior to the window manager, which means some # features either don't exist, or we can't guarantee they behave the way Toga would # like. @@ -144,6 +145,13 @@ def has_toolbar(self): return self.impl.native_toolbar.get_n_items() > 0 pytest.skip("Toolbars not implemented on GTK4") + def command_items(self, command): + return { + item + for item, item_command in self.impl.toolbar_items.items() + if item_command is command + } + def assert_is_toolbar_separator(self, index, section=False): item = self.impl.native_toolbar.get_nth_item(index) assert isinstance(item, Gtk.SeparatorToolItem) diff --git a/iOS/tests_backend/app.py b/iOS/tests_backend/app.py index 661f3688a6..1590664ae4 100644 --- a/iOS/tests_backend/app.py +++ b/iOS/tests_backend/app.py @@ -14,6 +14,8 @@ class AppProbe(BaseProbe, DialogsMixin): + supports_application_menu_command_native_items = False + supports_status_icon_command_native_items = False supports_key = False supports_dark_mode = True edit_menu_noop_enabled = False diff --git a/qt/src/toga_qt/command.py b/qt/src/toga_qt/command.py index f4d0009b4e..bc3de0a812 100644 --- a/qt/src/toga_qt/command.py +++ b/qt/src/toga_qt/command.py @@ -36,14 +36,14 @@ def __call__(self, interface): class Command: """ - Command `native` property is a list of native widgets associated with the command. + Command `native` property is a set of native widgets associated with the command. Native widgets is of type QAction """ def __init__(self, interface): self.interface = interface - self.native = [] + self.native = set() @classmethod def standard(self, app, id): @@ -138,6 +138,6 @@ def create_menu_item(self): item.setEnabled(self.interface.enabled) - self.native.append(item) + self.native.add(item) return item diff --git a/qt/src/toga_qt/statusicons.py b/qt/src/toga_qt/statusicons.py index f4d93ca38b..0dcc873e6d 100644 --- a/qt/src/toga_qt/statusicons.py +++ b/qt/src/toga_qt/statusicons.py @@ -55,6 +55,7 @@ def create_menu(self): class StatusIconSet: def __init__(self, interface): self.interface = interface + self._menu_items = {} def _submenu(self, group, group_cache): try: @@ -74,6 +75,9 @@ def create(self): This gets called on App creation, and then on any changes to the status icon command set. """ + for menu_item, cmd in self._menu_items.items(): + cmd._impl.native.remove(menu_item) + # Menu status icons are the only icons that have extra construction needs. # Clear existing menu items for item in self.interface._menu_status_icons: @@ -93,6 +97,7 @@ def create(self): } # Map the COMMANDS group to the primary status icon's menu. group_cache[Group.COMMANDS] = primary_group._impl.native.contextMenu() + self._menu_items = {} for cmd in self.interface.commands: try: @@ -106,4 +111,6 @@ def create(self): if isinstance(cmd, Separator): submenu.addSeparator() else: - submenu.addAction(cmd._impl.create_menu_item()) + menu_item = cmd._impl.create_menu_item() + submenu.addAction(menu_item) + self._menu_items[menu_item] = cmd diff --git a/qt/src/toga_qt/window.py b/qt/src/toga_qt/window.py index 23f5e25e95..613139f409 100644 --- a/qt/src/toga_qt/window.py +++ b/qt/src/toga_qt/window.py @@ -354,6 +354,26 @@ def set_content(self, widget): class MainWindow(Window): + def create(self): + super().create() + self.menu_items = {} + self.toolbar_items = {} + + def purge_menu_items(self): + for item, cmd in self.menu_items.items(): + cmd._impl.native.remove(item) + self.menu_items = {} + + def purge_toolbar_items(self): + for item, cmd in self.toolbar_items.items(): + cmd._impl.native.remove(item) + self.toolbar_items = {} + + def close(self): + self.purge_menu_items() + self.purge_toolbar_items() + super().close() + def _submenu(self, group, group_cache): try: return group_cache[group] @@ -366,6 +386,8 @@ def _submenu(self, group, group_cache): return submenu def create_menus(self): + self.purge_menu_items() + menubar = self.native.menuBar() menubar.clear() @@ -376,9 +398,13 @@ def create_menus(self): if isinstance(cmd, Separator): submenu.addSeparator() else: - submenu.addAction(cmd._impl.create_menu_item()) + item = cmd._impl.create_menu_item() + submenu.addAction(item) + self.menu_items[item] = cmd def create_toolbar(self): + self.purge_toolbar_items() + if self.interface.toolbar: if self.toolbar_native: self.toolbar_native.clear() @@ -414,7 +440,8 @@ def create_toolbar(self): action.triggered.connect(cmd.action) - cmd._impl.native.append(action) + cmd._impl.native.add(action) + self.toolbar_items[action] = cmd self.toolbar_native.addAction(action) @@ -422,3 +449,4 @@ def create_toolbar(self): self.native.removeToolBar(self.toolbar_native) self.toolbar_native.deleteLater() self.toolbar_native = None + self.toolbar_items = {} diff --git a/qt/tests_backend/app.py b/qt/tests_backend/app.py index d153d45d0e..1aab6047ce 100644 --- a/qt/tests_backend/app.py +++ b/qt/tests_backend/app.py @@ -15,6 +15,8 @@ class AppProbe(BaseProbe): formal_name = "Toga Testbed (Qt)" + supports_application_menu_command_native_items = True + supports_status_icon_command_native_items = True supports_key = True supports_key_mod3 = True supports_current_window_assignment = True diff --git a/qt/tests_backend/window.py b/qt/tests_backend/window.py index 8a96315577..8596ec58f1 100644 --- a/qt/tests_backend/window.py +++ b/qt/tests_backend/window.py @@ -9,6 +9,7 @@ class WindowProbe(BaseProbe): + supports_command_items = True # There *is* a close button hint but it doesn't seem to work # under KDE so we take similar handling as winforms here: disable # the action of the close button. @@ -104,6 +105,14 @@ def instantaneous_state(self): def has_toolbar(self): return self.window._impl.toolbar_native is not None + def command_items(self, command): + return { + item + for items in (self.window._impl.menu_items, self.window._impl.toolbar_items) + for item, item_command in items.items() + if item_command is command + } + def assert_is_toolbar_separator(self, index, section=False): assert self.window._impl.toolbar_native.actions()[index].isSeparator() diff --git a/testbed/tests/app/test_app.py b/testbed/tests/app/test_app.py index e78665f458..5743c129cb 100644 --- a/testbed/tests/app/test_app.py +++ b/testbed/tests/app/test_app.py @@ -115,6 +115,27 @@ async def test_main_window_toolbar(app, main_window, main_window_probe): assert not main_window_probe.has_toolbar() +async def test_command_rebuild_replaces_native_items(app, app_probe): + """Rebuilding menus must release obsolete native command items.""" + if not app_probe.supports_application_menu_command_native_items: + pytest.skip("Application menu commands don't expose native items.") + + command = app.cmd1 + old_items = set(command._impl.native) + assert old_items + + app.commands.add( + toga.Command( + action=None, + text="Rebuild command", + group=toga.Group.FILE, + ) + ) + await app_probe.redraw("Application menus rebuilt") + + assert old_items.isdisjoint(command._impl.native) + + async def test_system_menus(app_probe): """System-specific menus behave as expected""" # Check that the system menus (which can be platform specific) exist. diff --git a/testbed/tests/test_statusicons.py b/testbed/tests/test_statusicons.py index ff9e85985b..62172cb530 100644 --- a/testbed/tests/test_statusicons.py +++ b/testbed/tests/test_statusicons.py @@ -62,6 +62,9 @@ async def test_add_remove(app, app_probe): assert app_probe.status_menu_items(new_status_icon) == [ "New Action 1", ] + if app_probe.supports_status_icon_command_native_items: + first_native_items = set(new_cmd1._impl.native) + assert first_native_items # A second command new_cmd2 = toga.Command( @@ -78,6 +81,9 @@ async def test_add_remove(app, app_probe): "New Action 2", "New Action 1", ] + if app_probe.supports_status_icon_command_native_items: + assert first_native_items.isdisjoint(new_cmd1._impl.native) + assert new_cmd1._impl.native # Remove the first command app.status_icons.commands.remove(new_cmd1) @@ -86,6 +92,8 @@ async def test_add_remove(app, app_probe): assert app_probe.status_menu_items(new_status_icon) == [ "New Action 2", ] + if app_probe.supports_status_icon_command_native_items: + assert not new_cmd1._impl.native # Remove the second command app.status_icons.commands.remove(new_cmd2) diff --git a/testbed/tests/window/test_window.py b/testbed/tests/window/test_window.py index 78497a1863..cc06e9edfe 100644 --- a/testbed/tests/window/test_window.py +++ b/testbed/tests/window/test_window.py @@ -487,6 +487,34 @@ async def test_secondary_window_toolbar(app, second_window, second_window_probe) assert second_window_probe.has_toolbar() await second_window_probe.redraw("Secondary window has a toolbar") + @pytest.mark.parametrize( + "second_window_class, second_window_kwargs", + [(toga.MainWindow, {"title": "Temporary window"})], + ) + async def test_closed_window_releases_native_command_items( + app, second_window, second_window_probe + ): + """Closing a window releases its native command menu and toolbar items.""" + if not second_window_probe.supports_command_items: + skip_on_backends( + toga.backend, + reason="Window command items are not implemented on this backend.", + ) + + command = app.cmd1 + second_window.toolbar.add(command) + second_window.show() + await second_window_probe.wait_for_window("Temporary window created") + + window_items = second_window_probe.command_items(command) + assert window_items + assert window_items <= command._impl.native + + second_window.close() + await second_window_probe.wait_for_window("Temporary window closed") + + assert window_items.isdisjoint(command._impl.native) + @pytest.mark.parametrize( "second_window_class, second_window_kwargs", [ diff --git a/textual/tests_backend/app.py b/textual/tests_backend/app.py index b4a7221024..f1de2511da 100644 --- a/textual/tests_backend/app.py +++ b/textual/tests_backend/app.py @@ -15,6 +15,8 @@ class AppProbe(BaseProbe): + supports_application_menu_command_native_items = False + supports_status_icon_command_native_items = False supports_key = False supports_key_mod3 = False supports_current_window_assignment = True diff --git a/winforms/src/toga_winforms/command.py b/winforms/src/toga_winforms/command.py index 8ff4136385..57e7bd2553 100644 --- a/winforms/src/toga_winforms/command.py +++ b/winforms/src/toga_winforms/command.py @@ -10,7 +10,7 @@ class Command: def __init__(self, interface): self.interface = interface - self.native = [] + self.native = set() @classmethod def standard(self, app, id): @@ -117,6 +117,6 @@ def create_menu_item(self, WinformsClass): item.Enabled = self.interface.enabled - self.native.append(item) + self.native.add(item) return item diff --git a/winforms/src/toga_winforms/statusicons.py b/winforms/src/toga_winforms/statusicons.py index 7da241743e..9416c9e892 100644 --- a/winforms/src/toga_winforms/statusicons.py +++ b/winforms/src/toga_winforms/statusicons.py @@ -76,6 +76,9 @@ def _submenu(self, group, group_cache): return submenu def create(self): + for menu_item, cmd in self._menu_items.items(): + cmd._impl.native.remove(menu_item) + # Menu status icons are the only icons that have extra construction needs. # Clear existing menus for item in self.interface._menu_status_icons: @@ -114,6 +117,7 @@ def create(self): menu_item = "-" else: menu_item = cmd._impl.create_menu_item(ToolStripMenuItem) + self._menu_items[menu_item] = cmd attr = MENU_ATTR if cmd.group.parent is None else SUBMENU_ATTR getattr(submenu, attr).Add(menu_item) diff --git a/winforms/src/toga_winforms/window.py b/winforms/src/toga_winforms/window.py index fdb07bac51..f3b97a4188 100644 --- a/winforms/src/toga_winforms/window.py +++ b/winforms/src/toga_winforms/window.py @@ -514,6 +514,23 @@ class MainWindow(Window): def create(self): super().create() self.toolbar_native = None + self.menu_items = {} + self.toolbar_items = {} + + def purge_menu_items(self): + for item, cmd in self.menu_items.items(): + cmd._impl.native.remove(item) + self.menu_items = {} + + def purge_toolbar_items(self): + for item, cmd in self.toolbar_items.items(): + cmd._impl.native.remove(item) + self.toolbar_items = {} + + def close(self): + self.purge_menu_items() + self.purge_toolbar_items() + super().close() def update_fonts(self): # Update all the native fonts and determine the new preferred sizes. @@ -549,6 +566,8 @@ def _submenu(self, group, group_cache): return submenu def create_menus(self): + self.purge_menu_items() + menubar = self.native.MainMenuStrip if menubar: menubar.Items.Clear() @@ -570,12 +589,15 @@ def create_menus(self): item = "-" else: item = cmd._impl.create_menu_item(WinForms.ToolStripMenuItem) + self.menu_items[item] = cmd submenu.DropDownItems.Add(item) self.resize_content() def create_toolbar(self): + self.purge_toolbar_items() + if self.interface.toolbar: if self.toolbar_native: self.toolbar_native.Items.Clear() @@ -608,11 +630,13 @@ def create_toolbar(self): item.Image = cmd.icon._impl.native.ToBitmap() item.Enabled = cmd.enabled item.Click += WeakrefCallable(cmd._impl.winforms_Click) - cmd._impl.native.append(item) + cmd._impl.native.add(item) + self.toolbar_items[item] = cmd self.toolbar_native.Items.Add(item) elif self.toolbar_native: self.native.Controls.Remove(self.toolbar_native) self.toolbar_native = None + self.toolbar_items = {} self.resize_content() diff --git a/winforms/tests_backend/app.py b/winforms/tests_backend/app.py index 5a699a81bb..0ab1fe7413 100644 --- a/winforms/tests_backend/app.py +++ b/winforms/tests_backend/app.py @@ -21,6 +21,8 @@ class AppProbe(BaseProbe, DialogsMixin): + supports_application_menu_command_native_items = True + supports_status_icon_command_native_items = True supports_key = True supports_key_mod3 = False supports_current_window_assignment = True diff --git a/winforms/tests_backend/window.py b/winforms/tests_backend/window.py index abfe548c7e..0dd2b3bc23 100644 --- a/winforms/tests_backend/window.py +++ b/winforms/tests_backend/window.py @@ -18,6 +18,7 @@ class WindowProbe(BaseProbe, DialogsMixin): + supports_command_items = True # Disabling the close button requires overriding a protected method # (https://stackoverflow.com/a/7301828), which Python.NET doesn't support # (https://github.com/pythonnet/pythonnet/issues/2192). @@ -132,6 +133,14 @@ def _native_toolbar(self): def has_toolbar(self): return self._native_toolbar() is not None + def command_items(self, command): + return { + item + for items in (self.impl.menu_items, self.impl.toolbar_items) + for item, item_command in items.items() + if item_command is command + } + def _native_toolbar_item(self, index): return self._native_toolbar().Items[index]