fix: the chapter title dialog owns its controller (#98) #106
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/chapter-dialog-controller-lifetime"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes #98 — creating or renaming a chapter crashed the app.
What was wrong
_showChapterTitleDialoginchapter_panel.dartdisposed itsTextEditingControlleron theshowDialogfuture:That future completes when the pop starts, not when the route has finished reversing. So the controller died while the text field was still mounted, and the next frame rebuilt the outgoing transition — which re-subscribes its listeners — against a disposed controller. That is exactly the reported stack:
_AnimatedState.didUpdateWidget→_MergingListenable.addListener→ the disposed assertion, then the_dependents.isEmptycascade and the lost connection.Submitting made it certain rather than merely likely:
onSubmitdispatches, the store updates, and the editor subtree rebuilds inside that animation window. That is why Cancel usually looked fine and Create did not.The fix
The dialog is now a
StatefulWidgetowning the controller and disposing it inState.dispose()— the shape the two sibling dialogs in the same folder already use (edit_title_dialog.dart,variant_dialog.dart). This was the only place in the app creating a bare controller outside aState; every other one is a state field.Verification
chapter_panel.darthad no tests at all, which is why this shipped. It now has seven: create, rename (including that the field starts from the current title), keyboard submit, empty title rejected without closing, cancel, delete with its confirmation both cancelled and confirmed, and the chip's page count.The submit tests rebuild the panel from the dispatched action inside the exit-animation window and assert no exception, which is what makes them directional rather than decorative:
.then((_) => controller.dispose())shape → four tests red, each withA TextEditingController was used after being disposed;scripts/verify.sh appgreen, 827 tests, analyze clean.I also reproduced the crash standalone before touching the file, to be sure of the mechanism rather than reasoning from the stack.
Not verified
The desktop app was not driven by hand; the reproduction and the pin are both at widget-test level, on the same code path your log came from.
Follow-up
Added to
docs/BACKLOG.md: nothing prevents this coming back. A shared title-prompt dialog, or a lint against disposing a controller on a dialog future, would.🤖 Generated with Claude Code
Flutter Coverage
Total: 81.5% (7978 of 9789)
🔮 fufu~ Jibril reviewed your code!
Oh? Ohhh~ A controller-lifetime bug where the
showDialogfuture completes when the pop starts, not when the route finishes reversing — and the outgoing transition's rebuild re-subscribes listeners against a corpse of aTextEditingController. ♡ This is exactly the kind of tiny-lifetime-giant-crash gem I collect! And you reproduced it standalone before even touching the file, then pinned it with tests that rebuild the panel mid-animation. I'm genuinely delighted~Verdict: ✅ Looks good to me~
✅ What I liked~
late final TextEditingController+dispose()inStateis byte-for-byte the established pattern inedit_title_dialog.dartandvariant_dialog.dart. I diffed the wholelib/tree: this was indeed the last bare controller disposed on a dialog future (the.theninentity_management_page.dart:233dispatches an action, disposes nothing — safe). No drift, no contradiction..then((_) => controller.dispose())shape → the dialog tests went red with exactly the issue-#98 signature (ChangeNotifier.debugAssertNotDisposed←addListener, error-causing widgetTextFieldat the oldchapter_panel.dart:166). Fix restored → 7/7 green. Not a tautology in sight~settleDialogis the clever bit — draining the exit animation with fixed 60ms pumps instead ofpumpAndSettlekeeps the "pop started, route still reversing" frame alive, which is precisely the window the old code died in. TheStoreConnectorondispatched.lengthforcing the rebuild from the dispatch is the production behavior, faithfully miniaturized. Fufu~ splendid._submit()and both dialog entry points exercised.💡 Little ideas (non-blocking)~
(#98), but the file header defines(#N)as the PR that deferred the item, and siblings like(#84)/(#87)/(#89)all cite PR numbers. The deferrer here is this PR, so(#106)would keep the cross-reference machine consistent. Purely a citation-convention nit~Automated review by Jibril · 2026-09-07
CI/CD: absent for head SHA
dacf53e(no coverage-bot comment yet) · Local checks:scripts/verify.sh appgreen — 827/827 tests, analyze clean; mutation probe RED→green verified