From 4c8a13b7799b37d39e68ea644576ac804005598e Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Thu, 3 Sep 2026 12:53:23 -0400 Subject: [PATCH] Reload the previous directory's list after a failed navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/browser/manager.go | 9 +++- internal/browser/manager_test.go | 77 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/internal/browser/manager.go b/internal/browser/manager.go index 7955ee2..421c3b0 100644 --- a/internal/browser/manager.go +++ b/internal/browser/manager.go @@ -190,7 +190,14 @@ func (bm *BrowserManager) handleBuildIndexSuccess(result pool.Result) bool { // name, size, mtime, isdir — and excludes the synthetic ".." row, whose // mtime is rebuilt on every pass and would otherwise always differ. 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 } bm.lastIndexPath = bm.state.CurrentPath diff --git a/internal/browser/manager_test.go b/internal/browser/manager_test.go index 4936380..7c0c372 100644 --- a/internal/browser/manager_test.go +++ b/internal/browser/manager_test.go @@ -474,3 +474,80 @@ func TestHandleResult_UnknownType(t *testing.T) { // Should not panic 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") + } +}