fix(gui): stop the UI rebuild from destroying the registry's windows

CHANGELOG-INTERNAL.md

@@ -42,6 +42,48 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre
 - Help text and the flag tables in both READMEs now state that the flag is valid on its own.
 - Version bump deliberately NOT taken (convention 34): the owner closes it in `VERSION.txt`.
 
+### Fixed: two swallowed GUI teardown crashes, and the window geometry they were losing
+
+From a field `crashes.ndjson`: two `swallowed`/`debug` records, `TclError: bad window path name
+".!toplevel2"` at `gui/windows.py::_save_geometry` and `TclError: invalid command name ".!label"`
+at `gui/labels.py::_resize`. They share one cause: `App._build_ui` rebuilds the main UI by
+destroying every child of the root window, and a `Toplevel` is a child of the root.
+
+- `App._build_ui` now skips the Toplevels the registry owns (new `WindowManager.toplevels()`). It
+  used to tear the open panel windows down behind the registry's back, so the `windows.rebuild()`
+  that follows a language switch ran `close()` on windows that no longer existed. That cost two
+  things, not one: the TclError above, and the geometry was never saved - the window reopened where
+  it had last been CLOSED, not where the user had just put it.
+- `PanelWindow._save_geometry` returns early when the window is already gone (`winfo_exists`).
+  Nothing in the app should destroy a registry window behind its back now, but the root still takes
+  every Toplevel with it when the app quits.
+- `bind_wraplength._resize` returns early when the label is gone. The `<Configure>` binding lives on
+  the CONTAINER, which routinely outlives the label: the two banners built with
+  `wrapping_label(root, ...)` in `gui/app.py` hang off the root window itself, and every rebuild
+  destroys them and leaves their handler behind. Nothing unbinds it (it is added with `add="+"`, and
+  `Misc.unbind(seq, funcid)` still clears the WHOLE sequence on the oldest Python in the CI matrix -
+  verified only that 3.14 removes just the one binding). So this was never a one-off teardown race:
+  it is one dead handler per rebuild, each recording an entry on every resize from then on.
+- `on_close` already closed the windows before persisting the UI state, so their geometry survives a
+  quit. That ORDER had no test; it has one now.
+- `tests/fake_tk.py` made honest about destruction, or none of this could be caught: `destroy()` now
+  destroys children and leaves the widget DEAD, `winfo_exists()` is real, and `configure()` /
+  `geometry()` raise `TclError` afterwards, the way Tk does. `winfo_exists` HAD to be added rather
+  than inherited: `W.__getattr__` answers any unknown attribute with a no-op returning `None`, so a
+  `winfo_exists()` guard would have read as "destroyed" for every widget on the fake and silently
+  switched off the code it guards in every GUI test.
+- New tests: `tests/test_windows.py::test_a_language_switch_keeps_the_window_alive_and_saves_its_geometry`,
+  `tests/test_windows.py::test_closing_a_window_that_is_already_gone_is_not_a_crash`,
+  `tests/test_windows.py::test_closing_the_app_saves_an_open_window_before_the_root_goes`,
+  `tests/test_gui_release_fixes.py::test_a_resize_after_the_label_is_gone_is_not_a_crash`. All four
+  verified by MUTATION (convention 5): each guard was removed in turn and the test that claims to
+  catch it went red. The `on_close` mutation is what showed the fourth test was originally guarding
+  a duplicate call added in this chunk rather than the real one; the duplicate was removed.
+- Not fixed here: `App._build_ui` also adds a fresh `root.bind("<Configure>", self._on_root_configure,
+  add="+")` on every rebuild and never removes the old one, so that handler multiplies the same way.
+  It holds the App rather than a dead widget, so it costs duplicated work per resize, not crash
+  records. Left alone deliberately, not overlooked.
+
 ### Fixed: the connections "impaired?" column and its row highlight now use ONE signal
 
 - Field report: rows showed orange with the column reading "no" (and "yes" rows with no colour) -

CHANGELOG.md

@@ -36,6 +36,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol
 
 ### Fixed
 
+- **A window you have moved now keeps its place when you switch language.** Changing the language
+  rebuilds the whole main window, and a smaller window open at the time - Settings, for example -
+  was torn down with it. It came back at the size and position it had the last time you closed it,
+  quietly throwing away wherever you had just dragged it. It now stays where you put it.
+
 - **Setting a process target no longer makes the first start pause.** Working out which process
   owns each connection could take a second or two the first time, because it fell back to scanning
   every process on the system. It now resolves only what it needs, so starting - and typing a

beantester/gui/app.py

@@ -308,7 +308,14 @@ def _build_ui(self):
         """Build (or rebuild on a language change) the whole UI."""
         root = self.root
         set_language(self._lang)
+        # A rebuild owns the MAIN window's widgets - not the registry's windows.
+        # A Toplevel is a child of the root too, so destroying every child used to
+        # take the open panel windows with it, leaving WindowManager to close
+        # windows that were already gone (see WindowManager.toplevels).
+        owned = self.windows.toplevels()
         for widget in root.winfo_children():
+            if widget in owned:
+                continue
             widget.destroy()
 
         pad = scaled(14)

beantester/gui/labels.py

@@ -20,6 +20,17 @@ def bind_wraplength(label, container=None, pad=16):
 
     def _resize(event=None):
         try:
+            # The binding lives on the CONTAINER, and the container routinely
+            # outlives the label: App._build_ui() destroys the labels on every
+            # rebuild while the root window they hang off stays. Nothing unbinds
+            # this (it is added with add="+", and unbinding by funcid still clears
+            # the whole sequence on the oldest Python in the CI matrix), so a dead
+            # label is not an edge case here - it is the steady state after the
+            # first language switch. Without this check every resize afterwards
+            # raised TclError("invalid command name") into the crash log, once per
+            # dead label, forever.
+            if not label.winfo_exists():
+                return
             width = int(getattr(event, "width", 0) or holder.winfo_width() or 0)
             if width > scaled(80):
                 label.config(wraplength=max(scaled(80), width - scaled(pad)))

beantester/gui/windows.py

@@ -193,10 +193,21 @@ def _restore_geometry(self):
             crashlog.note(_exc, "gui.windows")
 
     def _save_geometry(self):
-        if self.win is None:
+        """Remember where the user left this window - if it is still there to ask.
+
+        A Toplevel is a CHILD OF ROOT, so anything that destroys the root takes
+        this window with it while ``self.win`` still points at it. Asking a
+        destroyed window for its geometry raises TclError ("bad window path
+        name"), which was then caught and RECORDED - a crash-log entry per close,
+        for a window that was simply already gone.
+        """
+        win = self.win
+        if win is None:
             return
         try:
-            self.app.ui.set(self._state_key(), self.win.geometry())
+            if not win.winfo_exists():
+                return
+            self.app.ui.set(self._state_key(), win.geometry())
         except Exception as _exc:
             crashlog.note(_exc, "gui.windows")
 
@@ -255,3 +266,16 @@ def rebuild(self):
 
     def open_ids(self):
         return [wid for wid, panel in self._open.items() if panel.is_open()]
+
+    def toplevels(self):
+        """The Toplevel widgets this registry currently owns.
+
+        ``App._build_ui`` rebuilds the main UI by destroying every child of the
+        root window - and a Toplevel is a child of the root. That tore the open
+        windows down behind the registry's back, so ``rebuild()`` then ran
+        ``close()`` on windows that no longer existed: their geometry was never
+        saved (after a language switch the window came back where it used to be,
+        not where the user had put it) and reading it raised into the crash log.
+        The rebuild skips these; closing them is the registry's job.
+        """
+        return {panel.win for panel in self._open.values() if panel.win is not None}

tests/fake_tk.py

@@ -46,13 +46,29 @@ def __init__(self, *args, **kw):
         self.grid_info = None
         self.bindings = {}
         self.states = set()         # ttk widget state flags (active, focus, ...)
+        self.alive = True
         master = args[0] if args else kw.get("master")
         self.master = master if isinstance(master, W) else None
         if isinstance(master, W):
             master.children.append(self)
 
+    def _path(self):
+        return f".!{type(self).__name__.lower()}"
+
+    def _alive_or_raise(self, reason="invalid command name"):
+        """Real Tk deletes the widget's command: touching it afterwards raises.
+
+        Only the two calls that the real teardown crashes made - ``configure`` and
+        ``geometry`` - are modelled. The rest stay permissive on purpose: this is a
+        test double, not a second implementation of Tk, and every method that
+        starts raising is a way for an unrelated test to fail for a fake reason.
+        """
+        if not self.alive:
+            raise TclError(f'{reason} "{self._path()}"')
+
     # -- options ------------------------------------------------------------ #
     def configure(self, *a, **kw):
+        self._alive_or_raise()
         self.kw.update(kw)
 
     config = configure
@@ -123,9 +139,31 @@ def unbind(self, sequence, funcid=None):
     unbind_all = unbind
 
     def destroy(self):
+        """Destroy the widget AND its children, and leave it DEAD.
+
+        The old version only unlinked the widget from its parent, so a widget that
+        had been destroyed still answered every call - and a callback poking a dead
+        widget looked perfectly healthy here while raising TclError for the user.
+        Two shipped crashes hid behind exactly that (``gui/labels.py::_resize`` and
+        ``gui/windows.py::_save_geometry``): the handler lives on a CONTAINER that
+        outlives the widget it was given, so it keeps firing after the destroy.
+        """
+        for child in list(self.children):
+            child.destroy()
         if self.master is not None and self in self.master.children:
             self.master.children.remove(self)
         self.children = []
+        self.alive = False
+
+    def winfo_exists(self):
+        """1 while the widget lives, 0 once it is destroyed.
+
+        Explicit, because ``W.__getattr__`` answers any unknown attribute with a
+        no-op returning ``None``: a guard written as ``if not w.winfo_exists()``
+        would then be TRUE for every widget on the fake, and silently turn the
+        code it guards off in every GUI test.
+        """
+        return 1 if self.alive else 0
 
     def winfo_children(self):
         return list(self.children)
@@ -236,8 +274,12 @@ def state(self, spec=None):
         return "normal"
 
     def geometry(self, spec=None):
+        # Tk answers a wm call on a destroyed toplevel with "bad window path
+        # name", not "invalid command name" - and that is the exact message the
+        # recorded _save_geometry crash carried.
+        self._alive_or_raise("bad window path name")
         if spec is None:
-            return self.winfo_geometry()
+            return self.kw.get("geometry") or self.winfo_geometry()
         self.kw["geometry"] = spec
         return None
 

tests/test_gui_release_fixes.py

@@ -262,6 +262,40 @@ def test_long_notes_wrap_instead_of_being_cut():
     """)
 
 
+def test_a_resize_after_the_label_is_gone_is_not_a_crash():
+    """``wrapping_label`` binds <Configure> on the CONTAINER, not on the label.
+
+    The container routinely outlives the label - the two banners at app.py:395 and
+    :403 hang off the root window itself, and every ``_build_ui`` rebuild destroys
+    the labels and leaves their handlers behind. Nothing unbinds them, so the next
+    resize called ``configure`` on a destroyed widget: TclError("invalid command
+    name .!label"), recorded once per dead handler, and one more handler added per
+    rebuild.
+    """
+    run_gui("""
+        import tkinter as tk
+        from beantester import crashlog
+        from beantester.gui.labels import wrapping_label
+
+        recorded = []
+        crashlog.record = lambda exc, **kw: recorded.append(
+            (type(exc).__name__, kw.get("subsystem")))
+
+        holder = tk.Frame(app.root)
+        label = wrapping_label(holder, "a note long enough to wrap")
+        assert label.kw.get("wraplength"), "the label must wrap while it lives"
+
+        handlers = holder.bindings.get("<Configure>") or []
+        assert handlers, "wrapping_label must listen on its container"
+
+        label.destroy()
+        for handler in handlers:                    # the resize still arrives
+            handler(type("Event", (), {"width": 500})())
+
+        assert [r for r in recorded if r[1] == "gui.labels"] == [], recorded
+    """)
+
+
 def test_the_connection_table_has_no_stretch_columns():
     """A stretch column is recomputed by ttk and snaps back after a drag."""
     run_gui("""

tests/test_windows.py

@@ -102,6 +102,107 @@ def build(self, body):
     """, lang="pl")
 
 
+def test_a_language_switch_keeps_the_window_alive_and_saves_its_geometry():
+    """A rebuild must not tear the registry's windows down behind its back.
+
+    ``App._build_ui`` rebuilds by destroying every child of the root window, and a
+    Toplevel is one of those children - so the open windows died there, and the
+    ``WindowManager.rebuild()`` that came next ran ``close()`` on windows that no
+    longer existed. Two things followed: the geometry was never saved (the window
+    reopened where it had been LAST closed, not where the user had just put it),
+    and reading it raised TclError into the crash log - the recorded
+    "bad window path name .!toplevel2", once per language switch.
+    """
+    run_gui("""
+        from beantester import crashlog
+        from beantester.gui.windows import PanelWindow, register_window
+
+        # Filtered by subsystem, not "nothing at all": a language switch logs a
+        # line, and the fake tkinter's log box answers index() with None, which
+        # records an unrelated gui.app entry on master too.
+        recorded = []
+        crashlog.record = lambda exc, **kw: recorded.append(
+            (type(exc).__name__, kw.get("subsystem")))
+
+        class Probe(PanelWindow):
+            ID = "probe_geo"
+            TITLE = "app.tabs.connections"
+            def build(self, body):
+                pass
+
+        register_window(Probe)
+        panel = app.open_window("probe_geo")
+        panel.win.geometry("640x480+11+22")          # the user moved it there
+
+        app.lang_var.set("English")
+        app._switch_language()
+
+        assert [r for r in recorded if r[1] == "gui.windows"] == [], recorded
+        assert app.ui.get("window.probe_geo") == "640x480+11+22", app.ui.get("window.probe_geo")
+        assert app.windows.open_ids() == ["probe_geo"], "the window must come back"
+    """, lang="pl")
+
+
+def test_closing_a_window_that_is_already_gone_is_not_a_crash():
+    """Defence in depth for the recorded "bad window path name .!toplevel2".
+
+    Nothing in the app should destroy a registry window behind its back now, but
+    the root window still takes every Toplevel with it whenever it goes - so
+    ``close()`` has to cope with a window that is already destroyed instead of
+    reading its geometry and recording the TclError.
+    """
+    run_gui("""
+        from beantester import crashlog
+        from beantester.gui.windows import PanelWindow, register_window
+
+        recorded = []
+        crashlog.record = lambda exc, **kw: recorded.append(
+            (type(exc).__name__, kw.get("subsystem")))
+
+        class Probe(PanelWindow):
+            ID = "probe_gone"
+            TITLE = "app.tabs.statistics"
+            def build(self, body):
+                pass
+
+        register_window(Probe)
+        panel = app.open_window("probe_gone")
+        panel.win.destroy()                 # destroyed from under the panel
+
+        panel.close()
+
+        assert [r for r in recorded if r[1] == "gui.windows"] == [], recorded
+        assert not panel.is_open()
+    """)
+
+
+def test_closing_the_app_saves_an_open_window_before_the_root_goes():
+    """Quitting destroys the root, and a destroyed root takes every Toplevel with
+    it - so a window still open at that moment has nowhere left to save from.
+
+    ``on_close`` closes them first for exactly that reason. It had no test, which
+    is how the ORDER (close the windows, then persist, then destroy) could have
+    been rearranged without anything noticing.
+    """
+    run_gui("""
+        from beantester.gui.windows import PanelWindow, register_window
+
+        class Probe(PanelWindow):
+            ID = "probe_quit"
+            TITLE = "app.tabs.statistics"
+            def build(self, body):
+                pass
+
+        register_window(Probe)
+        panel = app.open_window("probe_quit")
+        panel.win.geometry("500x400+7+8")
+
+        app.on_close()
+
+        assert app.ui.get("window.probe_quit") == "500x400+7+8", app.ui.get("window.probe_quit")
+    """)
+
+
 def test_a_broken_window_cannot_kill_the_tick():
     """The tick drains the log and moves the statistics. One bad window must not
     take it down - that was the whole point of wrapping _tick in try/finally."""