Skip to content

Roll back an undo or redo whose NavigationChanged handler throws [patch] - #112

Open
matt-edmondson wants to merge 2 commits into
mainfrom
fix/75-handler-throws-during-undo
Open

matt-edmondson wants to merge 2 commits into
mainfrom
fix/75-handler-throws-during-undo

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #75

What was wrong

Navigation<T>.RestoreState swapped in the restored items and index first, and raised NavigationChanged after. When a handler threw, SimpleUndoRedoProvider put the action back as though it had never run, but the stack had already moved. After a failed undo the page was undone, CanRedo was false, and the next undo appeared to do nothing. A failed redo had the same problem in reverse.

Change

RestoreState keeps the items and index from before the restore. If a handler throws, it puts the stack back to that state under the lock and rethrows. This keeps the provider's documented assumption that an action that throws changed nothing. The method's remarks now say so.

Tests

  • Undo_WhenAHandlerThrows_LeavesTheStackAndTheHistoryInAgreement: after the throw the stack is still A,B,C at C with 3 undo entries and nothing to redo, and the next undo goes to B.
  • Redo_WhenAHandlerThrows_LeavesTheStackAndTheHistoryInAgreement is the mirror case for redo.

Both fail on main (the stack is already undone or redone) and pass with the fix. Full suite: 67 tests, 65 passed, 2 skipped (the unreadable-file tests are inconclusive when run as root).

The branch has a second commit that reverts a .gitignore change the local build made. The net diff touches only the two files above.


Generated by Claude Code

RestoreState swapped in the restored items and index, then raised
NavigationChanged. When a handler threw, SimpleUndoRedoProvider put the
action back as though it had never run, but the stack had already moved,
so the stack and its history disagreed: after a failed undo the page was
undone while Redo stayed disabled and the next undo did nothing visible.

RestoreState now puts the stack back to its state before the restore when
a handler throws, then rethrows, which keeps the provider's assumption
that an action that throws changed nothing.

Fixes #75

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW5pmswBjLMVcM1HgMb
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW5pmswBjLMVcM1HgMb
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When a NavigationChanged handler throws during Undo, the stack is already undone but the provider records the action as not undone, so Redo is disabled

2 participants