Skip to content

A NavigationChanged handler that redirects during Undo erases the redo history and leaves Undo() returning true forever without changing anything #95

Closed

Description

@matt-edmondson

What happens

Undo restores state through RestoreState (Navigation/Services/NavigationStack.cs:315), which raises NavigationChanged while UndoRedoService.Undo() (SimpleUndoRedoProvider.cs:66-89) is still running. If a handler navigates in response — the common "redirect" pattern — its NavigateTo registers a new undo action (NavigationStack.cs:84-107, 347-356) in the middle of the undo. Recording a new action clears the redo stack, and the provider ends up with an undo entry that just re-applies the redirect.

Repro (HEAD 9a11451), handler if (e.CurrentItem?.Id == "login") nav.NavigateTo(home);, then navigate to login (redirects to home), then to settings, then Undo repeatedly:

history: login,home,settings current=settings
Undo True: history=login,home current=home   undo=[Login | Home] redo=[Settings]
Undo True: history=login,home current=home   undo=[Login | Home] redo=[]
Undo True: history=login,home current=home   undo=[Login | Home] redo=[]
Redo False

Why it matters

After the first undo, CanUndo never becomes false and every Undo() returns true while nothing visible changes, so an Undo button stays enabled and does nothing; the "Settings" redo entry is silently lost. Auth/route-guard redirects in NavigationChanged are a standard use of the event, so this isn't an exotic handler.

Suggested fix

Don't record navigations made while an undo or redo is being applied — the usual guard in undo frameworks:

  • set an "applying undo/redo" flag around RestoreState, and have the undo-registration path skip recording while it is set; or
  • raise NavigationChanged only after the provider has finished the undo/redo.

If the current behaviour is intended, document that handlers must not navigate during undo/redo.

Acceptance criteria

  • With the redirect handler above, repeated Undo() eventually returns false, and Redo() after the first undo restores "settings".
  • A test covers a handler that navigates during Undo and during Redo.

Related but distinct: #75 (a handler that throws during Undo), #90 (handler deadlock under the lock), #89 (concurrency with a shared provider).

Activity

  1. matt-edmondson commented on Oct 29, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

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

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions