Reload the previous directory's list after a failed navigation

Navigating into an unreadable directory failed cleanly in the header
(handleError reverts CurrentPath to the previous path) but left the
list empty forever: the recovery re-read of the previous directory
found it unchanged, so handleBuildIndexSuccess took the no-change
fast path — which assumes the view already shows that content — while
the failed navigation's navigateToDirectory reset had just wiped
TotalEntries/Pages. No frame was emitted and the browser sat on a
correct header over a dead list (seen on device tapping ".." into
/storage/emulated, which is media_rw:media_rw 0750; only
/storage/emulated/0 is app-exposed).

The fast path now requires a live view (TotalEntries != 0); a
torn-down view always takes the full rebuild, restoring the rows.
Regression test TestHandleResult_NavigationFailureRecoveryReloads
(red without the fix, green with it); verified on emulator and phone.
This commit is contained in:
Greg Pomerantz 2026-09-03 12:53:23 -04:00
parent ef54486683
commit 4c8a13b779
2 changed files with 85 additions and 1 deletions

View File

@ -190,7 +190,14 @@ func (bm *BrowserManager) handleBuildIndexSuccess(result pool.Result) bool {
// name, size, mtime, isdir — and excludes the synthetic ".." row, whose // name, size, mtime, isdir — and excludes the synthetic ".." row, whose
// mtime is rebuilt on every pass and would otherwise always differ. // mtime is rebuilt on every pass and would otherwise always differ.
sig := directorySignature(browserEntries) sig := directorySignature(browserEntries)
if bm.lastIndexPath == bm.state.CurrentPath && bm.lastIndexSignature == sig { // The no-change fast path assumes the view already shows this content.
// A failed navigation's recovery (handleError) breaks that: it resets the
// rows (navigateToDirectory zeros TotalEntries/Pages) and re-reads the
// previous directory, whose index is unchanged — without this guard the
// re-read would take the fast path and the browser would sit on an empty
// list forever. A torn-down view always takes the full rebuild below.
if bm.state.TotalEntries != 0 &&
bm.lastIndexPath == bm.state.CurrentPath && bm.lastIndexSignature == sig {
return false return false
} }
bm.lastIndexPath = bm.state.CurrentPath bm.lastIndexPath = bm.state.CurrentPath

View File

@ -474,3 +474,80 @@ func TestHandleResult_UnknownType(t *testing.T) {
// Should not panic // Should not panic
bm.HandleResult(result) bm.HandleResult(result)
} }
// TestHandleResult_NavigationFailureRecoveryReloads reproduces the empty-list
// bug: navigating into an unreadable directory fails, the manager reverts to
// the previous path and re-reads it; that re-read finds the directory
// UNCHANGED, and the no-change fast path must not skip restoring the rows the
// failed navigation's reset wiped — or the browser sits on an empty list
// forever (header correct, list dead).
func TestHandleResult_NavigationFailureRecoveryReloads(t *testing.T) {
state := NewBrowserState()
fs := mock.NewFileSystem()
wp := pool.NewWorkerPool(1)
// Started (unlike the direct-result tests above): the recovery path
// dispatches REAL follow-up tasks (BuildIndex, LoadPages), and three of
// them would overflow the unstarted pool's high-priority channel.
wp.Start()
defer wp.Stop()
bm, err := NewBrowserManager(state, wp, fs)
if err != nil {
t.Fatalf("NewBrowserManager failed: %v", err)
}
// A FIXED mtime: the two index results must carry byte-identical content
// (same directorySignature) so the recovery re-read genuinely hits the
// no-change fast path that the bug lived in.
fixed := time.Date(2026, 5, 15, 10, 0, 0, 0, time.UTC)
entries := func() []types.DirEntry {
return []types.DirEntry{
testDirEntry{name: "file1.txt", size: 8, mod: fixed},
testDirEntry{name: "subdir", isDir: true, mod: fixed},
}
}
indexResult := func(dirPath string) pool.Result {
return pool.Result{TaskType: pool.TypeBuildIndex, Success: true, DirPath: dirPath, Data: entries()}
}
// Land in /test/docs with a populated view.
state.CurrentPath = "/test/docs"
bm.indexInFlightPath = "/test/docs"
bm.indexInFlightIsNav = true
if !bm.HandleResult(indexResult("/test/docs")) {
t.Fatal("initial index did not change the view")
}
if state.TotalEntries != 3 { // 2 entries + ".."
t.Fatalf("TotalEntries = %d, want 3", state.TotalEntries)
}
// The user taps into an unreadable directory (as NavigateTo would set up).
state.History = append(state.History, state.CurrentPath)
navigateToDirectory(state, "/missing")
state.Loading = true
bm.indexInFlightPath = "/missing"
bm.indexInFlightIsNav = true
// That read fails: the manager must revert to /test/docs.
failed := pool.Result{TaskType: pool.TypeBuildIndex, Success: false, Error: ErrDirNotFound, DirPath: "/missing"}
bm.HandleResult(failed)
if state.CurrentPath != "/test/docs" {
t.Fatalf("CurrentPath = %q, want reverted /test/docs", state.CurrentPath)
}
if state.TotalEntries != 0 {
t.Fatalf("TotalEntries = %d after failed navigation, want 0 (rows reset)", state.TotalEntries)
}
// The recovery re-read of /test/docs lands with the directory unchanged.
// This is where the bug lived: the no-change fast path returned early and
// the view stayed empty.
if !bm.HandleResult(indexResult("/test/docs")) {
t.Fatal("recovery re-read reported no view change; the list would stay empty")
}
if state.TotalEntries != 3 {
t.Fatalf("TotalEntries = %d after recovery re-read, want 3", state.TotalEntries)
}
if page, ok := state.Pages[0]; !ok || page == nil || !page.Loaded {
t.Fatal("page 0 not restored after recovery re-read")
}
}