Release stale native menu command items on rebuild

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

changes/4580.bugfix.md

@@ -0,0 +1 @@
+Native menu and toolbar command instances are now released when their containers are rebuilt or closed.

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 = {}

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

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

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)

gtk/src/toga_gtk/command.py

@@ -4,15 +4,15 @@
 
 
 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.
     """
 
     def __init__(self, interface):
         self.interface = interface
-        self.native = []
+        self.native = set()
 
     @classmethod
     def standard(self, app, id):

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)
 

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

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)

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

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

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

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,11 +440,13 @@ 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)
 
         elif self.toolbar_native:
             self.native.removeToolBar(self.toolbar_native)
             self.toolbar_native.deleteLater()
             self.toolbar_native = None
+            self.toolbar_items = {}

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

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()