fix: clean up stale multi-window state
Keep browser-window teardown, title routing, and relay timeout handling from leaving ambiguous or orphaned UI state after review edge cases.
- commit
- 53e2aa321ac07733024b35fcc166827f84471b06
- author
- brice · · 2026-04-15 07:20 UTC
- parent
- f7b7a96
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +188 -22
M internal/infrastructure/desktop/browser_launch_relay.go +1 -1
| @@ -81,7 +81,7 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||
| 81 | 81 | return false, ctxErr | |
| 82 | 82 | } | |
| 83 | 83 | if _, ok := ctx.Deadline(); !ok { | |
| 84 | - | return false, nil | |
| 84 | + | return true, nil | |
| 85 | 85 | } | |
| 86 | 86 | if deadlineErr := setBrowserLaunchConnDeadline(ctx, conn); deadlineErr != nil { | |
| 87 | 87 | return false, deadlineErr | |
| @@ -81,7 +81,7 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||||
| 81 | return false, ctxErr | 81 | return false, ctxErr | ||
| 82 | } | 82 | } | ||
| 83 | if _, ok := ctx.Deadline(); !ok { | 83 | if _, ok := ctx.Deadline(); !ok { | ||
| 84 | - | return false, nil | 84 | + | return true, nil |
| 85 | } | 85 | } | ||
| 86 | if deadlineErr := setBrowserLaunchConnDeadline(ctx, conn); deadlineErr != nil { | 86 | if deadlineErr := setBrowserLaunchConnDeadline(ctx, conn); deadlineErr != nil { | ||
| 87 | return false, deadlineErr | 87 | return false, deadlineErr | ||
M internal/infrastructure/desktop/browser_launch_relay_test.go +2 -2
| @@ -312,7 +312,7 @@ func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RespectsResponseReadTimeout(t | |||
| 312 | 312 | assert.False(t, delivered) | |
| 313 | 313 | } | |
| 314 | 314 | ||
| 315 | - | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_ReturnsFalseOnNoDeadlineTimeout(t *testing.T) { | |
| 315 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_ReturnsTrueOnNoDeadlineTimeout(t *testing.T) { | |
| 316 | 316 | runtimeDir := shortTempDir(t) | |
| 317 | 317 | socketPath := filepath.Join(runtimeDir, "browser-launch.sock") | |
| 318 | 318 | require.NoError(t, os.MkdirAll(runtimeDir, 0o700)) | |
| @@ -354,7 +354,7 @@ func TestBrowserLaunchRelay_DeliverOpenFreshWindow_ReturnsFalseOnNoDeadlineTimeo | |||
| 354 | 354 | ||
| 355 | 355 | select { | |
| 356 | 356 | case got := <-result: | |
| 357 | - | assert.False(t, got.delivered) | |
| 357 | + | assert.True(t, got.delivered) | |
| 358 | 358 | require.NoError(t, got.err) | |
| 359 | 359 | case <-time.After(300 * time.Millisecond): | |
| 360 | 360 | t.Fatal("DeliverOpenFreshWindow blocked without a caller deadline") | |
| @@ -312,7 +312,7 @@ func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RespectsResponseReadTimeout(t | |||||
| 312 | assert.False(t, delivered) | 312 | assert.False(t, delivered) | ||
| 313 | } | 313 | } | ||
| 314 | 314 | ||||
| 315 | - | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_ReturnsFalseOnNoDeadlineTimeout(t *testing.T) { | 315 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_ReturnsTrueOnNoDeadlineTimeout(t *testing.T) { |
| 316 | runtimeDir := shortTempDir(t) | 316 | runtimeDir := shortTempDir(t) | ||
| 317 | socketPath := filepath.Join(runtimeDir, "browser-launch.sock") | 317 | socketPath := filepath.Join(runtimeDir, "browser-launch.sock") | ||
| 318 | require.NoError(t, os.MkdirAll(runtimeDir, 0o700)) | 318 | require.NoError(t, os.MkdirAll(runtimeDir, 0o700)) | ||
| @@ -354,7 +354,7 @@ func TestBrowserLaunchRelay_DeliverOpenFreshWindow_ReturnsFalseOnNoDeadlineTimeo | |||||
| 354 | 354 | ||||
| 355 | select { | 355 | select { | ||
| 356 | case got := <-result: | 356 | case got := <-result: | ||
| 357 | - | assert.False(t, got.delivered) | 357 | + | assert.True(t, got.delivered) |
| 358 | require.NoError(t, got.err) | 358 | require.NoError(t, got.err) | ||
| 359 | case <-time.After(300 * time.Millisecond): | 359 | case <-time.After(300 * time.Millisecond): | ||
| 360 | t.Fatal("DeliverOpenFreshWindow blocked without a caller deadline") | 360 | t.Fatal("DeliverOpenFreshWindow blocked without a caller deadline") | ||
M internal/ui/app.go +32 -1
| @@ -733,6 +733,29 @@ func (a *App) openFreshWindowWithTabsUC(ctx context.Context, url string, created | |||
| 733 | 733 | a.createWorkspaceView(ctx, output.Tab) | |
| 734 | 734 | a.switchWorkspaceView(ctx, output.Tab.ID) | |
| 735 | 735 | } | |
| 736 | + | if a.workspaceViews[output.Tab.ID] == nil { | |
| 737 | + | if a.tabs != nil { | |
| 738 | + | a.tabs.Remove(output.Tab.ID) | |
| 739 | + | } | |
| 740 | + | var tabBar *component.TabBar | |
| 741 | + | if a.mainWindow != nil { | |
| 742 | + | tabBar = a.mainWindow.TabBar() | |
| 743 | + | } | |
| 744 | + | if created.mainWindow != nil && created.mainWindow.TabBar() != nil { | |
| 745 | + | tabBar = created.mainWindow.TabBar() | |
| 746 | + | } | |
| 747 | + | if tabBar != nil { | |
| 748 | + | tabBar.RemoveTab(output.Tab.ID) | |
| 749 | + | if a.tabs != nil { | |
| 750 | + | tabBar.SetActive(a.tabs.ActiveTabID) | |
| 751 | + | } | |
| 752 | + | } | |
| 753 | + | a.removeBrowserWindow(created.id) | |
| 754 | + | if created.mainWindow != nil { | |
| 755 | + | created.mainWindow.Destroy() | |
| 756 | + | } | |
| 757 | + | return fmt.Errorf("workspace view not created for tab %s", output.Tab.ID) | |
| 758 | + | } | |
| 736 | 759 | if created.mainWindow != nil { | |
| 737 | 760 | created.mainWindow.Show() | |
| 738 | 761 | } | |
| @@ -1356,7 +1379,15 @@ func (a *App) MoveActivePaneToTab(ctx context.Context, targetTabID entity.TabID) | |||
| 1356 | 1379 | } | |
| 1357 | 1380 | ||
| 1358 | 1381 | func (a *App) moveActivePaneToTabFromBrowserWindow(ctx context.Context, bw *browserWindow, targetTabID entity.TabID) error { | |
| 1359 | - | if bw != nil { | |
| 1382 | + | if targetTabID == "" { | |
| 1383 | + | if bw != nil { | |
| 1384 | + | a.activateBrowserWindow(bw) | |
| 1385 | + | } | |
| 1386 | + | return a.MoveActivePaneToTab(ctx, targetTabID) | |
| 1387 | + | } | |
| 1388 | + | if owner := a.browserWindowForTab(targetTabID); owner != nil { | |
| 1389 | + | a.activateBrowserWindow(owner) | |
| 1390 | + | } else if bw != nil { | |
| 1360 | 1391 | a.activateBrowserWindow(bw) | |
| 1361 | 1392 | } | |
| 1362 | 1393 | return a.MoveActivePaneToTab(ctx, targetTabID) | |
| @@ -733,6 +733,29 @@ func (a *App) openFreshWindowWithTabsUC(ctx context.Context, url string, created | |||||
| 733 | a.createWorkspaceView(ctx, output.Tab) | 733 | a.createWorkspaceView(ctx, output.Tab) | ||
| 734 | a.switchWorkspaceView(ctx, output.Tab.ID) | 734 | a.switchWorkspaceView(ctx, output.Tab.ID) | ||
| 735 | } | 735 | } | ||
| 736 | + | if a.workspaceViews[output.Tab.ID] == nil { | |||
| 737 | + | if a.tabs != nil { | |||
| 738 | + | a.tabs.Remove(output.Tab.ID) | |||
| 739 | + | } | |||
| 740 | + | var tabBar *component.TabBar | |||
| 741 | + | if a.mainWindow != nil { | |||
| 742 | + | tabBar = a.mainWindow.TabBar() | |||
| 743 | + | } | |||
| 744 | + | if created.mainWindow != nil && created.mainWindow.TabBar() != nil { | |||
| 745 | + | tabBar = created.mainWindow.TabBar() | |||
| 746 | + | } | |||
| 747 | + | if tabBar != nil { | |||
| 748 | + | tabBar.RemoveTab(output.Tab.ID) | |||
| 749 | + | if a.tabs != nil { | |||
| 750 | + | tabBar.SetActive(a.tabs.ActiveTabID) | |||
| 751 | + | } | |||
| 752 | + | } | |||
| 753 | + | a.removeBrowserWindow(created.id) | |||
| 754 | + | if created.mainWindow != nil { | |||
| 755 | + | created.mainWindow.Destroy() | |||
| 756 | + | } | |||
| 757 | + | return fmt.Errorf("workspace view not created for tab %s", output.Tab.ID) | |||
| 758 | + | } | |||
| 736 | if created.mainWindow != nil { | 759 | if created.mainWindow != nil { | ||
| 737 | created.mainWindow.Show() | 760 | created.mainWindow.Show() | ||
| 738 | } | 761 | } | ||
| @@ -1356,7 +1379,15 @@ func (a *App) MoveActivePaneToTab(ctx context.Context, targetTabID entity.TabID) | |||||
| 1356 | } | 1379 | } | ||
| 1357 | 1380 | ||||
| 1358 | func (a *App) moveActivePaneToTabFromBrowserWindow(ctx context.Context, bw *browserWindow, targetTabID entity.TabID) error { | 1381 | func (a *App) moveActivePaneToTabFromBrowserWindow(ctx context.Context, bw *browserWindow, targetTabID entity.TabID) error { | ||
| 1359 | - | if bw != nil { | 1382 | + | if targetTabID == "" { |
| 1383 | + | if bw != nil { | |||
| 1384 | + | a.activateBrowserWindow(bw) | |||
| 1385 | + | } | |||
| 1386 | + | return a.MoveActivePaneToTab(ctx, targetTabID) | |||
| 1387 | + | } | |||
| 1388 | + | if owner := a.browserWindowForTab(targetTabID); owner != nil { | |||
| 1389 | + | a.activateBrowserWindow(owner) | |||
| 1390 | + | } else if bw != nil { | |||
| 1360 | a.activateBrowserWindow(bw) | 1391 | a.activateBrowserWindow(bw) | ||
| 1361 | } | 1392 | } | ||
| 1362 | return a.MoveActivePaneToTab(ctx, targetTabID) | 1393 | return a.MoveActivePaneToTab(ctx, targetTabID) | ||
M internal/ui/app_browser_launch_test.go +57 -8
| @@ -48,6 +48,8 @@ func (c *testCloser) Close() error { | |||
| 48 | 48 | return nil | |
| 49 | 49 | } | |
| 50 | 50 | ||
| 51 | + | // setDependencyField uses reflection to reach unexported dependency fields for tests. | |
| 52 | + | // Keep this test-only so production visibility stays narrow. | |
| 51 | 53 | func setDependencyField(t *testing.T, deps *Dependencies, field string, value any) { | |
| 52 | 54 | t.Helper() | |
| 53 | 55 | ||
| @@ -59,6 +61,8 @@ func setDependencyField(t *testing.T, deps *Dependencies, field string, value an | |||
| 59 | 61 | fv.Set(reflect.ValueOf(value)) | |
| 60 | 62 | } | |
| 61 | 63 | ||
| 64 | + | // windowForTabCount uses reflection to inspect unexported app state in tests. | |
| 65 | + | // Keep this test-only so production visibility stays narrow. | |
| 62 | 66 | func windowForTabCount(t *testing.T, app *App) int { | |
| 63 | 67 | t.Helper() | |
| 64 | 68 | ||
| @@ -70,6 +74,8 @@ func windowForTabCount(t *testing.T, app *App) int { | |||
| 70 | 74 | return fv.Len() | |
| 71 | 75 | } | |
| 72 | 76 | ||
| 77 | + | // tabCoordinatorMainWindowPtr uses reflection to inspect unexported coordinator state in tests. | |
| 78 | + | // Keep this test-only so production visibility stays narrow. | |
| 73 | 79 | func tabCoordinatorMainWindowPtr(t *testing.T, tc *coordinator.TabCoordinator) uintptr { | |
| 74 | 80 | t.Helper() | |
| 75 | 81 | ||
| @@ -129,15 +135,10 @@ func windowTitle(t *testing.T, mw *window.MainWindow) string { | |||
| 129 | 135 | ||
| 130 | 136 | func windowTabBarVisible(t *testing.T, mw *window.MainWindow) bool { | |
| 131 | 137 | t.Helper() | |
| 132 | - | if mw == nil || mw.TabBar() == nil { | |
| 138 | + | if mw == nil || mw.TabBar() == nil || mw.TabBar().Box() == nil { | |
| 133 | 139 | return false | |
| 134 | 140 | } | |
| 135 | - | rv := reflect.ValueOf(mw.TabBar()).Elem() | |
| 136 | - | fv := rv.FieldByName("visible") | |
| 137 | - | if !fv.IsValid() { | |
| 138 | - | t.Fatalf("TabBar missing visible field") | |
| 139 | - | } | |
| 140 | - | return *(*bool)(unsafe.Pointer(fv.UnsafeAddr())) | |
| 141 | + | return mw.TabBar().Box().GetVisible() | |
| 141 | 142 | } | |
| 142 | 143 | ||
| 143 | 144 | func newTestShellToaster(t *testing.T) (*component.Toaster, *layoutmocks.MockBoxWidget, *layoutmocks.MockLabelWidget) { | |
| @@ -231,6 +232,7 @@ func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) { | |||
| 231 | 232 | tabs: entity.NewTabList(), | |
| 232 | 233 | tabsUC: usecase.NewManageTabsUseCase(func() string { return "id-1" }), | |
| 233 | 234 | browserWindows: make(map[string]*browserWindow), | |
| 235 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{entity.TabID("id-1"): &component.WorkspaceView{}}, | |
| 234 | 236 | browserWindowFactory: func(context.Context, string) (*browserWindow, error) { | |
| 235 | 237 | return created, nil | |
| 236 | 238 | }, | |
| @@ -452,10 +454,12 @@ func TestApp_HandlePaneWindowTitleChangedTargetsOwningWindow(t *testing.T) { | |||
| 452 | 454 | if err != nil { | |
| 453 | 455 | t.Fatalf("first window creation failed: %v", err) | |
| 454 | 456 | } | |
| 457 | + | defer firstMainWindow.Destroy() | |
| 455 | 458 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | |
| 456 | 459 | if err != nil { | |
| 457 | 460 | t.Fatalf("second window creation failed: %v", err) | |
| 458 | 461 | } | |
| 462 | + | defer secondMainWindow.Destroy() | |
| 459 | 463 | ||
| 460 | 464 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | |
| 461 | 465 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | |
| @@ -494,10 +498,12 @@ func TestApp_ActivateBrowserWindowResyncsTitleFromBackgroundPane(t *testing.T) { | |||
| 494 | 498 | if err != nil { | |
| 495 | 499 | t.Fatalf("first window creation failed: %v", err) | |
| 496 | 500 | } | |
| 501 | + | defer firstMainWindow.Destroy() | |
| 497 | 502 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | |
| 498 | 503 | if err != nil { | |
| 499 | 504 | t.Fatalf("second window creation failed: %v", err) | |
| 500 | 505 | } | |
| 506 | + | defer secondMainWindow.Destroy() | |
| 501 | 507 | ||
| 502 | 508 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | |
| 503 | 509 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | |
| @@ -542,10 +548,12 @@ func TestApp_HandlePaneFullscreenChangedTargetsOwningWindow(t *testing.T) { | |||
| 542 | 548 | if err != nil { | |
| 543 | 549 | t.Fatalf("first window creation failed: %v", err) | |
| 544 | 550 | } | |
| 551 | + | defer firstMainWindow.Destroy() | |
| 545 | 552 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | |
| 546 | 553 | if err != nil { | |
| 547 | 554 | t.Fatalf("second window creation failed: %v", err) | |
| 548 | 555 | } | |
| 556 | + | defer secondMainWindow.Destroy() | |
| 549 | 557 | ||
| 550 | 558 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | |
| 551 | 559 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | |
| @@ -592,10 +600,12 @@ func TestApp_HandlePaneFullscreenChangedIgnoresUnresolvedPane(t *testing.T) { | |||
| 592 | 600 | if err != nil { | |
| 593 | 601 | t.Fatalf("first window creation failed: %v", err) | |
| 594 | 602 | } | |
| 603 | + | defer firstMainWindow.Destroy() | |
| 595 | 604 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | |
| 596 | 605 | if err != nil { | |
| 597 | 606 | t.Fatalf("second window creation failed: %v", err) | |
| 598 | 607 | } | |
| 608 | + | defer secondMainWindow.Destroy() | |
| 599 | 609 | firstMainWindow.TabBar().SetVisible(true) | |
| 600 | 610 | secondMainWindow.TabBar().SetVisible(true) | |
| 601 | 611 | first := &browserWindow{id: "window-1", mainWindow: firstMainWindow} | |
| @@ -664,6 +674,41 @@ func TestApp_MoveActivePaneToTabFromBrowserWindowAnchorsOwningWindow(t *testing. | |||
| 664 | 674 | } | |
| 665 | 675 | } | |
| 666 | 676 | ||
| 677 | + | func TestApp_MoveActivePaneToTabFromBrowserWindowActivatesTargetOwnerForCrossWindowTab(t *testing.T) { | |
| 678 | + | tabs := entity.NewTabList() | |
| 679 | + | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | |
| 680 | + | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | |
| 681 | + | tab3 := entity.NewTab(entity.TabID("tab-3"), entity.WorkspaceID("workspace-3"), entity.NewPane(entity.PaneID("pane-3"))) | |
| 682 | + | tabs.Add(tab1) | |
| 683 | + | tabs.Add(tab2) | |
| 684 | + | tabs.Add(tab3) | |
| 685 | + | tabs.SetActive(tab1.ID) | |
| 686 | + | first := &browserWindow{id: "window-1", activeTabID: tab1.ID} | |
| 687 | + | second := &browserWindow{id: "window-2", activeTabID: tab2.ID} | |
| 688 | + | app := &App{ | |
| 689 | + | tabs: tabs, | |
| 690 | + | movePaneToTabUC: usecase.NewMovePaneToTabUseCase(func() string { return "generated" }), | |
| 691 | + | deps: &Dependencies{Config: &config.Config{}}, | |
| 692 | + | browserWindows: map[string]*browserWindow{first.id: first, second.id: second}, | |
| 693 | + | windowForTab: map[entity.TabID]*browserWindow{tab1.ID: first, tab2.ID: second, tab3.ID: second}, | |
| 694 | + | lastFocusedWindowID: first.id, | |
| 695 | + | } | |
| 696 | + | ||
| 697 | + | if err := app.moveActivePaneToTabFromBrowserWindow(context.Background(), first, tab3.ID); err != nil { | |
| 698 | + | t.Fatalf("moveActivePaneToTabFromBrowserWindow returned error: %v", err) | |
| 699 | + | } | |
| 700 | + | ||
| 701 | + | if tabs.Find(tab1.ID) == nil { | |
| 702 | + | t.Fatalf("tab-1 should remain when target owner is activated first") | |
| 703 | + | } | |
| 704 | + | if tabs.Find(tab2.ID) != nil { | |
| 705 | + | t.Fatalf("tab-2 should have been moved from the target owner window") | |
| 706 | + | } | |
| 707 | + | if tabs.Find(tab3.ID) == nil { | |
| 708 | + | t.Fatalf("tab-3 should remain as the move target") | |
| 709 | + | } | |
| 710 | + | } | |
| 711 | + | ||
| 667 | 712 | func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) { | |
| 668 | 713 | EnsureAdwaitaInitialized() | |
| 669 | 714 | appID := AppID | |
| @@ -677,6 +722,7 @@ func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) | |||
| 677 | 722 | if err != nil { | |
| 678 | 723 | t.Fatalf("window creation failed: %v", err) | |
| 679 | 724 | } | |
| 725 | + | defer mainWindow.Destroy() | |
| 680 | 726 | ||
| 681 | 727 | sentinelKeyboardHandler := &input.KeyboardHandler{} | |
| 682 | 728 | sentinelShortcutHandler := &input.GlobalShortcutHandler{} | |
| @@ -738,7 +784,10 @@ func TestApp_UpdateBrowserWindowTabBarVisibilityHonorsHideWhenSingleTabDisabled( | |||
| 738 | 784 | defer gtkApp.Unref() | |
| 739 | 785 | ||
| 740 | 786 | mainWindow := &window.MainWindow{} | |
| 741 | - | tabBar := newTestTabBarShell(t, entity.TabID("tab-1")) | |
| 787 | + | tabBar := component.NewTabBar() | |
| 788 | + | if tabBar == nil { | |
| 789 | + | t.Fatal("tab bar creation failed") | |
| 790 | + | } | |
| 742 | 791 | tabBar.SetVisible(false) | |
| 743 | 792 | setWindowTabBar(t, mainWindow, tabBar) | |
| 744 | 793 | bw := &browserWindow{id: "window-1", mainWindow: mainWindow} | |
| @@ -48,6 +48,8 @@ func (c *testCloser) Close() error { | |||||
| 48 | return nil | 48 | return nil | ||
| 49 | } | 49 | } | ||
| 50 | 50 | ||||
| 51 | + | // setDependencyField uses reflection to reach unexported dependency fields for tests. | |||
| 52 | + | // Keep this test-only so production visibility stays narrow. | |||
| 51 | func setDependencyField(t *testing.T, deps *Dependencies, field string, value any) { | 53 | func setDependencyField(t *testing.T, deps *Dependencies, field string, value any) { | ||
| 52 | t.Helper() | 54 | t.Helper() | ||
| 53 | 55 | ||||
| @@ -59,6 +61,8 @@ func setDependencyField(t *testing.T, deps *Dependencies, field string, value an | |||||
| 59 | fv.Set(reflect.ValueOf(value)) | 61 | fv.Set(reflect.ValueOf(value)) | ||
| 60 | } | 62 | } | ||
| 61 | 63 | ||||
| 64 | + | // windowForTabCount uses reflection to inspect unexported app state in tests. | |||
| 65 | + | // Keep this test-only so production visibility stays narrow. | |||
| 62 | func windowForTabCount(t *testing.T, app *App) int { | 66 | func windowForTabCount(t *testing.T, app *App) int { | ||
| 63 | t.Helper() | 67 | t.Helper() | ||
| 64 | 68 | ||||
| @@ -70,6 +74,8 @@ func windowForTabCount(t *testing.T, app *App) int { | |||||
| 70 | return fv.Len() | 74 | return fv.Len() | ||
| 71 | } | 75 | } | ||
| 72 | 76 | ||||
| 77 | + | // tabCoordinatorMainWindowPtr uses reflection to inspect unexported coordinator state in tests. | |||
| 78 | + | // Keep this test-only so production visibility stays narrow. | |||
| 73 | func tabCoordinatorMainWindowPtr(t *testing.T, tc *coordinator.TabCoordinator) uintptr { | 79 | func tabCoordinatorMainWindowPtr(t *testing.T, tc *coordinator.TabCoordinator) uintptr { | ||
| 74 | t.Helper() | 80 | t.Helper() | ||
| 75 | 81 | ||||
| @@ -129,15 +135,10 @@ func windowTitle(t *testing.T, mw *window.MainWindow) string { | |||||
| 129 | 135 | ||||
| 130 | func windowTabBarVisible(t *testing.T, mw *window.MainWindow) bool { | 136 | func windowTabBarVisible(t *testing.T, mw *window.MainWindow) bool { | ||
| 131 | t.Helper() | 137 | t.Helper() | ||
| 132 | - | if mw == nil || mw.TabBar() == nil { | 138 | + | if mw == nil || mw.TabBar() == nil || mw.TabBar().Box() == nil { |
| 133 | return false | 139 | return false | ||
| 134 | } | 140 | } | ||
| 135 | - | rv := reflect.ValueOf(mw.TabBar()).Elem() | 141 | + | return mw.TabBar().Box().GetVisible() |
| 136 | - | fv := rv.FieldByName("visible") | |||
| 137 | - | if !fv.IsValid() { | |||
| 138 | - | t.Fatalf("TabBar missing visible field") | |||
| 139 | - | } | |||
| 140 | - | return *(*bool)(unsafe.Pointer(fv.UnsafeAddr())) | |||
| 141 | } | 142 | } | ||
| 142 | 143 | ||||
| 143 | func newTestShellToaster(t *testing.T) (*component.Toaster, *layoutmocks.MockBoxWidget, *layoutmocks.MockLabelWidget) { | 144 | func newTestShellToaster(t *testing.T) (*component.Toaster, *layoutmocks.MockBoxWidget, *layoutmocks.MockLabelWidget) { | ||
| @@ -231,6 +232,7 @@ func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) { | |||||
| 231 | tabs: entity.NewTabList(), | 232 | tabs: entity.NewTabList(), | ||
| 232 | tabsUC: usecase.NewManageTabsUseCase(func() string { return "id-1" }), | 233 | tabsUC: usecase.NewManageTabsUseCase(func() string { return "id-1" }), | ||
| 233 | browserWindows: make(map[string]*browserWindow), | 234 | browserWindows: make(map[string]*browserWindow), | ||
| 235 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{entity.TabID("id-1"): &component.WorkspaceView{}}, | |||
| 234 | browserWindowFactory: func(context.Context, string) (*browserWindow, error) { | 236 | browserWindowFactory: func(context.Context, string) (*browserWindow, error) { | ||
| 235 | return created, nil | 237 | return created, nil | ||
| 236 | }, | 238 | }, | ||
| @@ -452,10 +454,12 @@ func TestApp_HandlePaneWindowTitleChangedTargetsOwningWindow(t *testing.T) { | |||||
| 452 | if err != nil { | 454 | if err != nil { | ||
| 453 | t.Fatalf("first window creation failed: %v", err) | 455 | t.Fatalf("first window creation failed: %v", err) | ||
| 454 | } | 456 | } | ||
| 457 | + | defer firstMainWindow.Destroy() | |||
| 455 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | 458 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | ||
| 456 | if err != nil { | 459 | if err != nil { | ||
| 457 | t.Fatalf("second window creation failed: %v", err) | 460 | t.Fatalf("second window creation failed: %v", err) | ||
| 458 | } | 461 | } | ||
| 462 | + | defer secondMainWindow.Destroy() | |||
| 459 | 463 | ||||
| 460 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | 464 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | ||
| 461 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | 465 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | ||
| @@ -494,10 +498,12 @@ func TestApp_ActivateBrowserWindowResyncsTitleFromBackgroundPane(t *testing.T) { | |||||
| 494 | if err != nil { | 498 | if err != nil { | ||
| 495 | t.Fatalf("first window creation failed: %v", err) | 499 | t.Fatalf("first window creation failed: %v", err) | ||
| 496 | } | 500 | } | ||
| 501 | + | defer firstMainWindow.Destroy() | |||
| 497 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | 502 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | ||
| 498 | if err != nil { | 503 | if err != nil { | ||
| 499 | t.Fatalf("second window creation failed: %v", err) | 504 | t.Fatalf("second window creation failed: %v", err) | ||
| 500 | } | 505 | } | ||
| 506 | + | defer secondMainWindow.Destroy() | |||
| 501 | 507 | ||||
| 502 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | 508 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | ||
| 503 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | 509 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | ||
| @@ -542,10 +548,12 @@ func TestApp_HandlePaneFullscreenChangedTargetsOwningWindow(t *testing.T) { | |||||
| 542 | if err != nil { | 548 | if err != nil { | ||
| 543 | t.Fatalf("first window creation failed: %v", err) | 549 | t.Fatalf("first window creation failed: %v", err) | ||
| 544 | } | 550 | } | ||
| 551 | + | defer firstMainWindow.Destroy() | |||
| 545 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | 552 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | ||
| 546 | if err != nil { | 553 | if err != nil { | ||
| 547 | t.Fatalf("second window creation failed: %v", err) | 554 | t.Fatalf("second window creation failed: %v", err) | ||
| 548 | } | 555 | } | ||
| 556 | + | defer secondMainWindow.Destroy() | |||
| 549 | 557 | ||||
| 550 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | 558 | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | ||
| 551 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | 559 | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | ||
| @@ -592,10 +600,12 @@ func TestApp_HandlePaneFullscreenChangedIgnoresUnresolvedPane(t *testing.T) { | |||||
| 592 | if err != nil { | 600 | if err != nil { | ||
| 593 | t.Fatalf("first window creation failed: %v", err) | 601 | t.Fatalf("first window creation failed: %v", err) | ||
| 594 | } | 602 | } | ||
| 603 | + | defer firstMainWindow.Destroy() | |||
| 595 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | 604 | secondMainWindow, err := window.New(context.Background(), gtkApp, "top") | ||
| 596 | if err != nil { | 605 | if err != nil { | ||
| 597 | t.Fatalf("second window creation failed: %v", err) | 606 | t.Fatalf("second window creation failed: %v", err) | ||
| 598 | } | 607 | } | ||
| 608 | + | defer secondMainWindow.Destroy() | |||
| 599 | firstMainWindow.TabBar().SetVisible(true) | 609 | firstMainWindow.TabBar().SetVisible(true) | ||
| 600 | secondMainWindow.TabBar().SetVisible(true) | 610 | secondMainWindow.TabBar().SetVisible(true) | ||
| 601 | first := &browserWindow{id: "window-1", mainWindow: firstMainWindow} | 611 | first := &browserWindow{id: "window-1", mainWindow: firstMainWindow} | ||
| @@ -664,6 +674,41 @@ func TestApp_MoveActivePaneToTabFromBrowserWindowAnchorsOwningWindow(t *testing. | |||||
| 664 | } | 674 | } | ||
| 665 | } | 675 | } | ||
| 666 | 676 | ||||
| 677 | + | func TestApp_MoveActivePaneToTabFromBrowserWindowActivatesTargetOwnerForCrossWindowTab(t *testing.T) { | |||
| 678 | + | tabs := entity.NewTabList() | |||
| 679 | + | tab1 := entity.NewTab(entity.TabID("tab-1"), entity.WorkspaceID("workspace-1"), entity.NewPane(entity.PaneID("pane-1"))) | |||
| 680 | + | tab2 := entity.NewTab(entity.TabID("tab-2"), entity.WorkspaceID("workspace-2"), entity.NewPane(entity.PaneID("pane-2"))) | |||
| 681 | + | tab3 := entity.NewTab(entity.TabID("tab-3"), entity.WorkspaceID("workspace-3"), entity.NewPane(entity.PaneID("pane-3"))) | |||
| 682 | + | tabs.Add(tab1) | |||
| 683 | + | tabs.Add(tab2) | |||
| 684 | + | tabs.Add(tab3) | |||
| 685 | + | tabs.SetActive(tab1.ID) | |||
| 686 | + | first := &browserWindow{id: "window-1", activeTabID: tab1.ID} | |||
| 687 | + | second := &browserWindow{id: "window-2", activeTabID: tab2.ID} | |||
| 688 | + | app := &App{ | |||
| 689 | + | tabs: tabs, | |||
| 690 | + | movePaneToTabUC: usecase.NewMovePaneToTabUseCase(func() string { return "generated" }), | |||
| 691 | + | deps: &Dependencies{Config: &config.Config{}}, | |||
| 692 | + | browserWindows: map[string]*browserWindow{first.id: first, second.id: second}, | |||
| 693 | + | windowForTab: map[entity.TabID]*browserWindow{tab1.ID: first, tab2.ID: second, tab3.ID: second}, | |||
| 694 | + | lastFocusedWindowID: first.id, | |||
| 695 | + | } | |||
| 696 | + | ||||
| 697 | + | if err := app.moveActivePaneToTabFromBrowserWindow(context.Background(), first, tab3.ID); err != nil { | |||
| 698 | + | t.Fatalf("moveActivePaneToTabFromBrowserWindow returned error: %v", err) | |||
| 699 | + | } | |||
| 700 | + | ||||
| 701 | + | if tabs.Find(tab1.ID) == nil { | |||
| 702 | + | t.Fatalf("tab-1 should remain when target owner is activated first") | |||
| 703 | + | } | |||
| 704 | + | if tabs.Find(tab2.ID) != nil { | |||
| 705 | + | t.Fatalf("tab-2 should have been moved from the target owner window") | |||
| 706 | + | } | |||
| 707 | + | if tabs.Find(tab3.ID) == nil { | |||
| 708 | + | t.Fatalf("tab-3 should remain as the move target") | |||
| 709 | + | } | |||
| 710 | + | } | |||
| 711 | + | ||||
| 667 | func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) { | 712 | func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) { | ||
| 668 | EnsureAdwaitaInitialized() | 713 | EnsureAdwaitaInitialized() | ||
| 669 | appID := AppID | 714 | appID := AppID | ||
| @@ -677,6 +722,7 @@ func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) | |||||
| 677 | if err != nil { | 722 | if err != nil { | ||
| 678 | t.Fatalf("window creation failed: %v", err) | 723 | t.Fatalf("window creation failed: %v", err) | ||
| 679 | } | 724 | } | ||
| 725 | + | defer mainWindow.Destroy() | |||
| 680 | 726 | ||||
| 681 | sentinelKeyboardHandler := &input.KeyboardHandler{} | 727 | sentinelKeyboardHandler := &input.KeyboardHandler{} | ||
| 682 | sentinelShortcutHandler := &input.GlobalShortcutHandler{} | 728 | sentinelShortcutHandler := &input.GlobalShortcutHandler{} | ||
| @@ -738,7 +784,10 @@ func TestApp_UpdateBrowserWindowTabBarVisibilityHonorsHideWhenSingleTabDisabled( | |||||
| 738 | defer gtkApp.Unref() | 784 | defer gtkApp.Unref() | ||
| 739 | 785 | ||||
| 740 | mainWindow := &window.MainWindow{} | 786 | mainWindow := &window.MainWindow{} | ||
| 741 | - | tabBar := newTestTabBarShell(t, entity.TabID("tab-1")) | 787 | + | tabBar := component.NewTabBar() |
| 788 | + | if tabBar == nil { | |||
| 789 | + | t.Fatal("tab bar creation failed") | |||
| 790 | + | } | |||
| 742 | tabBar.SetVisible(false) | 791 | tabBar.SetVisible(false) | ||
| 743 | setWindowTabBar(t, mainWindow, tabBar) | 792 | setWindowTabBar(t, mainWindow, tabBar) | ||
| 744 | bw := &browserWindow{id: "window-1", mainWindow: mainWindow} | 793 | bw := &browserWindow{id: "window-1", mainWindow: mainWindow} | ||
M internal/ui/browser_window.go +10 -1
| @@ -246,9 +246,18 @@ func (a *App) removeBrowserWindow(id string) { | |||
| 246 | 246 | } | |
| 247 | 247 | removed := a.browserWindows[id] | |
| 248 | 248 | wasMainWindow := removed != nil && removed.mainWindow == a.mainWindow | |
| 249 | + | ownedTabIDs := make([]entity.TabID, 0) | |
| 249 | 250 | for tabID, bw := range a.windowForTab { | |
| 250 | 251 | if bw != nil && bw.id == id { | |
| 251 | - | delete(a.windowForTab, tabID) | |
| 252 | + | ownedTabIDs = append(ownedTabIDs, tabID) | |
| 253 | + | } | |
| 254 | + | } | |
| 255 | + | for _, tabID := range ownedTabIDs { | |
| 256 | + | a.releaseFloatingSessionsForTab(context.Background(), tabID) | |
| 257 | + | delete(a.workspaceViews, tabID) | |
| 258 | + | delete(a.windowForTab, tabID) | |
| 259 | + | if a.tabs != nil && a.tabs.Find(tabID) != nil { | |
| 260 | + | a.tabs.Remove(tabID) | |
| 252 | 261 | } | |
| 253 | 262 | } | |
| 254 | 263 | if removed != nil { | |
| @@ -246,9 +246,18 @@ func (a *App) removeBrowserWindow(id string) { | |||||
| 246 | } | 246 | } | ||
| 247 | removed := a.browserWindows[id] | 247 | removed := a.browserWindows[id] | ||
| 248 | wasMainWindow := removed != nil && removed.mainWindow == a.mainWindow | 248 | wasMainWindow := removed != nil && removed.mainWindow == a.mainWindow | ||
| 249 | + | ownedTabIDs := make([]entity.TabID, 0) | |||
| 249 | for tabID, bw := range a.windowForTab { | 250 | for tabID, bw := range a.windowForTab { | ||
| 250 | if bw != nil && bw.id == id { | 251 | if bw != nil && bw.id == id { | ||
| 251 | - | delete(a.windowForTab, tabID) | 252 | + | ownedTabIDs = append(ownedTabIDs, tabID) |
| 253 | + | } | |||
| 254 | + | } | |||
| 255 | + | for _, tabID := range ownedTabIDs { | |||
| 256 | + | a.releaseFloatingSessionsForTab(context.Background(), tabID) | |||
| 257 | + | delete(a.workspaceViews, tabID) | |||
| 258 | + | delete(a.windowForTab, tabID) | |||
| 259 | + | if a.tabs != nil && a.tabs.Find(tabID) != nil { | |||
| 260 | + | a.tabs.Remove(tabID) | |||
| 252 | } | 261 | } | ||
| 253 | } | 262 | } | ||
| 254 | if removed != nil { | 263 | if removed != nil { | ||
M internal/ui/browser_window_test.go +51 -0
| @@ -61,6 +61,57 @@ func TestBrowserWindow_RemoveBrowserWindowClearsShellState(t *testing.T) { | |||
| 61 | 61 | } | |
| 62 | 62 | } | |
| 63 | 63 | ||
| 64 | + | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | |
| 65 | + | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | |
| 66 | + | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | |
| 67 | + | removedMainWindow := &window.MainWindow{} | |
| 68 | + | remainingMainWindow := &window.MainWindow{} | |
| 69 | + | removed := &browserWindow{id: "window-1", mainWindow: removedMainWindow} | |
| 70 | + | remaining := &browserWindow{id: "window-2", mainWindow: remainingMainWindow} | |
| 71 | + | ownedSessionKey := floatingSessionKey{tabID: ownedTab.ID, sessionID: "profile:owned"} | |
| 72 | + | otherSessionKey := floatingSessionKey{tabID: otherTab.ID, sessionID: "profile:other"} | |
| 73 | + | app := &App{ | |
| 74 | + | mainWindow: removedMainWindow, | |
| 75 | + | browserWindows: map[string]*browserWindow{removed.id: removed, remaining.id: remaining}, | |
| 76 | + | tabs: entity.NewTabList(), | |
| 77 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ownedTab.ID: &component.WorkspaceView{}, otherTab.ID: &component.WorkspaceView{}}, | |
| 78 | + | windowForTab: map[entity.TabID]*browserWindow{ownedTab.ID: removed, otherTab.ID: remaining}, | |
| 79 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{ | |
| 80 | + | ownedSessionKey: {}, | |
| 81 | + | otherSessionKey: {}, | |
| 82 | + | }, | |
| 83 | + | } | |
| 84 | + | app.tabs.Add(ownedTab) | |
| 85 | + | app.tabs.Add(otherTab) | |
| 86 | + | ||
| 87 | + | app.removeBrowserWindow(removed.id) | |
| 88 | + | ||
| 89 | + | if app.workspaceViews[ownedTab.ID] != nil { | |
| 90 | + | t.Fatalf("owned workspace view was not removed") | |
| 91 | + | } | |
| 92 | + | if app.windowForTab[ownedTab.ID] != nil { | |
| 93 | + | t.Fatalf("owned window mapping was not removed") | |
| 94 | + | } | |
| 95 | + | if app.tabs.Find(ownedTab.ID) != nil { | |
| 96 | + | t.Fatalf("owned tab was not removed") | |
| 97 | + | } | |
| 98 | + | if app.workspaceViews[otherTab.ID] == nil { | |
| 99 | + | t.Fatalf("other workspace view should remain") | |
| 100 | + | } | |
| 101 | + | if app.windowForTab[otherTab.ID] != remaining { | |
| 102 | + | t.Fatalf("other tab should remain mapped to remaining window") | |
| 103 | + | } | |
| 104 | + | if app.tabs.Find(otherTab.ID) == nil { | |
| 105 | + | t.Fatalf("other tab should remain") | |
| 106 | + | } | |
| 107 | + | if _, ok := app.floatingSessions[ownedSessionKey]; ok { | |
| 108 | + | t.Fatalf("owned floating session was not released") | |
| 109 | + | } | |
| 110 | + | if _, ok := app.floatingSessions[otherSessionKey]; !ok { | |
| 111 | + | t.Fatalf("other floating session should remain") | |
| 112 | + | } | |
| 113 | + | } | |
| 114 | + | ||
| 64 | 115 | func TestBrowserWindow_RegisterAndRemoveTrackCollection(t *testing.T) { | |
| 65 | 116 | app := &App{browserWindows: make(map[string]*browserWindow)} | |
| 66 | 117 | first := &browserWindow{id: "window-1"} | |
| @@ -61,6 +61,57 @@ func TestBrowserWindow_RemoveBrowserWindowClearsShellState(t *testing.T) { | |||||
| 61 | } | 61 | } | ||
| 62 | } | 62 | } | ||
| 63 | 63 | ||||
| 64 | + | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | |||
| 65 | + | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | |||
| 66 | + | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | |||
| 67 | + | removedMainWindow := &window.MainWindow{} | |||
| 68 | + | remainingMainWindow := &window.MainWindow{} | |||
| 69 | + | removed := &browserWindow{id: "window-1", mainWindow: removedMainWindow} | |||
| 70 | + | remaining := &browserWindow{id: "window-2", mainWindow: remainingMainWindow} | |||
| 71 | + | ownedSessionKey := floatingSessionKey{tabID: ownedTab.ID, sessionID: "profile:owned"} | |||
| 72 | + | otherSessionKey := floatingSessionKey{tabID: otherTab.ID, sessionID: "profile:other"} | |||
| 73 | + | app := &App{ | |||
| 74 | + | mainWindow: removedMainWindow, | |||
| 75 | + | browserWindows: map[string]*browserWindow{removed.id: removed, remaining.id: remaining}, | |||
| 76 | + | tabs: entity.NewTabList(), | |||
| 77 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ownedTab.ID: &component.WorkspaceView{}, otherTab.ID: &component.WorkspaceView{}}, | |||
| 78 | + | windowForTab: map[entity.TabID]*browserWindow{ownedTab.ID: removed, otherTab.ID: remaining}, | |||
| 79 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{ | |||
| 80 | + | ownedSessionKey: {}, | |||
| 81 | + | otherSessionKey: {}, | |||
| 82 | + | }, | |||
| 83 | + | } | |||
| 84 | + | app.tabs.Add(ownedTab) | |||
| 85 | + | app.tabs.Add(otherTab) | |||
| 86 | + | ||||
| 87 | + | app.removeBrowserWindow(removed.id) | |||
| 88 | + | ||||
| 89 | + | if app.workspaceViews[ownedTab.ID] != nil { | |||
| 90 | + | t.Fatalf("owned workspace view was not removed") | |||
| 91 | + | } | |||
| 92 | + | if app.windowForTab[ownedTab.ID] != nil { | |||
| 93 | + | t.Fatalf("owned window mapping was not removed") | |||
| 94 | + | } | |||
| 95 | + | if app.tabs.Find(ownedTab.ID) != nil { | |||
| 96 | + | t.Fatalf("owned tab was not removed") | |||
| 97 | + | } | |||
| 98 | + | if app.workspaceViews[otherTab.ID] == nil { | |||
| 99 | + | t.Fatalf("other workspace view should remain") | |||
| 100 | + | } | |||
| 101 | + | if app.windowForTab[otherTab.ID] != remaining { | |||
| 102 | + | t.Fatalf("other tab should remain mapped to remaining window") | |||
| 103 | + | } | |||
| 104 | + | if app.tabs.Find(otherTab.ID) == nil { | |||
| 105 | + | t.Fatalf("other tab should remain") | |||
| 106 | + | } | |||
| 107 | + | if _, ok := app.floatingSessions[ownedSessionKey]; ok { | |||
| 108 | + | t.Fatalf("owned floating session was not released") | |||
| 109 | + | } | |||
| 110 | + | if _, ok := app.floatingSessions[otherSessionKey]; !ok { | |||
| 111 | + | t.Fatalf("other floating session should remain") | |||
| 112 | + | } | |||
| 113 | + | } | |||
| 114 | + | ||||
| 64 | func TestBrowserWindow_RegisterAndRemoveTrackCollection(t *testing.T) { | 115 | func TestBrowserWindow_RegisterAndRemoveTrackCollection(t *testing.T) { | ||
| 65 | app := &App{browserWindows: make(map[string]*browserWindow)} | 116 | app := &App{browserWindows: make(map[string]*browserWindow)} | ||
| 66 | first := &browserWindow{id: "window-1"} | 117 | first := &browserWindow{id: "window-1"} | ||
M internal/ui/component/tab_bar.go +0 -3
| @@ -11,7 +11,6 @@ import ( | |||
| 11 | 11 | type TabBar struct { | |
| 12 | 12 | box *gtk.Box | |
| 13 | 13 | buttons map[entity.TabID]*TabButton | |
| 14 | - | visible bool | |
| 15 | 14 | ||
| 16 | 15 | // Active tab tracking | |
| 17 | 16 | activeTabID entity.TabID | |
| @@ -28,7 +27,6 @@ type TabBar struct { | |||
| 28 | 27 | func NewTabBar() *TabBar { | |
| 29 | 28 | tb := &TabBar{ | |
| 30 | 29 | buttons: make(map[entity.TabID]*TabButton), | |
| 31 | - | visible: true, | |
| 32 | 30 | } | |
| 33 | 31 | ||
| 34 | 32 | // Create horizontal box container | |
| @@ -161,7 +159,6 @@ func (tb *TabBar) ActiveTabID() entity.TabID { | |||
| 161 | 159 | ||
| 162 | 160 | // SetVisible shows or hides the tab bar. | |
| 163 | 161 | func (tb *TabBar) SetVisible(visible bool) { | |
| 164 | - | tb.visible = visible | |
| 165 | 162 | if tb.box == nil { | |
| 166 | 163 | return | |
| 167 | 164 | } | |
| @@ -11,7 +11,6 @@ import ( | |||||
| 11 | type TabBar struct { | 11 | type TabBar struct { | ||
| 12 | box *gtk.Box | 12 | box *gtk.Box | ||
| 13 | buttons map[entity.TabID]*TabButton | 13 | buttons map[entity.TabID]*TabButton | ||
| 14 | - | visible bool | |||
| 15 | 14 | ||||
| 16 | // Active tab tracking | 15 | // Active tab tracking | ||
| 17 | activeTabID entity.TabID | 16 | activeTabID entity.TabID | ||
| @@ -28,7 +27,6 @@ type TabBar struct { | |||||
| 28 | func NewTabBar() *TabBar { | 27 | func NewTabBar() *TabBar { | ||
| 29 | tb := &TabBar{ | 28 | tb := &TabBar{ | ||
| 30 | buttons: make(map[entity.TabID]*TabButton), | 29 | buttons: make(map[entity.TabID]*TabButton), | ||
| 31 | - | visible: true, | |||
| 32 | } | 30 | } | ||
| 33 | 31 | ||||
| 34 | // Create horizontal box container | 32 | // Create horizontal box container | ||
| @@ -161,7 +159,6 @@ func (tb *TabBar) ActiveTabID() entity.TabID { | |||||
| 161 | 159 | ||||
| 162 | // SetVisible shows or hides the tab bar. | 160 | // SetVisible shows or hides the tab bar. | ||
| 163 | func (tb *TabBar) SetVisible(visible bool) { | 161 | func (tb *TabBar) SetVisible(visible bool) { | ||
| 164 | - | tb.visible = visible | |||
| 165 | if tb.box == nil { | 162 | if tb.box == nil { | ||
| 166 | return | 163 | return | ||
| 167 | } | 164 | } | ||
M internal/ui/coordinator/content/title.go +5 -3
| @@ -59,9 +59,11 @@ func (c *Coordinator) onTitleChanged(ctx context.Context, paneID entity.PaneID, | |||
| 59 | 59 | } | |
| 60 | 60 | } | |
| 61 | 61 | ||
| 62 | - | // Notify window title update for every pane title change. | |
| 63 | - | if c.onWindowTitleChanged != nil { | |
| 64 | - | c.onWindowTitleChanged(paneID, title) | |
| 62 | + | // Notify window title updates only for the active pane. | |
| 63 | + | if c.onWindowTitleChanged != nil && c.getActiveWS != nil { | |
| 64 | + | if ws, _ := c.getActiveWS(); ws != nil && ws.ActivePaneID == paneID { | |
| 65 | + | c.onWindowTitleChanged(paneID, title) | |
| 66 | + | } | |
| 65 | 67 | } | |
| 66 | 68 | ||
| 67 | 69 | log.Debug(). | |
| @@ -59,9 +59,11 @@ func (c *Coordinator) onTitleChanged(ctx context.Context, paneID entity.PaneID, | |||||
| 59 | } | 59 | } | ||
| 60 | } | 60 | } | ||
| 61 | 61 | ||||
| 62 | - | // Notify window title update for every pane title change. | 62 | + | // Notify window title updates only for the active pane. |
| 63 | - | if c.onWindowTitleChanged != nil { | 63 | + | if c.onWindowTitleChanged != nil && c.getActiveWS != nil { |
| 64 | - | c.onWindowTitleChanged(paneID, title) | 64 | + | if ws, _ := c.getActiveWS(); ws != nil && ws.ActivePaneID == paneID { |
| 65 | + | c.onWindowTitleChanged(paneID, title) | |||
| 66 | + | } | |||
| 65 | } | 67 | } | ||
| 66 | 68 | ||||
| 67 | log.Debug(). | 69 | log.Debug(). | ||
M internal/ui/coordinator/content/title_test.go +30 -3
| @@ -11,7 +11,7 @@ import ( | |||
| 11 | 11 | "github.com/bnema/dumber/internal/ui/component" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | - | func TestOnTitleChanged_EmitsCallbackForBackgroundPane(t *testing.T) { | |
| 14 | + | func TestOnTitleChanged_DoesNotEmitWindowCallbackForBackgroundPane(t *testing.T) { | |
| 15 | 15 | t.Parallel() | |
| 16 | 16 | ||
| 17 | 17 | pane1 := entity.PaneID("pane-1") | |
| @@ -35,11 +35,38 @@ func TestOnTitleChanged_EmitsCallbackForBackgroundPane(t *testing.T) { | |||
| 35 | 35 | ||
| 36 | 36 | c.onTitleChanged(context.Background(), pane2, "Pane Two") | |
| 37 | 37 | ||
| 38 | - | assert.Equal(t, pane2, gotPaneID) | |
| 39 | - | assert.Equal(t, "Pane Two", gotTitle) | |
| 38 | + | assert.Empty(t, gotPaneID) | |
| 39 | + | assert.Empty(t, gotTitle) | |
| 40 | 40 | assert.Equal(t, "Pane Two", c.GetTitle(pane2)) | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | + | func TestOnTitleChanged_EmitsWindowCallbackForActivePane(t *testing.T) { | |
| 44 | + | t.Parallel() | |
| 45 | + | ||
| 46 | + | pane := entity.PaneID("pane-1") | |
| 47 | + | root := &entity.PaneNode{ID: "root"} | |
| 48 | + | leaf := &entity.PaneNode{ID: string(pane), Pane: entity.NewPane(pane), Parent: root} | |
| 49 | + | root.Children = []*entity.PaneNode{leaf} | |
| 50 | + | ||
| 51 | + | ws := &entity.Workspace{Root: root, ActivePaneID: pane} | |
| 52 | + | var gotPaneID entity.PaneID | |
| 53 | + | var gotTitle string | |
| 54 | + | c := &Coordinator{ | |
| 55 | + | paneTitles: make(map[entity.PaneID]string), | |
| 56 | + | getActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { return ws, nil }, | |
| 57 | + | onWindowTitleChanged: func(paneID entity.PaneID, title string) { | |
| 58 | + | gotPaneID = paneID | |
| 59 | + | gotTitle = title | |
| 60 | + | }, | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | c.onTitleChanged(context.Background(), pane, "Pane One") | |
| 64 | + | ||
| 65 | + | assert.Equal(t, pane, gotPaneID) | |
| 66 | + | assert.Equal(t, "Pane One", gotTitle) | |
| 67 | + | assert.Equal(t, "Pane One", c.GetTitle(pane)) | |
| 68 | + | } | |
| 69 | + | ||
| 43 | 70 | // TestGetTitle tests the GetTitle method for title state retrieval. | |
| 44 | 71 | ||
| 45 | 72 | func TestGetTitle_ReturnsEmptyForUnknownPane(t *testing.T) { | |
| @@ -11,7 +11,7 @@ import ( | |||||
| 11 | "github.com/bnema/dumber/internal/ui/component" | 11 | "github.com/bnema/dumber/internal/ui/component" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | - | func TestOnTitleChanged_EmitsCallbackForBackgroundPane(t *testing.T) { | 14 | + | func TestOnTitleChanged_DoesNotEmitWindowCallbackForBackgroundPane(t *testing.T) { |
| 15 | t.Parallel() | 15 | t.Parallel() | ||
| 16 | 16 | ||||
| 17 | pane1 := entity.PaneID("pane-1") | 17 | pane1 := entity.PaneID("pane-1") | ||
| @@ -35,11 +35,38 @@ func TestOnTitleChanged_EmitsCallbackForBackgroundPane(t *testing.T) { | |||||
| 35 | 35 | ||||
| 36 | c.onTitleChanged(context.Background(), pane2, "Pane Two") | 36 | c.onTitleChanged(context.Background(), pane2, "Pane Two") | ||
| 37 | 37 | ||||
| 38 | - | assert.Equal(t, pane2, gotPaneID) | 38 | + | assert.Empty(t, gotPaneID) |
| 39 | - | assert.Equal(t, "Pane Two", gotTitle) | 39 | + | assert.Empty(t, gotTitle) |
| 40 | assert.Equal(t, "Pane Two", c.GetTitle(pane2)) | 40 | assert.Equal(t, "Pane Two", c.GetTitle(pane2)) | ||
| 41 | } | 41 | } | ||
| 42 | 42 | ||||
| 43 | + | func TestOnTitleChanged_EmitsWindowCallbackForActivePane(t *testing.T) { | |||
| 44 | + | t.Parallel() | |||
| 45 | + | ||||
| 46 | + | pane := entity.PaneID("pane-1") | |||
| 47 | + | root := &entity.PaneNode{ID: "root"} | |||
| 48 | + | leaf := &entity.PaneNode{ID: string(pane), Pane: entity.NewPane(pane), Parent: root} | |||
| 49 | + | root.Children = []*entity.PaneNode{leaf} | |||
| 50 | + | ||||
| 51 | + | ws := &entity.Workspace{Root: root, ActivePaneID: pane} | |||
| 52 | + | var gotPaneID entity.PaneID | |||
| 53 | + | var gotTitle string | |||
| 54 | + | c := &Coordinator{ | |||
| 55 | + | paneTitles: make(map[entity.PaneID]string), | |||
| 56 | + | getActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { return ws, nil }, | |||
| 57 | + | onWindowTitleChanged: func(paneID entity.PaneID, title string) { | |||
| 58 | + | gotPaneID = paneID | |||
| 59 | + | gotTitle = title | |||
| 60 | + | }, | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | c.onTitleChanged(context.Background(), pane, "Pane One") | |||
| 64 | + | ||||
| 65 | + | assert.Equal(t, pane, gotPaneID) | |||
| 66 | + | assert.Equal(t, "Pane One", gotTitle) | |||
| 67 | + | assert.Equal(t, "Pane One", c.GetTitle(pane)) | |||
| 68 | + | } | |||
| 69 | + | ||||
| 43 | // TestGetTitle tests the GetTitle method for title state retrieval. | 70 | // TestGetTitle tests the GetTitle method for title state retrieval. | ||
| 44 | 71 | ||||
| 45 | func TestGetTitle_ReturnsEmptyForUnknownPane(t *testing.T) { | 72 | func TestGetTitle_ReturnsEmptyForUnknownPane(t *testing.T) { | ||