fix(ui): protected-bands edit dialog opened EMPTY — grab_set before viewable #46

Merged
ykp merged 1 commit from fix-protected-dialog-empty into master 2026-09-28 21:08:06 +02:00
Owner

Why the dialog was empty

ProtectedBandsDialog.__init__ called grab_set() immediately after creating the Toplevel — before the body was built and before the window was mapped. On Windows (and any WM where the toplevel isn't yet viewable) grab_set() raises TclError: grab failed: window not viewable; the constructor aborted before _build_body() ran, leaving the dialog window open and completely empty. The button-callback traceback goes to an invisible stderr (pythonw), so the only visible symptom was an empty "Protected bands" window.

The CI widget tier never caught it because the edit test monkeypatched the constructor away — the real construction path had zero coverage.

Fix

  1. Dialog — build the body first, then the modal grab (the standard viewable-then-grab pattern):
    protocol(WM_DELETE_WINDOW → cancel) → deiconify() → wait_visibility() → grab_set() → focus_set().
  2. Main window — _on_edit_protected_bands now blocks with wait_window(dialog): proper modal semantics, keeps a live reference to the dialog for its whole life (an unreferenced Toplevel can be garbage-collected — a second classic bug the old code was exposed to), and refreshes the count label after the dialog closes (matching the documented commit-on-OK behavior; the previous code refreshed while the dialog was still open).
  3. Tests — the fake-dialog test returns an already-destroyed Toplevel so wait_window returns at once; two new real-dialog tests (no monkeypatch, CI widget tier): the dialog opens with content (tree rows, status label, copy semantics — add-on-copy leaves the project untouched until OK), OK commits, cancel/WM-close discards. These exercise the fixed construction path end-to-end.

Gate

  • pytest: 1278 passed / 64 skipped (headless tier), coverage 93.51 %
  • ruff + mypy: clean
  • The two real-dialog tests run in the CI widget tier (tkinter + display) — they would have caught this bug.
## Why the dialog was empty `ProtectedBandsDialog.__init__` called **`grab_set()` immediately after creating the Toplevel — before the body was built and before the window was mapped**. On Windows (and any WM where the toplevel isn't yet viewable) `grab_set()` raises `TclError: grab failed: window not viewable`; the constructor **aborted before `_build_body()` ran**, leaving the dialog window open and completely empty. The button-callback traceback goes to an invisible stderr (`pythonw`), so the only visible symptom was an empty "Protected bands" window. The CI widget tier never caught it because the edit test **monkeypatched the constructor away** — the real construction path had zero coverage. ## Fix 1. **Dialog** — build the body first, then the modal grab (the standard viewable-then-grab pattern): `protocol(WM_DELETE_WINDOW → cancel)` → `deiconify()` → `wait_visibility()` → `grab_set()` → `focus_set()`. 2. **Main window** — `_on_edit_protected_bands` now blocks with `wait_window(dialog)`: proper modal semantics, keeps a **live reference** to the dialog for its whole life (an unreferenced Toplevel can be garbage-collected — a second classic bug the old code was exposed to), and refreshes the count label **after** the dialog closes (matching the documented commit-on-OK behavior; the previous code refreshed while the dialog was still open). 3. **Tests** — the fake-dialog test returns an already-destroyed Toplevel so `wait_window` returns at once; **two new real-dialog tests** (no monkeypatch, CI widget tier): the dialog opens with content (tree rows, status label, copy semantics — add-on-copy leaves the project untouched until OK), OK commits, cancel/WM-close discards. These exercise the fixed construction path end-to-end. ## Gate - `pytest`: **1278 passed / 64 skipped** (headless tier), coverage **93.51 %** - `ruff` + `mypy`: clean - The two real-dialog tests run in the CI widget tier (tkinter + display) — they would have caught this bug.
fix(ui): protected-bands edit dialog opened EMPTY — grab_set before viewable (TclError aborted construction)
Some checks failed
test / test (ubuntu-latest) (pull_request) Failing after 2m44s
f4fb011e40
Root cause: ProtectedBandsDialog.__init__ called grab_set() immediately
after creating the Toplevel — BEFORE the body was built and before the
window was mapped. On Windows (and any WM where the toplevel is not yet
viewable) grab_set() raises TclError: 'grab failed: window not
viewable'; the constructor aborted before _build_body(), leaving the
dialog window open and EMPTY (the button-callback traceback goes to an
invisible stderr). The CI widget tier never caught it because the edit
test monkeypatched the constructor away.

Fix:
- dialog: build the body FIRST, then the modal grab —
  protocol(WM_DELETE_WINDOW → cancel), deiconify(), wait_visibility(),
  grab_set(), focus_set() (the standard viewable-then-grab pattern).
- main window: _on_edit_protected_bands now blocks with
  wait_window(dialog) — proper modal semantics, keeps a live reference
  to the dialog for its whole life (an unreferenced Toplevel can be
  garbage-collected), and refreshes the count label AFTER the dialog
  closes (matching the documented commit-on-OK behavior; the previous
  code refreshed while the dialog was still open).
- tests: the fake-dialog test returns an already-destroyed Toplevel so
  wait_window returns at once; two NEW real-dialog tests (no
  monkeypatch — CI widget tier): the dialog opens with content (tree
  rows, status label, copy semantics — add on the copy leaves the
  project untouched until OK), OK commits the copy, and cancel/WM-close
  discards. These exercise the fixed construction path end-to-end.

Gate: pytest 1278 passed / 64 skipped (headless tier; the new real-
dialog tests run in the CI widget tier), coverage 93.51 %; ruff + mypy
clean.
ykp merged commit 5474088858 into master 2026-09-28 21:08:06 +02:00
ykp deleted branch fix-protected-dialog-empty 2026-09-28 21:08:07 +02:00
Sign in to join this conversation.
No description provided.