fix(history): harden sidebar callbacks
- commit
- b5b4cefe6546505e487fec046dbb88f29440082d
- author
- brice · · 2026-06-13 06:51 UTC
- parent
- 9b42f5b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +56 -37
M internal/ui/browser_window.go +12 -1
| @@ -315,7 +315,7 @@ func (a *App) buildHistorySidebarConfig(ctx context.Context, bw *browserWindow) | |||
| 315 | 315 | return a.navigateHistorySidebarSelection(navCtx, bw, url) | |
| 316 | 316 | }, | |
| 317 | 317 | OnOpenInNewPane: func(splitCtx context.Context, url string) error { | |
| 318 | - | if a.wsCoord == nil { | |
| 318 | + | if a.wsCoord == nil || !a.hasBrowserWindow(bw) { | |
| 319 | 319 | return nil | |
| 320 | 320 | } | |
| 321 | 321 | a.activateBrowserWindow(bw) | |
| @@ -328,6 +328,9 @@ func (a *App) buildHistorySidebarConfig(ctx context.Context, bw *browserWindow) | |||
| 328 | 328 | } | |
| 329 | 329 | ||
| 330 | 330 | func (a *App) navigateHistorySidebarSelection(ctx context.Context, bw *browserWindow, url string) error { | |
| 331 | + | if a == nil || bw == nil || !a.hasBrowserWindow(bw) { | |
| 332 | + | return nil | |
| 333 | + | } | |
| 331 | 334 | return a.navigateFromBrowserWindow(ctx, bw, url) | |
| 332 | 335 | } | |
| 333 | 336 | ||
| @@ -401,6 +404,14 @@ func (bw *browserWindow) hideHistorySidebar() { | |||
| 401 | 404 | bw.sidebarVisible = false | |
| 402 | 405 | } | |
| 403 | 406 | ||
| 407 | + | func (a *App) hasBrowserWindow(target *browserWindow) bool { | |
| 408 | + | if a == nil || target == nil || a.browserWindows == nil { | |
| 409 | + | return false | |
| 410 | + | } | |
| 411 | + | bw, ok := a.browserWindows[target.id] | |
| 412 | + | return ok && bw == target | |
| 413 | + | } | |
| 414 | + | ||
| 404 | 415 | func (a *App) registerBrowserWindow(bw *browserWindow) { | |
| 405 | 416 | if bw == nil { | |
| 406 | 417 | return | |
| @@ -315,7 +315,7 @@ func (a *App) buildHistorySidebarConfig(ctx context.Context, bw *browserWindow) | |||||
| 315 | return a.navigateHistorySidebarSelection(navCtx, bw, url) | 315 | return a.navigateHistorySidebarSelection(navCtx, bw, url) | ||
| 316 | }, | 316 | }, | ||
| 317 | OnOpenInNewPane: func(splitCtx context.Context, url string) error { | 317 | OnOpenInNewPane: func(splitCtx context.Context, url string) error { | ||
| 318 | - | if a.wsCoord == nil { | 318 | + | if a.wsCoord == nil || !a.hasBrowserWindow(bw) { |
| 319 | return nil | 319 | return nil | ||
| 320 | } | 320 | } | ||
| 321 | a.activateBrowserWindow(bw) | 321 | a.activateBrowserWindow(bw) | ||
| @@ -328,6 +328,9 @@ func (a *App) buildHistorySidebarConfig(ctx context.Context, bw *browserWindow) | |||||
| 328 | } | 328 | } | ||
| 329 | 329 | ||||
| 330 | func (a *App) navigateHistorySidebarSelection(ctx context.Context, bw *browserWindow, url string) error { | 330 | func (a *App) navigateHistorySidebarSelection(ctx context.Context, bw *browserWindow, url string) error { | ||
| 331 | + | if a == nil || bw == nil || !a.hasBrowserWindow(bw) { | |||
| 332 | + | return nil | |||
| 333 | + | } | |||
| 331 | return a.navigateFromBrowserWindow(ctx, bw, url) | 334 | return a.navigateFromBrowserWindow(ctx, bw, url) | ||
| 332 | } | 335 | } | ||
| 333 | 336 | ||||
| @@ -401,6 +404,14 @@ func (bw *browserWindow) hideHistorySidebar() { | |||||
| 401 | bw.sidebarVisible = false | 404 | bw.sidebarVisible = false | ||
| 402 | } | 405 | } | ||
| 403 | 406 | ||||
| 407 | + | func (a *App) hasBrowserWindow(target *browserWindow) bool { | |||
| 408 | + | if a == nil || target == nil || a.browserWindows == nil { | |||
| 409 | + | return false | |||
| 410 | + | } | |||
| 411 | + | bw, ok := a.browserWindows[target.id] | |||
| 412 | + | return ok && bw == target | |||
| 413 | + | } | |||
| 414 | + | ||||
| 404 | func (a *App) registerBrowserWindow(bw *browserWindow) { | 415 | func (a *App) registerBrowserWindow(bw *browserWindow) { | ||
| 405 | if bw == nil { | 416 | if bw == nil { | ||
| 406 | return | 417 | return | ||
M internal/ui/browser_window_test.go +22 -33
| @@ -907,13 +907,10 @@ func TestRestoreSession_ActiveWindowIndexSyncsState(t *testing.T) { | |||
| 907 | 907 | // History sidebar integration tests | |
| 908 | 908 | // ============================================================================= | |
| 909 | 909 | ||
| 910 | - | // TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndCloses verifies | |
| 911 | - | // that the OnNavigate callback wiring (from initHistorySidebar) calls | |
| 912 | - | // navigateFromBrowserWindow for the owning browser window's active pane, | |
| 913 | - | // then schedules a hide+focus-restore via idle callback. We verify the | |
| 914 | - | // navigation target directly and confirm the idle hide intent is registered | |
| 915 | - | // (without executing GTK idle callbacks). | |
| 916 | - | func TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndCloses(t *testing.T) { | |
| 910 | + | // TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndKeepsSidebar verifies | |
| 911 | + | // that the default OnNavigate callback targets the owning browser window's | |
| 912 | + | // active pane and keeps the sidebar visible. | |
| 913 | + | func TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndKeepsSidebar(t *testing.T) { | |
| 917 | 914 | ctx := context.Background() | |
| 918 | 915 | ||
| 919 | 916 | // Build two browser windows with independent tabs and panes. | |
| @@ -954,25 +951,27 @@ func TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndCloses(t *testing. | |||
| 954 | 951 | app.tabs.Add(tab1) | |
| 955 | 952 | app.tabs.Add(tab2) | |
| 956 | 953 | ||
| 957 | - | navigateURL := "https://example.com" | |
| 954 | + | second.mainWindow = &window.MainWindow{} | |
| 955 | + | second.historySidebar = &component.HistorySidebar{} | |
| 956 | + | second.sidebarVisible = true | |
| 958 | 957 | ||
| 959 | - | // Simulate the initHistorySidebar OnNavigate wiring for the SECOND window: | |
| 960 | - | // it should navigate through the second window's active pane (pane2 → fakeWv2). | |
| 961 | - | err := app.navigateFromBrowserWindow(ctx, second, navigateURL) | |
| 962 | - | require.NoError(t, err, "navigateFromBrowserWindow should succeed") | |
| 958 | + | cfg := app.buildHistorySidebarConfig(ctx, second) | |
| 959 | + | navigateURL := "https://example.com" | |
| 960 | + | err := cfg.OnNavigate(ctx, navigateURL) | |
| 961 | + | require.NoError(t, err, "OnNavigate should succeed") | |
| 963 | 962 | ||
| 964 | 963 | // The second window's webview must have received the navigation. | |
| 965 | 964 | assert.True(t, fakeWv2.loadURICalled, "second window webview should receive navigation") | |
| 966 | 965 | assert.Equal(t, navigateURL, fakeWv2.loadURILastURI) | |
| 966 | + | assert.True(t, second.sidebarVisible, "default history activation should keep the sidebar visible") | |
| 967 | 967 | ||
| 968 | 968 | // The first window's webview must NOT have been touched (stale-focus guard). | |
| 969 | - | assert.False(t, fakeWv1.loadURICalled, "first window webview must not receive navigation from second window call") | |
| 969 | + | assert.False(t, fakeWv1.loadURICalled, "first window webview must not receive navigation from second window callback") | |
| 970 | 970 | } | |
| 971 | 971 | ||
| 972 | 972 | // TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing verifies | |
| 973 | 973 | // that the OnNavigateKeepOpen callback (Ctrl+Enter) navigates the owning window's | |
| 974 | - | // active pane but does NOT close the sidebar. The idle callback that would hide | |
| 975 | - | // the sidebar is NOT scheduled by OnNavigateKeepOpen. | |
| 974 | + | // active pane and keeps the sidebar visible. | |
| 976 | 975 | func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testing.T) { | |
| 977 | 976 | ctx := context.Background() | |
| 978 | 977 | ||
| @@ -999,12 +998,9 @@ func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testi | |||
| 999 | 998 | } | |
| 1000 | 999 | app.tabs.Add(tab) | |
| 1001 | 1000 | ||
| 1002 | - | // This is the key OnNavigateKeepOpen behavior: the sidebar window tracks | |
| 1003 | - | // visible state separately. OnNavigateKeepOpen does NOT call hideAndRestoreFocus. | |
| 1004 | - | // By calling navigateFromBrowserWindow directly we verify the navigation path; | |
| 1005 | - | // the absence of the hide side-effect is the structural guarantee. | |
| 1001 | + | cfg := app.buildHistorySidebarConfig(ctx, bw) | |
| 1006 | 1002 | navigateURL := "https://keep-open.com" | |
| 1007 | - | err := app.navigateFromBrowserWindow(ctx, bw, navigateURL) | |
| 1003 | + | err := cfg.OnNavigateKeepOpen(ctx, navigateURL) | |
| 1008 | 1004 | require.NoError(t, err) | |
| 1009 | 1005 | assert.True(t, fakeWv.loadURICalled) | |
| 1010 | 1006 | assert.Equal(t, navigateURL, fakeWv.loadURILastURI, "Ctrl+Enter navigation should go to the URL") | |
| @@ -1226,10 +1222,9 @@ func TestApp_HistorySidebarToggleHandler_NilFocusedWindowIsNoOp(t *testing.T) { | |||
| 1226 | 1222 | // History sidebar config callbacks: focus restoration on close | |
| 1227 | 1223 | // ============================================================================= | |
| 1228 | 1224 | ||
| 1229 | - | // TestHistorySidebarConfig_OnCloseRestoresFocusToActivePane verifies that | |
| 1230 | - | // the OnClose callback (from initHistorySidebar) hides the sidebar, | |
| 1231 | - | // which is the first step before restores focus to the active pane. | |
| 1232 | - | func TestHistorySidebarConfig_OnCloseRestoresFocusToActivePane(t *testing.T) { | |
| 1225 | + | // TestHistorySidebarConfig_OnCloseHidesSidebar verifies that the OnClose | |
| 1226 | + | // callback hides the sidebar for the owning browser window. | |
| 1227 | + | func TestHistorySidebarConfig_OnCloseHidesSidebar(t *testing.T) { | |
| 1233 | 1228 | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(entity.PaneID("pane-1"))) | |
| 1234 | 1229 | bwTabs := entity.NewTabList() | |
| 1235 | 1230 | bwTabs.Add(tab) | |
| @@ -1247,10 +1242,6 @@ func TestHistorySidebarConfig_OnCloseRestoresFocusToActivePane(t *testing.T) { | |||
| 1247 | 1242 | bw.hideHistorySidebar() | |
| 1248 | 1243 | assert.False(t, bw.sidebarVisible, "sidebar must be hidden by hideAndRestoreFocus") | |
| 1249 | 1244 | ||
| 1250 | - | // The focus restoration (wsView.FocusPane) is called after the hide. | |
| 1251 | - | // We verify the hide part here; the focus restoration relies on | |
| 1252 | - | // activeWorkspaceViewForBrowserWindow which requires full App wiring. | |
| 1253 | - | _ = tab | |
| 1254 | 1245 | } | |
| 1255 | 1246 | ||
| 1256 | 1247 | // ============================================================================= | |
| @@ -1694,9 +1685,9 @@ func TestApp_HistorySidebarConfig_CloseWithNoSidebarIsSafe(t *testing.T) { | |||
| 1694 | 1685 | require.NotPanics(t, func() { cfg.OnClose() }) | |
| 1695 | 1686 | } | |
| 1696 | 1687 | ||
| 1697 | - | // TestApp_HideAndRestoreFocusForBrowserWindow_HidesAndFocuses verifies that | |
| 1698 | - | // hideAndRestoreFocusForBrowserWindow hides the sidebar and restores focus. | |
| 1699 | - | func TestApp_HideAndRestoreFocusForBrowserWindow_HidesAndFocuses(t *testing.T) { | |
| 1688 | + | // TestApp_HideAndRestoreFocusForBrowserWindow_HidesSidebar verifies that | |
| 1689 | + | // hideAndRestoreFocusForBrowserWindow hides the sidebar. | |
| 1690 | + | func TestApp_HideAndRestoreFocusForBrowserWindow_HidesSidebar(t *testing.T) { | |
| 1700 | 1691 | paneID := entity.PaneID("pane-1") | |
| 1701 | 1692 | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(paneID)) | |
| 1702 | 1693 | bwTabs := entity.NewTabList() | |
| @@ -1732,8 +1723,6 @@ func TestApp_HideAndRestoreFocusForBrowserWindow_HidesAndFocuses(t *testing.T) { | |||
| 1732 | 1723 | // Sidebar must be hidden. | |
| 1733 | 1724 | assert.False(t, bw.sidebarVisible, "sidebar must be hidden") | |
| 1734 | 1725 | ||
| 1735 | - | // Focus restoration is called on the wsView (GTK-dependent). | |
| 1736 | - | // We verify the state changes we can check without GTK. | |
| 1737 | 1726 | _ = wsView | |
| 1738 | 1727 | } | |
| 1739 | 1728 | ||
| @@ -907,13 +907,10 @@ func TestRestoreSession_ActiveWindowIndexSyncsState(t *testing.T) { | |||||
| 907 | // History sidebar integration tests | 907 | // History sidebar integration tests | ||
| 908 | // ============================================================================= | 908 | // ============================================================================= | ||
| 909 | 909 | ||||
| 910 | - | // TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndCloses verifies | 910 | + | // TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndKeepsSidebar verifies |
| 911 | - | // that the OnNavigate callback wiring (from initHistorySidebar) calls | 911 | + | // that the default OnNavigate callback targets the owning browser window's |
| 912 | - | // navigateFromBrowserWindow for the owning browser window's active pane, | 912 | + | // active pane and keeps the sidebar visible. |
| 913 | - | // then schedules a hide+focus-restore via idle callback. We verify the | 913 | + | func TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndKeepsSidebar(t *testing.T) { |
| 914 | - | // navigation target directly and confirm the idle hide intent is registered | |||
| 915 | - | // (without executing GTK idle callbacks). | |||
| 916 | - | func TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndCloses(t *testing.T) { | |||
| 917 | ctx := context.Background() | 914 | ctx := context.Background() | ||
| 918 | 915 | ||||
| 919 | // Build two browser windows with independent tabs and panes. | 916 | // Build two browser windows with independent tabs and panes. | ||
| @@ -954,25 +951,27 @@ func TestHistorySidebarConfig_OnNavigateNavigatesActivePaneAndCloses(t *testing. | |||||
| 954 | app.tabs.Add(tab1) | 951 | app.tabs.Add(tab1) | ||
| 955 | app.tabs.Add(tab2) | 952 | app.tabs.Add(tab2) | ||
| 956 | 953 | ||||
| 957 | - | navigateURL := "https://example.com" | 954 | + | second.mainWindow = &window.MainWindow{} |
| 955 | + | second.historySidebar = &component.HistorySidebar{} | |||
| 956 | + | second.sidebarVisible = true | |||
| 958 | 957 | ||||
| 959 | - | // Simulate the initHistorySidebar OnNavigate wiring for the SECOND window: | 958 | + | cfg := app.buildHistorySidebarConfig(ctx, second) |
| 960 | - | // it should navigate through the second window's active pane (pane2 → fakeWv2). | 959 | + | navigateURL := "https://example.com" |
| 961 | - | err := app.navigateFromBrowserWindow(ctx, second, navigateURL) | 960 | + | err := cfg.OnNavigate(ctx, navigateURL) |
| 962 | - | require.NoError(t, err, "navigateFromBrowserWindow should succeed") | 961 | + | require.NoError(t, err, "OnNavigate should succeed") |
| 963 | 962 | ||||
| 964 | // The second window's webview must have received the navigation. | 963 | // The second window's webview must have received the navigation. | ||
| 965 | assert.True(t, fakeWv2.loadURICalled, "second window webview should receive navigation") | 964 | assert.True(t, fakeWv2.loadURICalled, "second window webview should receive navigation") | ||
| 966 | assert.Equal(t, navigateURL, fakeWv2.loadURILastURI) | 965 | assert.Equal(t, navigateURL, fakeWv2.loadURILastURI) | ||
| 966 | + | assert.True(t, second.sidebarVisible, "default history activation should keep the sidebar visible") | |||
| 967 | 967 | ||||
| 968 | // The first window's webview must NOT have been touched (stale-focus guard). | 968 | // The first window's webview must NOT have been touched (stale-focus guard). | ||
| 969 | - | assert.False(t, fakeWv1.loadURICalled, "first window webview must not receive navigation from second window call") | 969 | + | assert.False(t, fakeWv1.loadURICalled, "first window webview must not receive navigation from second window callback") |
| 970 | } | 970 | } | ||
| 971 | 971 | ||||
| 972 | // TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing verifies | 972 | // TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing verifies | ||
| 973 | // that the OnNavigateKeepOpen callback (Ctrl+Enter) navigates the owning window's | 973 | // that the OnNavigateKeepOpen callback (Ctrl+Enter) navigates the owning window's | ||
| 974 | - | // active pane but does NOT close the sidebar. The idle callback that would hide | 974 | + | // active pane and keeps the sidebar visible. |
| 975 | - | // the sidebar is NOT scheduled by OnNavigateKeepOpen. | |||
| 976 | func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testing.T) { | 975 | func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testing.T) { | ||
| 977 | ctx := context.Background() | 976 | ctx := context.Background() | ||
| 978 | 977 | ||||
| @@ -999,12 +998,9 @@ func TestHistorySidebarConfig_OnNavigateKeepOpenNavigatesWithoutClosing(t *testi | |||||
| 999 | } | 998 | } | ||
| 1000 | app.tabs.Add(tab) | 999 | app.tabs.Add(tab) | ||
| 1001 | 1000 | ||||
| 1002 | - | // This is the key OnNavigateKeepOpen behavior: the sidebar window tracks | 1001 | + | cfg := app.buildHistorySidebarConfig(ctx, bw) |
| 1003 | - | // visible state separately. OnNavigateKeepOpen does NOT call hideAndRestoreFocus. | |||
| 1004 | - | // By calling navigateFromBrowserWindow directly we verify the navigation path; | |||
| 1005 | - | // the absence of the hide side-effect is the structural guarantee. | |||
| 1006 | navigateURL := "https://keep-open.com" | 1002 | navigateURL := "https://keep-open.com" | ||
| 1007 | - | err := app.navigateFromBrowserWindow(ctx, bw, navigateURL) | 1003 | + | err := cfg.OnNavigateKeepOpen(ctx, navigateURL) |
| 1008 | require.NoError(t, err) | 1004 | require.NoError(t, err) | ||
| 1009 | assert.True(t, fakeWv.loadURICalled) | 1005 | assert.True(t, fakeWv.loadURICalled) | ||
| 1010 | assert.Equal(t, navigateURL, fakeWv.loadURILastURI, "Ctrl+Enter navigation should go to the URL") | 1006 | assert.Equal(t, navigateURL, fakeWv.loadURILastURI, "Ctrl+Enter navigation should go to the URL") | ||
| @@ -1226,10 +1222,9 @@ func TestApp_HistorySidebarToggleHandler_NilFocusedWindowIsNoOp(t *testing.T) { | |||||
| 1226 | // History sidebar config callbacks: focus restoration on close | 1222 | // History sidebar config callbacks: focus restoration on close | ||
| 1227 | // ============================================================================= | 1223 | // ============================================================================= | ||
| 1228 | 1224 | ||||
| 1229 | - | // TestHistorySidebarConfig_OnCloseRestoresFocusToActivePane verifies that | 1225 | + | // TestHistorySidebarConfig_OnCloseHidesSidebar verifies that the OnClose |
| 1230 | - | // the OnClose callback (from initHistorySidebar) hides the sidebar, | 1226 | + | // callback hides the sidebar for the owning browser window. |
| 1231 | - | // which is the first step before restores focus to the active pane. | 1227 | + | func TestHistorySidebarConfig_OnCloseHidesSidebar(t *testing.T) { |
| 1232 | - | func TestHistorySidebarConfig_OnCloseRestoresFocusToActivePane(t *testing.T) { | |||
| 1233 | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(entity.PaneID("pane-1"))) | 1228 | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(entity.PaneID("pane-1"))) | ||
| 1234 | bwTabs := entity.NewTabList() | 1229 | bwTabs := entity.NewTabList() | ||
| 1235 | bwTabs.Add(tab) | 1230 | bwTabs.Add(tab) | ||
| @@ -1247,10 +1242,6 @@ func TestHistorySidebarConfig_OnCloseRestoresFocusToActivePane(t *testing.T) { | |||||
| 1247 | bw.hideHistorySidebar() | 1242 | bw.hideHistorySidebar() | ||
| 1248 | assert.False(t, bw.sidebarVisible, "sidebar must be hidden by hideAndRestoreFocus") | 1243 | assert.False(t, bw.sidebarVisible, "sidebar must be hidden by hideAndRestoreFocus") | ||
| 1249 | 1244 | ||||
| 1250 | - | // The focus restoration (wsView.FocusPane) is called after the hide. | |||
| 1251 | - | // We verify the hide part here; the focus restoration relies on | |||
| 1252 | - | // activeWorkspaceViewForBrowserWindow which requires full App wiring. | |||
| 1253 | - | _ = tab | |||
| 1254 | } | 1245 | } | ||
| 1255 | 1246 | ||||
| 1256 | // ============================================================================= | 1247 | // ============================================================================= | ||
| @@ -1694,9 +1685,9 @@ func TestApp_HistorySidebarConfig_CloseWithNoSidebarIsSafe(t *testing.T) { | |||||
| 1694 | require.NotPanics(t, func() { cfg.OnClose() }) | 1685 | require.NotPanics(t, func() { cfg.OnClose() }) | ||
| 1695 | } | 1686 | } | ||
| 1696 | 1687 | ||||
| 1697 | - | // TestApp_HideAndRestoreFocusForBrowserWindow_HidesAndFocuses verifies that | 1688 | + | // TestApp_HideAndRestoreFocusForBrowserWindow_HidesSidebar verifies that |
| 1698 | - | // hideAndRestoreFocusForBrowserWindow hides the sidebar and restores focus. | 1689 | + | // hideAndRestoreFocusForBrowserWindow hides the sidebar. |
| 1699 | - | func TestApp_HideAndRestoreFocusForBrowserWindow_HidesAndFocuses(t *testing.T) { | 1690 | + | func TestApp_HideAndRestoreFocusForBrowserWindow_HidesSidebar(t *testing.T) { |
| 1700 | paneID := entity.PaneID("pane-1") | 1691 | paneID := entity.PaneID("pane-1") | ||
| 1701 | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(paneID)) | 1692 | tab := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("ws-1"), entity.NewPane(paneID)) | ||
| 1702 | bwTabs := entity.NewTabList() | 1693 | bwTabs := entity.NewTabList() | ||
| @@ -1732,8 +1723,6 @@ func TestApp_HideAndRestoreFocusForBrowserWindow_HidesAndFocuses(t *testing.T) { | |||||
| 1732 | // Sidebar must be hidden. | 1723 | // Sidebar must be hidden. | ||
| 1733 | assert.False(t, bw.sidebarVisible, "sidebar must be hidden") | 1724 | assert.False(t, bw.sidebarVisible, "sidebar must be hidden") | ||
| 1734 | 1725 | ||||
| 1735 | - | // Focus restoration is called on the wsView (GTK-dependent). | |||
| 1736 | - | // We verify the state changes we can check without GTK. | |||
| 1737 | _ = wsView | 1726 | _ = wsView | ||
| 1738 | } | 1727 | } | ||
| 1739 | 1728 | ||||
M internal/ui/component/history_sidebar.go +19 -0
| @@ -1238,6 +1238,7 @@ func (hs *HistorySidebar) handleDeleteKey() bool { | |||
| 1238 | 1238 | } | |
| 1239 | 1239 | hs.preserveScrollAndSelection() | |
| 1240 | 1240 | hs.prevSelectedURL = nextSelectedURL | |
| 1241 | + | hs.searchGen++ | |
| 1241 | 1242 | hs.removeFromAllEntries(url, entryID) | |
| 1242 | 1243 | hs.removeFromSearchResults(entryID) | |
| 1243 | 1244 | hs.rebuildLocalGroups() | |
| @@ -1577,6 +1578,12 @@ func (hs *HistorySidebar) navigateToURL(url string) { | |||
| 1577 | 1578 | } | |
| 1578 | 1579 | ||
| 1579 | 1580 | navigateCb := glib.SourceFunc(func(uintptr) bool { | |
| 1581 | + | hs.mu.RLock() | |
| 1582 | + | destroyed := hs.destroyed | |
| 1583 | + | hs.mu.RUnlock() | |
| 1584 | + | if destroyed { | |
| 1585 | + | return false | |
| 1586 | + | } | |
| 1580 | 1587 | hs.onURL(hs.ctx, url) | |
| 1581 | 1588 | return false | |
| 1582 | 1589 | }) | |
| @@ -1597,6 +1604,12 @@ func (hs *HistorySidebar) navigateWithoutClosing(url string) { | |||
| 1597 | 1604 | // default activation behavior when they need a distinct keep-open action. | |
| 1598 | 1605 | func (hs *HistorySidebar) doNavigateWithoutClose(url string) { | |
| 1599 | 1606 | navigateCb := glib.SourceFunc(func(uintptr) bool { | |
| 1607 | + | hs.mu.RLock() | |
| 1608 | + | destroyed := hs.destroyed | |
| 1609 | + | hs.mu.RUnlock() | |
| 1610 | + | if destroyed { | |
| 1611 | + | return false | |
| 1612 | + | } | |
| 1600 | 1613 | hs.onNavigateKeepOpen(hs.ctx, url) | |
| 1601 | 1614 | return false | |
| 1602 | 1615 | }) | |
| @@ -1611,6 +1624,12 @@ func (hs *HistorySidebar) navigateToNewPane(url string) { | |||
| 1611 | 1624 | } | |
| 1612 | 1625 | ||
| 1613 | 1626 | navigateCb := glib.SourceFunc(func(uintptr) bool { | |
| 1627 | + | hs.mu.RLock() | |
| 1628 | + | destroyed := hs.destroyed | |
| 1629 | + | hs.mu.RUnlock() | |
| 1630 | + | if destroyed { | |
| 1631 | + | return false | |
| 1632 | + | } | |
| 1614 | 1633 | if err := hs.onOpenInNewPane(hs.ctx, url); err != nil { | |
| 1615 | 1634 | hs.logger.Error().Err(err).Str("url", url).Msg("history sidebar new-pane navigation failed") | |
| 1616 | 1635 | } | |
| @@ -1238,6 +1238,7 @@ func (hs *HistorySidebar) handleDeleteKey() bool { | |||||
| 1238 | } | 1238 | } | ||
| 1239 | hs.preserveScrollAndSelection() | 1239 | hs.preserveScrollAndSelection() | ||
| 1240 | hs.prevSelectedURL = nextSelectedURL | 1240 | hs.prevSelectedURL = nextSelectedURL | ||
| 1241 | + | hs.searchGen++ | |||
| 1241 | hs.removeFromAllEntries(url, entryID) | 1242 | hs.removeFromAllEntries(url, entryID) | ||
| 1242 | hs.removeFromSearchResults(entryID) | 1243 | hs.removeFromSearchResults(entryID) | ||
| 1243 | hs.rebuildLocalGroups() | 1244 | hs.rebuildLocalGroups() | ||
| @@ -1577,6 +1578,12 @@ func (hs *HistorySidebar) navigateToURL(url string) { | |||||
| 1577 | } | 1578 | } | ||
| 1578 | 1579 | ||||
| 1579 | navigateCb := glib.SourceFunc(func(uintptr) bool { | 1580 | navigateCb := glib.SourceFunc(func(uintptr) bool { | ||
| 1581 | + | hs.mu.RLock() | |||
| 1582 | + | destroyed := hs.destroyed | |||
| 1583 | + | hs.mu.RUnlock() | |||
| 1584 | + | if destroyed { | |||
| 1585 | + | return false | |||
| 1586 | + | } | |||
| 1580 | hs.onURL(hs.ctx, url) | 1587 | hs.onURL(hs.ctx, url) | ||
| 1581 | return false | 1588 | return false | ||
| 1582 | }) | 1589 | }) | ||
| @@ -1597,6 +1604,12 @@ func (hs *HistorySidebar) navigateWithoutClosing(url string) { | |||||
| 1597 | // default activation behavior when they need a distinct keep-open action. | 1604 | // default activation behavior when they need a distinct keep-open action. | ||
| 1598 | func (hs *HistorySidebar) doNavigateWithoutClose(url string) { | 1605 | func (hs *HistorySidebar) doNavigateWithoutClose(url string) { | ||
| 1599 | navigateCb := glib.SourceFunc(func(uintptr) bool { | 1606 | navigateCb := glib.SourceFunc(func(uintptr) bool { | ||
| 1607 | + | hs.mu.RLock() | |||
| 1608 | + | destroyed := hs.destroyed | |||
| 1609 | + | hs.mu.RUnlock() | |||
| 1610 | + | if destroyed { | |||
| 1611 | + | return false | |||
| 1612 | + | } | |||
| 1600 | hs.onNavigateKeepOpen(hs.ctx, url) | 1613 | hs.onNavigateKeepOpen(hs.ctx, url) | ||
| 1601 | return false | 1614 | return false | ||
| 1602 | }) | 1615 | }) | ||
| @@ -1611,6 +1624,12 @@ func (hs *HistorySidebar) navigateToNewPane(url string) { | |||||
| 1611 | } | 1624 | } | ||
| 1612 | 1625 | ||||
| 1613 | navigateCb := glib.SourceFunc(func(uintptr) bool { | 1626 | navigateCb := glib.SourceFunc(func(uintptr) bool { | ||
| 1627 | + | hs.mu.RLock() | |||
| 1628 | + | destroyed := hs.destroyed | |||
| 1629 | + | hs.mu.RUnlock() | |||
| 1630 | + | if destroyed { | |||
| 1631 | + | return false | |||
| 1632 | + | } | |||
| 1614 | if err := hs.onOpenInNewPane(hs.ctx, url); err != nil { | 1633 | if err := hs.onOpenInNewPane(hs.ctx, url); err != nil { | ||
| 1615 | hs.logger.Error().Err(err).Str("url", url).Msg("history sidebar new-pane navigation failed") | 1634 | hs.logger.Error().Err(err).Str("url", url).Msg("history sidebar new-pane navigation failed") | ||
| 1616 | } | 1635 | } | ||
M internal/ui/component/history_sidebar_search_test.go +3 -3
| @@ -118,7 +118,7 @@ func TestApplySearchResults_EmptyResultsApplied(t *testing.T) { | |||
| 118 | 118 | } | |
| 119 | 119 | ||
| 120 | 120 | // ============================================================================= | |
| 121 | - | // doFTSearch seam: controllable history port fake | |
| 121 | + | // Async search seam: controllable history port fake | |
| 122 | 122 | // ============================================================================= | |
| 123 | 123 | ||
| 124 | 124 | type fakeHistorySidebarHistory struct { | |
| @@ -149,7 +149,7 @@ func (f *fakeHistorySidebarHistory) Delete(ctx context.Context, id int64) error | |||
| 149 | 149 | return nil | |
| 150 | 150 | } | |
| 151 | 151 | ||
| 152 | - | func TestDoFTSearch_WithFakeUC_StaleGenerationDropsResults(t *testing.T) { | |
| 152 | + | func TestApplySearchResults_StaleGenerationDropsResultsAfterSearch(t *testing.T) { | |
| 153 | 153 | searchCalled := make(chan struct{}, 1) | |
| 154 | 154 | ||
| 155 | 155 | history := &fakeHistorySidebarHistory{ | |
| @@ -187,7 +187,7 @@ func TestDoFTSearch_WithFakeUC_StaleGenerationDropsResults(t *testing.T) { | |||
| 187 | 187 | assert.Nil(t, hs.groups) | |
| 188 | 188 | } | |
| 189 | 189 | ||
| 190 | - | func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | |
| 190 | + | func TestApplySearchResults_CurrentGenerationAppliedAfterSearch(t *testing.T) { | |
| 191 | 191 | searchCalled := make(chan struct{}, 1) | |
| 192 | 192 | ||
| 193 | 193 | history := &fakeHistorySidebarHistory{ | |
| @@ -118,7 +118,7 @@ func TestApplySearchResults_EmptyResultsApplied(t *testing.T) { | |||||
| 118 | } | 118 | } | ||
| 119 | 119 | ||||
| 120 | // ============================================================================= | 120 | // ============================================================================= | ||
| 121 | - | // doFTSearch seam: controllable history port fake | 121 | + | // Async search seam: controllable history port fake |
| 122 | // ============================================================================= | 122 | // ============================================================================= | ||
| 123 | 123 | ||||
| 124 | type fakeHistorySidebarHistory struct { | 124 | type fakeHistorySidebarHistory struct { | ||
| @@ -149,7 +149,7 @@ func (f *fakeHistorySidebarHistory) Delete(ctx context.Context, id int64) error | |||||
| 149 | return nil | 149 | return nil | ||
| 150 | } | 150 | } | ||
| 151 | 151 | ||||
| 152 | - | func TestDoFTSearch_WithFakeUC_StaleGenerationDropsResults(t *testing.T) { | 152 | + | func TestApplySearchResults_StaleGenerationDropsResultsAfterSearch(t *testing.T) { |
| 153 | searchCalled := make(chan struct{}, 1) | 153 | searchCalled := make(chan struct{}, 1) | ||
| 154 | 154 | ||||
| 155 | history := &fakeHistorySidebarHistory{ | 155 | history := &fakeHistorySidebarHistory{ | ||
| @@ -187,7 +187,7 @@ func TestDoFTSearch_WithFakeUC_StaleGenerationDropsResults(t *testing.T) { | |||||
| 187 | assert.Nil(t, hs.groups) | 187 | assert.Nil(t, hs.groups) | ||
| 188 | } | 188 | } | ||
| 189 | 189 | ||||
| 190 | - | func TestDoFTSearch_WithFakeUC_CurrentGenApplied(t *testing.T) { | 190 | + | func TestApplySearchResults_CurrentGenerationAppliedAfterSearch(t *testing.T) { |
| 191 | searchCalled := make(chan struct{}, 1) | 191 | searchCalled := make(chan struct{}, 1) | ||
| 192 | 192 | ||||
| 193 | history := &fakeHistorySidebarHistory{ | 193 | history := &fakeHistorySidebarHistory{ | ||