Repository navigation
fix: keep Manage Timelines from selecting a deleted timeline - #680
Merged
Merged
Conversation
With Manage Timelines open after moving a timeline up or down, File > New or deleting another timeline closed TiLiA through the crash dialog. Each deletion rebuilds the window's list, and clearing the old items could make the current item one whose timeline was just deleted. The window then asked for that timeline, got None, and raised AttributeError in a Qt slot. The list now rebuilds with its signals blocked; its callers set the current row afterwards, which updates the window as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With Manage Timelines open after moving a timeline up or down, File > New or deleting another timeline could close TiLiA through the crash dialog. Whether it did depended on where the moved timeline ended up.
Every timeline deletion rebuilds the window's list.
TimelinesListWidget.update_items()cleared it with its signals live, and while the old items went, the current item could land on the one whose timeline had just been deleted.ManageTimelines.on_list_current_item_changed()then gotNonefromGet.TIMELINEand raisedAttributeErrorin a Qt slot, which the app's exception hook turns into the crash dialog.The list now rebuilds with its signals blocked. Its callers already set the current row afterwards, which updates the window's checkbox and buttons as before.
Regression tests:
tests/ui/windows/test_manage_timelines.py::TesttimelinesChangeWhileOpen::test_timeline_is_deleted_after_reorderingand::test_new_file_after_reordering. Both fail ondevwith theAttributeError. They collect what reachessys.excepthook, since exceptions raised in Qt slots never reach pytest.Repro bundle
The fixture is a Slider timeline followed by two marker timelines, First and Second, with no media and a length of 100 s. It is already built, untracked, in this branch's worktree, with the CLI:
See the bug, on
devatdac53f4c, this branch's base:uv run --directory /home/felipe.dm/dev/worktrees/base-manage-timelines-rebuild --python 3.12 tilia "/home/felipe.dm/dev/worktrees/fix-manage-timelines-rebuild/repro/manage_timelines.tla"See the fix:
uv run --directory /home/felipe.dm/dev/worktrees/fix-manage-timelines-rebuild --python 3.12 tilia "/home/felipe.dm/dev/worktrees/fix-manage-timelines-rebuild/repro/manage_timelines.tla"Checked before opening: a throwaway test that opens this fixture and replays the action with real mouse clicks gets the
AttributeErrorondevand nothing on this branch.Not verified by the bundle: deleting another timeline instead of File > New, which the first regression test covers.
Release-checklist rows: N15. The IDs refer to the checklist copy in tilia-testing (
release-checklist/test-links.csv); the code doesn't mention them.