fix: address remaining CodeRabbit issues
- commit
- d1704de8b88d7eaa6106054b63c048da2df8b9c7
- author
- brice · · 2026-04-16 04:05 UTC
- parent
- dccb0ab
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
10 files · +170 -39
M cmd/dumber/main.go +6 -2
| @@ -115,8 +115,12 @@ func main() { | |||
| 115 | 115 | // Run GUI mode for browse command | |
| 116 | 116 | if mode == launchModeBrowse { | |
| 117 | 117 | if forwarded, err := tryForwardBrowseURLToRunningInstance(context.Background(), browserLaunchRelay, browseURL); err != nil { | |
| 118 | - | fmt.Fprintf(os.Stderr, "failed to forward browse URL %q to a running instance: %v\n", browseURL, err) | |
| 119 | - | os.Exit(1) | |
| 118 | + | fmt.Fprintf( | |
| 119 | + | os.Stderr, | |
| 120 | + | "warning: failed to forward browse URL %q to a running instance, falling back to a new process: %v\n", | |
| 121 | + | browseURL, | |
| 122 | + | err, | |
| 123 | + | ) | |
| 120 | 124 | } else if forwarded { | |
| 121 | 125 | os.Exit(0) | |
| 122 | 126 | } | |
| @@ -115,8 +115,12 @@ func main() { | |||||
| 115 | // Run GUI mode for browse command | 115 | // Run GUI mode for browse command | ||
| 116 | if mode == launchModeBrowse { | 116 | if mode == launchModeBrowse { | ||
| 117 | if forwarded, err := tryForwardBrowseURLToRunningInstance(context.Background(), browserLaunchRelay, browseURL); err != nil { | 117 | if forwarded, err := tryForwardBrowseURLToRunningInstance(context.Background(), browserLaunchRelay, browseURL); err != nil { | ||
| 118 | - | fmt.Fprintf(os.Stderr, "failed to forward browse URL %q to a running instance: %v\n", browseURL, err) | 118 | + | fmt.Fprintf( |
| 119 | - | os.Exit(1) | 119 | + | os.Stderr, |
| 120 | + | "warning: failed to forward browse URL %q to a running instance, falling back to a new process: %v\n", | |||
| 121 | + | browseURL, | |||
| 122 | + | err, | |||
| 123 | + | ) | |||
| 120 | } else if forwarded { | 124 | } else if forwarded { | ||
| 121 | os.Exit(0) | 125 | os.Exit(0) | ||
| 122 | } | 126 | } | ||
M internal/infrastructure/cef/app.go +4 -2
| @@ -199,8 +199,10 @@ func parseBrowseURLFromRelaunchCommandLine(commandLine purecef.CommandLine) stri | |||
| 199 | 199 | ||
| 200 | 200 | func (h *dumberBPH) OnAlreadyRunningAppRelaunch(commandLine purecef.CommandLine, _ string) int32 { | |
| 201 | 201 | if browseURL, ok := parseBrowseRelaunchCommandLine(commandLine); ok { | |
| 202 | - | if h != nil && h.engine != nil && h.engine.alreadyRunningAppRelaunchHandler != nil { | |
| 203 | - | h.engine.alreadyRunningAppRelaunchHandler(browseURL) | |
| 202 | + | if h != nil && h.engine != nil { | |
| 203 | + | if handler := h.engine.alreadyRunningAppRelaunchCallback(); handler != nil { | |
| 204 | + | handler(browseURL) | |
| 205 | + | } | |
| 204 | 206 | } | |
| 205 | 207 | return 1 | |
| 206 | 208 | } | |
| @@ -199,8 +199,10 @@ func parseBrowseURLFromRelaunchCommandLine(commandLine purecef.CommandLine) stri | |||||
| 199 | 199 | ||||
| 200 | func (h *dumberBPH) OnAlreadyRunningAppRelaunch(commandLine purecef.CommandLine, _ string) int32 { | 200 | func (h *dumberBPH) OnAlreadyRunningAppRelaunch(commandLine purecef.CommandLine, _ string) int32 { | ||
| 201 | if browseURL, ok := parseBrowseRelaunchCommandLine(commandLine); ok { | 201 | if browseURL, ok := parseBrowseRelaunchCommandLine(commandLine); ok { | ||
| 202 | - | if h != nil && h.engine != nil && h.engine.alreadyRunningAppRelaunchHandler != nil { | 202 | + | if h != nil && h.engine != nil { |
| 203 | - | h.engine.alreadyRunningAppRelaunchHandler(browseURL) | 203 | + | if handler := h.engine.alreadyRunningAppRelaunchCallback(); handler != nil { |
| 204 | + | handler(browseURL) | |||
| 205 | + | } | |||
| 204 | } | 206 | } | ||
| 205 | return 1 | 207 | return 1 | ||
| 206 | } | 208 | } | ||
M internal/infrastructure/cef/engine.go +9 -0
| @@ -37,6 +37,7 @@ type Engine struct { | |||
| 37 | 37 | clipboard port.Clipboard | |
| 38 | 38 | resolver port.ImageDataResolver | |
| 39 | 39 | mediaClassifier MediaClassifier | |
| 40 | + | alreadyRunningAppRelaunchMu sync.RWMutex | |
| 40 | 41 | alreadyRunningAppRelaunchHandler func(string) | |
| 41 | 42 | ||
| 42 | 43 | // activeWebViews tracks all live webviews for CSS broadcast. | |
| @@ -119,9 +120,17 @@ func (e *Engine) SetColorResolver(resolver port.ColorSchemeResolver) { | |||
| 119 | 120 | // SetAlreadyRunningAppRelaunchHandler configures the callback invoked when CEF | |
| 120 | 121 | // relaunches an already-running app instance. | |
| 121 | 122 | func (e *Engine) SetAlreadyRunningAppRelaunchHandler(handler func(string)) { | |
| 123 | + | e.alreadyRunningAppRelaunchMu.Lock() | |
| 124 | + | defer e.alreadyRunningAppRelaunchMu.Unlock() | |
| 122 | 125 | e.alreadyRunningAppRelaunchHandler = handler | |
| 123 | 126 | } | |
| 124 | 127 | ||
| 128 | + | func (e *Engine) alreadyRunningAppRelaunchCallback() func(string) { | |
| 129 | + | e.alreadyRunningAppRelaunchMu.RLock() | |
| 130 | + | defer e.alreadyRunningAppRelaunchMu.RUnlock() | |
| 131 | + | return e.alreadyRunningAppRelaunchHandler | |
| 132 | + | } | |
| 133 | + | ||
| 125 | 134 | // registerWebView adds a webview to the active tracking map. | |
| 126 | 135 | func (e *Engine) registerWebView(wv *WebView) { | |
| 127 | 136 | e.activeWebViews.Store(wv.id, wv) | |
| @@ -37,6 +37,7 @@ type Engine struct { | |||||
| 37 | clipboard port.Clipboard | 37 | clipboard port.Clipboard | ||
| 38 | resolver port.ImageDataResolver | 38 | resolver port.ImageDataResolver | ||
| 39 | mediaClassifier MediaClassifier | 39 | mediaClassifier MediaClassifier | ||
| 40 | + | alreadyRunningAppRelaunchMu sync.RWMutex | |||
| 40 | alreadyRunningAppRelaunchHandler func(string) | 41 | alreadyRunningAppRelaunchHandler func(string) | ||
| 41 | 42 | ||||
| 42 | // activeWebViews tracks all live webviews for CSS broadcast. | 43 | // activeWebViews tracks all live webviews for CSS broadcast. | ||
| @@ -119,9 +120,17 @@ func (e *Engine) SetColorResolver(resolver port.ColorSchemeResolver) { | |||||
| 119 | // SetAlreadyRunningAppRelaunchHandler configures the callback invoked when CEF | 120 | // SetAlreadyRunningAppRelaunchHandler configures the callback invoked when CEF | ||
| 120 | // relaunches an already-running app instance. | 121 | // relaunches an already-running app instance. | ||
| 121 | func (e *Engine) SetAlreadyRunningAppRelaunchHandler(handler func(string)) { | 122 | func (e *Engine) SetAlreadyRunningAppRelaunchHandler(handler func(string)) { | ||
| 123 | + | e.alreadyRunningAppRelaunchMu.Lock() | |||
| 124 | + | defer e.alreadyRunningAppRelaunchMu.Unlock() | |||
| 122 | e.alreadyRunningAppRelaunchHandler = handler | 125 | e.alreadyRunningAppRelaunchHandler = handler | ||
| 123 | } | 126 | } | ||
| 124 | 127 | ||||
| 128 | + | func (e *Engine) alreadyRunningAppRelaunchCallback() func(string) { | |||
| 129 | + | e.alreadyRunningAppRelaunchMu.RLock() | |||
| 130 | + | defer e.alreadyRunningAppRelaunchMu.RUnlock() | |||
| 131 | + | return e.alreadyRunningAppRelaunchHandler | |||
| 132 | + | } | |||
| 133 | + | ||||
| 125 | // registerWebView adds a webview to the active tracking map. | 134 | // registerWebView adds a webview to the active tracking map. | ||
| 126 | func (e *Engine) registerWebView(wv *WebView) { | 135 | func (e *Engine) registerWebView(wv *WebView) { | ||
| 127 | e.activeWebViews.Store(wv.id, wv) | 136 | e.activeWebViews.Store(wv.id, wv) | ||
M internal/infrastructure/cef/engine_init_test.go +3 -0
| @@ -27,6 +27,9 @@ func TestResolvedStateRoot_UsesEnvOverride(t *testing.T) { | |||
| 27 | 27 | ||
| 28 | 28 | root := resolvedStateRoot(port.EngineOptions{}) | |
| 29 | 29 | require.Equal(t, "/tmp/override", root) | |
| 30 | + | ||
| 31 | + | root = resolvedStateRoot(port.EngineOptions{DataDir: "/tmp/data", CacheDir: "/tmp/cache"}) | |
| 32 | + | require.Equal(t, "/tmp/override", root) | |
| 30 | 33 | } | |
| 31 | 34 | ||
| 32 | 35 | func TestPrepareCEFSettings_UsesResolvedStateRoot(t *testing.T) { | |
| @@ -27,6 +27,9 @@ func TestResolvedStateRoot_UsesEnvOverride(t *testing.T) { | |||||
| 27 | 27 | ||||
| 28 | root := resolvedStateRoot(port.EngineOptions{}) | 28 | root := resolvedStateRoot(port.EngineOptions{}) | ||
| 29 | require.Equal(t, "/tmp/override", root) | 29 | require.Equal(t, "/tmp/override", root) | ||
| 30 | + | ||||
| 31 | + | root = resolvedStateRoot(port.EngineOptions{DataDir: "/tmp/data", CacheDir: "/tmp/cache"}) | |||
| 32 | + | require.Equal(t, "/tmp/override", root) | |||
| 30 | } | 33 | } | ||
| 31 | 34 | ||||
| 32 | func TestPrepareCEFSettings_UsesResolvedStateRoot(t *testing.T) { | 35 | func TestPrepareCEFSettings_UsesResolvedStateRoot(t *testing.T) { | ||
M internal/infrastructure/desktop/adapter.go +31 -2
| @@ -481,6 +481,33 @@ func sanitizedChildEnv(env []string) []string { | |||
| 481 | 481 | return cleaned | |
| 482 | 482 | } | |
| 483 | 483 | ||
| 484 | + | func withoutEnvKeys(env []string, keys ...string) []string { | |
| 485 | + | if len(keys) == 0 { | |
| 486 | + | return append([]string(nil), env...) | |
| 487 | + | } | |
| 488 | + | ||
| 489 | + | blocked := make(map[string]struct{}, len(keys)) | |
| 490 | + | for _, key := range keys { | |
| 491 | + | if key == "" { | |
| 492 | + | continue | |
| 493 | + | } | |
| 494 | + | blocked[key] = struct{}{} | |
| 495 | + | } | |
| 496 | + | ||
| 497 | + | filtered := make([]string, 0, len(env)) | |
| 498 | + | for _, entry := range env { | |
| 499 | + | key, _, ok := strings.Cut(entry, "=") | |
| 500 | + | if ok { | |
| 501 | + | if _, drop := blocked[key]; drop { | |
| 502 | + | continue | |
| 503 | + | } | |
| 504 | + | } | |
| 505 | + | filtered = append(filtered, entry) | |
| 506 | + | } | |
| 507 | + | ||
| 508 | + | return filtered | |
| 509 | + | } | |
| 510 | + | ||
| 484 | 511 | // SpawnWithSession starts a new dumber instance to restore a session. | |
| 485 | 512 | func (s *SessionSpawner) SpawnWithSession(sessionID entity.SessionID) error { | |
| 486 | 513 | log := logging.FromContext(s.ctx) | |
| @@ -494,11 +521,13 @@ func (s *SessionSpawner) SpawnWithSession(sessionID entity.SessionID) error { | |||
| 494 | 521 | cmd := exec.Command(execPath, "browse") | |
| 495 | 522 | ||
| 496 | 523 | // Restore sessions get an isolated CEF state root. | |
| 497 | - | cmd.Env = append( | |
| 498 | - | sanitizedChildEnv(os.Environ()), | |
| 524 | + | sanitizedEnv := withoutEnvKeys(sanitizedChildEnv(os.Environ()), RestoreSessionEnvVar, cef.CEFRootCachePathEnvVar) | |
| 525 | + | sanitizedEnv = append( | |
| 526 | + | sanitizedEnv, | |
| 499 | 527 | RestoreSessionEnvVar+"="+string(sessionID), | |
| 500 | 528 | cef.CEFRootCachePathEnvVar+"="+sessionCEFRootCachePath(sessionID), | |
| 501 | 529 | ) | |
| 530 | + | cmd.Env = sanitizedEnv | |
| 502 | 531 | ||
| 503 | 532 | // Detach from current process group so the new process survives. | |
| 504 | 533 | cmd.Stdin = nil | |
| @@ -481,6 +481,33 @@ func sanitizedChildEnv(env []string) []string { | |||||
| 481 | return cleaned | 481 | return cleaned | ||
| 482 | } | 482 | } | ||
| 483 | 483 | ||||
| 484 | + | func withoutEnvKeys(env []string, keys ...string) []string { | |||
| 485 | + | if len(keys) == 0 { | |||
| 486 | + | return append([]string(nil), env...) | |||
| 487 | + | } | |||
| 488 | + | ||||
| 489 | + | blocked := make(map[string]struct{}, len(keys)) | |||
| 490 | + | for _, key := range keys { | |||
| 491 | + | if key == "" { | |||
| 492 | + | continue | |||
| 493 | + | } | |||
| 494 | + | blocked[key] = struct{}{} | |||
| 495 | + | } | |||
| 496 | + | ||||
| 497 | + | filtered := make([]string, 0, len(env)) | |||
| 498 | + | for _, entry := range env { | |||
| 499 | + | key, _, ok := strings.Cut(entry, "=") | |||
| 500 | + | if ok { | |||
| 501 | + | if _, drop := blocked[key]; drop { | |||
| 502 | + | continue | |||
| 503 | + | } | |||
| 504 | + | } | |||
| 505 | + | filtered = append(filtered, entry) | |||
| 506 | + | } | |||
| 507 | + | ||||
| 508 | + | return filtered | |||
| 509 | + | } | |||
| 510 | + | ||||
| 484 | // SpawnWithSession starts a new dumber instance to restore a session. | 511 | // SpawnWithSession starts a new dumber instance to restore a session. | ||
| 485 | func (s *SessionSpawner) SpawnWithSession(sessionID entity.SessionID) error { | 512 | func (s *SessionSpawner) SpawnWithSession(sessionID entity.SessionID) error { | ||
| 486 | log := logging.FromContext(s.ctx) | 513 | log := logging.FromContext(s.ctx) | ||
| @@ -494,11 +521,13 @@ func (s *SessionSpawner) SpawnWithSession(sessionID entity.SessionID) error { | |||||
| 494 | cmd := exec.Command(execPath, "browse") | 521 | cmd := exec.Command(execPath, "browse") | ||
| 495 | 522 | ||||
| 496 | // Restore sessions get an isolated CEF state root. | 523 | // Restore sessions get an isolated CEF state root. | ||
| 497 | - | cmd.Env = append( | 524 | + | sanitizedEnv := withoutEnvKeys(sanitizedChildEnv(os.Environ()), RestoreSessionEnvVar, cef.CEFRootCachePathEnvVar) |
| 498 | - | sanitizedChildEnv(os.Environ()), | 525 | + | sanitizedEnv = append( |
| 526 | + | sanitizedEnv, | |||
| 499 | RestoreSessionEnvVar+"="+string(sessionID), | 527 | RestoreSessionEnvVar+"="+string(sessionID), | ||
| 500 | cef.CEFRootCachePathEnvVar+"="+sessionCEFRootCachePath(sessionID), | 528 | cef.CEFRootCachePathEnvVar+"="+sessionCEFRootCachePath(sessionID), | ||
| 501 | ) | 529 | ) | ||
| 530 | + | cmd.Env = sanitizedEnv | |||
| 502 | 531 | ||||
| 503 | // Detach from current process group so the new process survives. | 532 | // Detach from current process group so the new process survives. | ||
| 504 | cmd.Stdin = nil | 533 | cmd.Stdin = nil | ||
M internal/infrastructure/desktop/adapter_test.go +12 -0
| @@ -55,6 +55,18 @@ func TestSanitizedChildEnv_PreservesColonSeparatedEntries(t *testing.T) { | |||
| 55 | 55 | } | |
| 56 | 56 | } | |
| 57 | 57 | ||
| 58 | + | func TestWithoutEnvKeys_RemovesExplicitOverrides(t *testing.T) { | |
| 59 | + | env := withoutEnvKeys([]string{ | |
| 60 | + | "PATH=/usr/bin", | |
| 61 | + | RestoreSessionEnvVar + "=old-session", | |
| 62 | + | cef.CEFRootCachePathEnvVar + "=/tmp/old-root", | |
| 63 | + | }, RestoreSessionEnvVar, cef.CEFRootCachePathEnvVar) | |
| 64 | + | ||
| 65 | + | if len(env) != 1 || env[0] != "PATH=/usr/bin" { | |
| 66 | + | t.Fatalf("expected override keys to be removed, got %#v", env) | |
| 67 | + | } | |
| 68 | + | } | |
| 69 | + | ||
| 58 | 70 | func TestSessionSpawner_SpawnWithSession_PassesRestoreSessionAndCEFOverride(t *testing.T) { | |
| 59 | 71 | spawner := NewSessionSpawner(t.Context()) | |
| 60 | 72 | childEnvFile := filepath.Join(t.TempDir(), "child-env.txt") | |
| @@ -55,6 +55,18 @@ func TestSanitizedChildEnv_PreservesColonSeparatedEntries(t *testing.T) { | |||||
| 55 | } | 55 | } | ||
| 56 | } | 56 | } | ||
| 57 | 57 | ||||
| 58 | + | func TestWithoutEnvKeys_RemovesExplicitOverrides(t *testing.T) { | |||
| 59 | + | env := withoutEnvKeys([]string{ | |||
| 60 | + | "PATH=/usr/bin", | |||
| 61 | + | RestoreSessionEnvVar + "=old-session", | |||
| 62 | + | cef.CEFRootCachePathEnvVar + "=/tmp/old-root", | |||
| 63 | + | }, RestoreSessionEnvVar, cef.CEFRootCachePathEnvVar) | |||
| 64 | + | ||||
| 65 | + | if len(env) != 1 || env[0] != "PATH=/usr/bin" { | |||
| 66 | + | t.Fatalf("expected override keys to be removed, got %#v", env) | |||
| 67 | + | } | |||
| 68 | + | } | |||
| 69 | + | ||||
| 58 | func TestSessionSpawner_SpawnWithSession_PassesRestoreSessionAndCEFOverride(t *testing.T) { | 70 | func TestSessionSpawner_SpawnWithSession_PassesRestoreSessionAndCEFOverride(t *testing.T) { | ||
| 59 | spawner := NewSessionSpawner(t.Context()) | 71 | spawner := NewSessionSpawner(t.Context()) | ||
| 60 | childEnvFile := filepath.Join(t.TempDir(), "child-env.txt") | 72 | childEnvFile := filepath.Join(t.TempDir(), "child-env.txt") | ||
M internal/ui/app.go +62 -25
| @@ -614,6 +614,16 @@ func (a *App) setBrowserWindowForTab(tabID entity.TabID, bw *browserWindow) { | |||
| 614 | 614 | } | |
| 615 | 615 | } | |
| 616 | 616 | ||
| 617 | + | func (a *App) tabBarForBrowserWindow(bw *browserWindow) *component.TabBar { | |
| 618 | + | if bw != nil && bw.mainWindow != nil { | |
| 619 | + | return bw.mainWindow.TabBar() | |
| 620 | + | } | |
| 621 | + | if a.mainWindow != nil { | |
| 622 | + | return a.mainWindow.TabBar() | |
| 623 | + | } | |
| 624 | + | return nil | |
| 625 | + | } | |
| 626 | + | ||
| 617 | 627 | func (a *App) startBrowserLaunchRelayListener(ctx context.Context) { | |
| 618 | 628 | if a == nil || a.deps == nil || a.deps.BrowserLaunchRelay == nil { | |
| 619 | 629 | return | |
| @@ -1363,6 +1373,17 @@ func (a *App) MoveActivePaneToTab(ctx context.Context, targetTabID entity.TabID) | |||
| 1363 | 1373 | return nil | |
| 1364 | 1374 | } | |
| 1365 | 1375 | ||
| 1376 | + | sourceWindow := a.browserWindowForTab(sourceTab.ID) | |
| 1377 | + | if sourceWindow == nil { | |
| 1378 | + | sourceWindow = a.browserWindowForMainWindow(a.mainWindow) | |
| 1379 | + | } | |
| 1380 | + | targetWindow := sourceWindow | |
| 1381 | + | if targetTabID != "" { | |
| 1382 | + | if owner := a.browserWindowForTab(targetTabID); owner != nil { | |
| 1383 | + | targetWindow = owner | |
| 1384 | + | } | |
| 1385 | + | } | |
| 1386 | + | ||
| 1366 | 1387 | out, err := a.movePaneToTabUC.Execute(*in) | |
| 1367 | 1388 | if err != nil { | |
| 1368 | 1389 | return err | |
| @@ -1370,24 +1391,19 @@ func (a *App) MoveActivePaneToTab(ctx context.Context, targetTabID entity.TabID) | |||
| 1370 | 1391 | if out == nil || out.TargetTab == nil { | |
| 1371 | 1392 | return nil | |
| 1372 | 1393 | } | |
| 1394 | + | if out.NewTabCreated && targetWindow != nil { | |
| 1395 | + | a.setBrowserWindowForTab(out.TargetTab.ID, targetWindow) | |
| 1396 | + | } | |
| 1373 | 1397 | ||
| 1374 | - | a.applyMovePaneToTabUI(ctx, out, sourceTab) | |
| 1375 | - | a.switchToTargetTabIfConfigured(ctx, out) | |
| 1398 | + | a.applyMovePaneToTabUI(ctx, out, sourceTab, sourceWindow, targetWindow) | |
| 1399 | + | a.switchToTargetTabIfConfigured(ctx, out, targetWindow) | |
| 1376 | 1400 | a.updateTabBarVisibility(ctx) | |
| 1377 | 1401 | a.MarkDirty() | |
| 1378 | 1402 | return nil | |
| 1379 | 1403 | } | |
| 1380 | 1404 | ||
| 1381 | 1405 | func (a *App) moveActivePaneToTabFromBrowserWindow(ctx context.Context, bw *browserWindow, targetTabID entity.TabID) error { | |
| 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 { | |
| 1406 | + | if bw != nil { | |
| 1391 | 1407 | a.activateBrowserWindow(bw) | |
| 1392 | 1408 | } | |
| 1393 | 1409 | return a.MoveActivePaneToTab(ctx, targetTabID) | |
| @@ -1415,7 +1431,13 @@ func (a *App) buildMovePaneToTabInput(targetTabID entity.TabID) (*usecase.MovePa | |||
| 1415 | 1431 | return in, activeTab | |
| 1416 | 1432 | } | |
| 1417 | 1433 | ||
| 1418 | - | func (a *App) applyMovePaneToTabUI(ctx context.Context, out *usecase.MovePaneToTabOutput, sourceTab *entity.Tab) { | |
| 1434 | + | func (a *App) applyMovePaneToTabUI( | |
| 1435 | + | ctx context.Context, | |
| 1436 | + | out *usecase.MovePaneToTabOutput, | |
| 1437 | + | sourceTab *entity.Tab, | |
| 1438 | + | sourceWindow *browserWindow, | |
| 1439 | + | targetWindow *browserWindow, | |
| 1440 | + | ) { | |
| 1419 | 1441 | if out == nil || out.TargetTab == nil { | |
| 1420 | 1442 | return | |
| 1421 | 1443 | } | |
| @@ -1425,10 +1447,10 @@ func (a *App) applyMovePaneToTabUI(ctx context.Context, out *usecase.MovePaneToT | |||
| 1425 | 1447 | } | |
| 1426 | 1448 | ||
| 1427 | 1449 | if out.SourceTabClosed { | |
| 1428 | - | a.removeSourceTabUI(sourceTabID) | |
| 1450 | + | a.removeSourceTabUI(sourceTabID, sourceWindow) | |
| 1429 | 1451 | } | |
| 1430 | 1452 | if out.NewTabCreated { | |
| 1431 | - | a.ensureTargetTabUI(ctx, out.TargetTab) | |
| 1453 | + | a.ensureTargetTabUI(ctx, out.TargetTab, targetWindow) | |
| 1432 | 1454 | } | |
| 1433 | 1455 | ||
| 1434 | 1456 | if !out.SourceTabClosed { | |
| @@ -1437,20 +1459,20 @@ func (a *App) applyMovePaneToTabUI(ctx context.Context, out *usecase.MovePaneToT | |||
| 1437 | 1459 | a.rebuildAndAttachWorkspace(ctx, out.TargetTab.ID, out.TargetTab) | |
| 1438 | 1460 | } | |
| 1439 | 1461 | ||
| 1440 | - | func (a *App) removeSourceTabUI(sourceTabID entity.TabID) { | |
| 1462 | + | func (a *App) removeSourceTabUI(sourceTabID entity.TabID, sourceWindow *browserWindow) { | |
| 1441 | 1463 | a.releaseFloatingSessionsForTab(context.Background(), sourceTabID) | |
| 1442 | 1464 | delete(a.workspaceViews, sourceTabID) | |
| 1443 | - | if a.mainWindow != nil && a.mainWindow.TabBar() != nil { | |
| 1444 | - | a.mainWindow.TabBar().RemoveTab(sourceTabID) | |
| 1465 | + | if tabBar := a.tabBarForBrowserWindow(sourceWindow); tabBar != nil { | |
| 1466 | + | tabBar.RemoveTab(sourceTabID) | |
| 1445 | 1467 | } | |
| 1446 | 1468 | } | |
| 1447 | 1469 | ||
| 1448 | - | func (a *App) ensureTargetTabUI(ctx context.Context, tab *entity.Tab) { | |
| 1470 | + | func (a *App) ensureTargetTabUI(ctx context.Context, tab *entity.Tab, targetWindow *browserWindow) { | |
| 1449 | 1471 | if tab == nil { | |
| 1450 | 1472 | return | |
| 1451 | 1473 | } | |
| 1452 | - | if a.mainWindow != nil && a.mainWindow.TabBar() != nil { | |
| 1453 | - | a.mainWindow.TabBar().AddTab(tab) | |
| 1474 | + | if tabBar := a.tabBarForBrowserWindow(targetWindow); tabBar != nil { | |
| 1475 | + | tabBar.AddTab(tab) | |
| 1454 | 1476 | } | |
| 1455 | 1477 | a.createWorkspaceViewWithoutAttach(ctx, tab) | |
| 1456 | 1478 | } | |
| @@ -1498,12 +1520,19 @@ func (a *App) reattachFloatingSessions(tabID entity.TabID, wsView *component.Wor | |||
| 1498 | 1520 | } | |
| 1499 | 1521 | } | |
| 1500 | 1522 | ||
| 1501 | - | func (a *App) switchToTargetTabIfConfigured(ctx context.Context, out *usecase.MovePaneToTabOutput) { | |
| 1523 | + | func (a *App) switchToTargetTabIfConfigured( | |
| 1524 | + | ctx context.Context, | |
| 1525 | + | out *usecase.MovePaneToTabOutput, | |
| 1526 | + | targetWindow *browserWindow, | |
| 1527 | + | ) { | |
| 1502 | 1528 | if out == nil || out.TargetTab == nil { | |
| 1503 | 1529 | return | |
| 1504 | 1530 | } | |
| 1505 | 1531 | ||
| 1506 | - | switchToTarget := a.deps.Config.Workspace.SwitchToTabOnMove | |
| 1532 | + | switchToTarget := false | |
| 1533 | + | if a.deps != nil && a.deps.Config != nil { | |
| 1534 | + | switchToTarget = a.deps.Config.Workspace.SwitchToTabOnMove | |
| 1535 | + | } | |
| 1507 | 1536 | if out.SourceTabClosed { | |
| 1508 | 1537 | switchToTarget = true | |
| 1509 | 1538 | } | |
| @@ -1512,8 +1541,14 @@ func (a *App) switchToTargetTabIfConfigured(ctx context.Context, out *usecase.Mo | |||
| 1512 | 1541 | } | |
| 1513 | 1542 | ||
| 1514 | 1543 | a.tabs.SetActive(out.TargetTab.ID) | |
| 1515 | - | if a.mainWindow != nil && a.mainWindow.TabBar() != nil { | |
| 1516 | - | a.mainWindow.TabBar().SetActive(out.TargetTab.ID) | |
| 1544 | + | if targetWindow != nil { | |
| 1545 | + | targetWindow.activeTabID = out.TargetTab.ID | |
| 1546 | + | } | |
| 1547 | + | if tabBar := a.tabBarForBrowserWindow(targetWindow); tabBar != nil { | |
| 1548 | + | tabBar.SetActive(out.TargetTab.ID) | |
| 1549 | + | } | |
| 1550 | + | if targetWindow != nil { | |
| 1551 | + | a.activateBrowserWindow(targetWindow) | |
| 1517 | 1552 | } | |
| 1518 | 1553 | a.switchWorkspaceView(ctx, out.TargetTab.ID) | |
| 1519 | 1554 | } | |
| @@ -1807,6 +1842,9 @@ func (a *App) onShutdown(ctx context.Context) { | |||
| 1807 | 1842 | } | |
| 1808 | 1843 | } | |
| 1809 | 1844 | ||
| 1845 | + | // Stop accepting relaunches before teardown starts. | |
| 1846 | + | a.closeBrowserLaunchRelayListener() | |
| 1847 | + | ||
| 1810 | 1848 | // Cancel context to signal all goroutines | |
| 1811 | 1849 | a.cancel(errors.New("application shutdown")) | |
| 1812 | 1850 | ||
| @@ -1836,7 +1874,6 @@ func (a *App) onShutdown(ctx context.Context) { | |||
| 1836 | 1874 | log.Warn().Err(err).Msg("failed to close idle inhibitor") | |
| 1837 | 1875 | } | |
| 1838 | 1876 | } | |
| 1839 | - | a.closeBrowserLaunchRelayListener() | |
| 1840 | 1877 | ||
| 1841 | 1878 | log.Info().Msg("application shutdown complete") | |
| 1842 | 1879 | } | |
| @@ -614,6 +614,16 @@ func (a *App) setBrowserWindowForTab(tabID entity.TabID, bw *browserWindow) { | |||||
| 614 | } | 614 | } | ||
| 615 | } | 615 | } | ||
| 616 | 616 | ||||
| 617 | + | func (a *App) tabBarForBrowserWindow(bw *browserWindow) *component.TabBar { | |||
| 618 | + | if bw != nil && bw.mainWindow != nil { | |||
| 619 | + | return bw.mainWindow.TabBar() | |||
| 620 | + | } | |||
| 621 | + | if a.mainWindow != nil { | |||
| 622 | + | return a.mainWindow.TabBar() | |||
| 623 | + | } | |||
| 624 | + | return nil | |||
| 625 | + | } | |||
| 626 | + | ||||
| 617 | func (a *App) startBrowserLaunchRelayListener(ctx context.Context) { | 627 | func (a *App) startBrowserLaunchRelayListener(ctx context.Context) { | ||
| 618 | if a == nil || a.deps == nil || a.deps.BrowserLaunchRelay == nil { | 628 | if a == nil || a.deps == nil || a.deps.BrowserLaunchRelay == nil { | ||
| 619 | return | 629 | return | ||
| @@ -1363,6 +1373,17 @@ func (a *App) MoveActivePaneToTab(ctx context.Context, targetTabID entity.TabID) | |||||
| 1363 | return nil | 1373 | return nil | ||
| 1364 | } | 1374 | } | ||
| 1365 | 1375 | ||||
| 1376 | + | sourceWindow := a.browserWindowForTab(sourceTab.ID) | |||
| 1377 | + | if sourceWindow == nil { | |||
| 1378 | + | sourceWindow = a.browserWindowForMainWindow(a.mainWindow) | |||
| 1379 | + | } | |||
| 1380 | + | targetWindow := sourceWindow | |||
| 1381 | + | if targetTabID != "" { | |||
| 1382 | + | if owner := a.browserWindowForTab(targetTabID); owner != nil { | |||
| 1383 | + | targetWindow = owner | |||
| 1384 | + | } | |||
| 1385 | + | } | |||
| 1386 | + | ||||
| 1366 | out, err := a.movePaneToTabUC.Execute(*in) | 1387 | out, err := a.movePaneToTabUC.Execute(*in) | ||
| 1367 | if err != nil { | 1388 | if err != nil { | ||
| 1368 | return err | 1389 | return err | ||
| @@ -1370,24 +1391,19 @@ func (a *App) MoveActivePaneToTab(ctx context.Context, targetTabID entity.TabID) | |||||
| 1370 | if out == nil || out.TargetTab == nil { | 1391 | if out == nil || out.TargetTab == nil { | ||
| 1371 | return nil | 1392 | return nil | ||
| 1372 | } | 1393 | } | ||
| 1394 | + | if out.NewTabCreated && targetWindow != nil { | |||
| 1395 | + | a.setBrowserWindowForTab(out.TargetTab.ID, targetWindow) | |||
| 1396 | + | } | |||
| 1373 | 1397 | ||||
| 1374 | - | a.applyMovePaneToTabUI(ctx, out, sourceTab) | 1398 | + | a.applyMovePaneToTabUI(ctx, out, sourceTab, sourceWindow, targetWindow) |
| 1375 | - | a.switchToTargetTabIfConfigured(ctx, out) | 1399 | + | a.switchToTargetTabIfConfigured(ctx, out, targetWindow) |
| 1376 | a.updateTabBarVisibility(ctx) | 1400 | a.updateTabBarVisibility(ctx) | ||
| 1377 | a.MarkDirty() | 1401 | a.MarkDirty() | ||
| 1378 | return nil | 1402 | return nil | ||
| 1379 | } | 1403 | } | ||
| 1380 | 1404 | ||||
| 1381 | func (a *App) moveActivePaneToTabFromBrowserWindow(ctx context.Context, bw *browserWindow, targetTabID entity.TabID) error { | 1405 | func (a *App) moveActivePaneToTabFromBrowserWindow(ctx context.Context, bw *browserWindow, targetTabID entity.TabID) error { | ||
| 1382 | - | if targetTabID == "" { | 1406 | + | if bw != nil { |
| 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 { | |||
| 1391 | a.activateBrowserWindow(bw) | 1407 | a.activateBrowserWindow(bw) | ||
| 1392 | } | 1408 | } | ||
| 1393 | return a.MoveActivePaneToTab(ctx, targetTabID) | 1409 | return a.MoveActivePaneToTab(ctx, targetTabID) | ||
| @@ -1415,7 +1431,13 @@ func (a *App) buildMovePaneToTabInput(targetTabID entity.TabID) (*usecase.MovePa | |||||
| 1415 | return in, activeTab | 1431 | return in, activeTab | ||
| 1416 | } | 1432 | } | ||
| 1417 | 1433 | ||||
| 1418 | - | func (a *App) applyMovePaneToTabUI(ctx context.Context, out *usecase.MovePaneToTabOutput, sourceTab *entity.Tab) { | 1434 | + | func (a *App) applyMovePaneToTabUI( |
| 1435 | + | ctx context.Context, | |||
| 1436 | + | out *usecase.MovePaneToTabOutput, | |||
| 1437 | + | sourceTab *entity.Tab, | |||
| 1438 | + | sourceWindow *browserWindow, | |||
| 1439 | + | targetWindow *browserWindow, | |||
| 1440 | + | ) { | |||
| 1419 | if out == nil || out.TargetTab == nil { | 1441 | if out == nil || out.TargetTab == nil { | ||
| 1420 | return | 1442 | return | ||
| 1421 | } | 1443 | } | ||
| @@ -1425,10 +1447,10 @@ func (a *App) applyMovePaneToTabUI(ctx context.Context, out *usecase.MovePaneToT | |||||
| 1425 | } | 1447 | } | ||
| 1426 | 1448 | ||||
| 1427 | if out.SourceTabClosed { | 1449 | if out.SourceTabClosed { | ||
| 1428 | - | a.removeSourceTabUI(sourceTabID) | 1450 | + | a.removeSourceTabUI(sourceTabID, sourceWindow) |
| 1429 | } | 1451 | } | ||
| 1430 | if out.NewTabCreated { | 1452 | if out.NewTabCreated { | ||
| 1431 | - | a.ensureTargetTabUI(ctx, out.TargetTab) | 1453 | + | a.ensureTargetTabUI(ctx, out.TargetTab, targetWindow) |
| 1432 | } | 1454 | } | ||
| 1433 | 1455 | ||||
| 1434 | if !out.SourceTabClosed { | 1456 | if !out.SourceTabClosed { | ||
| @@ -1437,20 +1459,20 @@ func (a *App) applyMovePaneToTabUI(ctx context.Context, out *usecase.MovePaneToT | |||||
| 1437 | a.rebuildAndAttachWorkspace(ctx, out.TargetTab.ID, out.TargetTab) | 1459 | a.rebuildAndAttachWorkspace(ctx, out.TargetTab.ID, out.TargetTab) | ||
| 1438 | } | 1460 | } | ||
| 1439 | 1461 | ||||
| 1440 | - | func (a *App) removeSourceTabUI(sourceTabID entity.TabID) { | 1462 | + | func (a *App) removeSourceTabUI(sourceTabID entity.TabID, sourceWindow *browserWindow) { |
| 1441 | a.releaseFloatingSessionsForTab(context.Background(), sourceTabID) | 1463 | a.releaseFloatingSessionsForTab(context.Background(), sourceTabID) | ||
| 1442 | delete(a.workspaceViews, sourceTabID) | 1464 | delete(a.workspaceViews, sourceTabID) | ||
| 1443 | - | if a.mainWindow != nil && a.mainWindow.TabBar() != nil { | 1465 | + | if tabBar := a.tabBarForBrowserWindow(sourceWindow); tabBar != nil { |
| 1444 | - | a.mainWindow.TabBar().RemoveTab(sourceTabID) | 1466 | + | tabBar.RemoveTab(sourceTabID) |
| 1445 | } | 1467 | } | ||
| 1446 | } | 1468 | } | ||
| 1447 | 1469 | ||||
| 1448 | - | func (a *App) ensureTargetTabUI(ctx context.Context, tab *entity.Tab) { | 1470 | + | func (a *App) ensureTargetTabUI(ctx context.Context, tab *entity.Tab, targetWindow *browserWindow) { |
| 1449 | if tab == nil { | 1471 | if tab == nil { | ||
| 1450 | return | 1472 | return | ||
| 1451 | } | 1473 | } | ||
| 1452 | - | if a.mainWindow != nil && a.mainWindow.TabBar() != nil { | 1474 | + | if tabBar := a.tabBarForBrowserWindow(targetWindow); tabBar != nil { |
| 1453 | - | a.mainWindow.TabBar().AddTab(tab) | 1475 | + | tabBar.AddTab(tab) |
| 1454 | } | 1476 | } | ||
| 1455 | a.createWorkspaceViewWithoutAttach(ctx, tab) | 1477 | a.createWorkspaceViewWithoutAttach(ctx, tab) | ||
| 1456 | } | 1478 | } | ||
| @@ -1498,12 +1520,19 @@ func (a *App) reattachFloatingSessions(tabID entity.TabID, wsView *component.Wor | |||||
| 1498 | } | 1520 | } | ||
| 1499 | } | 1521 | } | ||
| 1500 | 1522 | ||||
| 1501 | - | func (a *App) switchToTargetTabIfConfigured(ctx context.Context, out *usecase.MovePaneToTabOutput) { | 1523 | + | func (a *App) switchToTargetTabIfConfigured( |
| 1524 | + | ctx context.Context, | |||
| 1525 | + | out *usecase.MovePaneToTabOutput, | |||
| 1526 | + | targetWindow *browserWindow, | |||
| 1527 | + | ) { | |||
| 1502 | if out == nil || out.TargetTab == nil { | 1528 | if out == nil || out.TargetTab == nil { | ||
| 1503 | return | 1529 | return | ||
| 1504 | } | 1530 | } | ||
| 1505 | 1531 | ||||
| 1506 | - | switchToTarget := a.deps.Config.Workspace.SwitchToTabOnMove | 1532 | + | switchToTarget := false |
| 1533 | + | if a.deps != nil && a.deps.Config != nil { | |||
| 1534 | + | switchToTarget = a.deps.Config.Workspace.SwitchToTabOnMove | |||
| 1535 | + | } | |||
| 1507 | if out.SourceTabClosed { | 1536 | if out.SourceTabClosed { | ||
| 1508 | switchToTarget = true | 1537 | switchToTarget = true | ||
| 1509 | } | 1538 | } | ||
| @@ -1512,8 +1541,14 @@ func (a *App) switchToTargetTabIfConfigured(ctx context.Context, out *usecase.Mo | |||||
| 1512 | } | 1541 | } | ||
| 1513 | 1542 | ||||
| 1514 | a.tabs.SetActive(out.TargetTab.ID) | 1543 | a.tabs.SetActive(out.TargetTab.ID) | ||
| 1515 | - | if a.mainWindow != nil && a.mainWindow.TabBar() != nil { | 1544 | + | if targetWindow != nil { |
| 1516 | - | a.mainWindow.TabBar().SetActive(out.TargetTab.ID) | 1545 | + | targetWindow.activeTabID = out.TargetTab.ID |
| 1546 | + | } | |||
| 1547 | + | if tabBar := a.tabBarForBrowserWindow(targetWindow); tabBar != nil { | |||
| 1548 | + | tabBar.SetActive(out.TargetTab.ID) | |||
| 1549 | + | } | |||
| 1550 | + | if targetWindow != nil { | |||
| 1551 | + | a.activateBrowserWindow(targetWindow) | |||
| 1517 | } | 1552 | } | ||
| 1518 | a.switchWorkspaceView(ctx, out.TargetTab.ID) | 1553 | a.switchWorkspaceView(ctx, out.TargetTab.ID) | ||
| 1519 | } | 1554 | } | ||
| @@ -1807,6 +1842,9 @@ func (a *App) onShutdown(ctx context.Context) { | |||||
| 1807 | } | 1842 | } | ||
| 1808 | } | 1843 | } | ||
| 1809 | 1844 | ||||
| 1845 | + | // Stop accepting relaunches before teardown starts. | |||
| 1846 | + | a.closeBrowserLaunchRelayListener() | |||
| 1847 | + | ||||
| 1810 | // Cancel context to signal all goroutines | 1848 | // Cancel context to signal all goroutines | ||
| 1811 | a.cancel(errors.New("application shutdown")) | 1849 | a.cancel(errors.New("application shutdown")) | ||
| 1812 | 1850 | ||||
| @@ -1836,7 +1874,6 @@ func (a *App) onShutdown(ctx context.Context) { | |||||
| 1836 | log.Warn().Err(err).Msg("failed to close idle inhibitor") | 1874 | log.Warn().Err(err).Msg("failed to close idle inhibitor") | ||
| 1837 | } | 1875 | } | ||
| 1838 | } | 1876 | } | ||
| 1839 | - | a.closeBrowserLaunchRelayListener() | |||
| 1840 | 1877 | ||||
| 1841 | log.Info().Msg("application shutdown complete") | 1878 | log.Info().Msg("application shutdown complete") | ||
| 1842 | } | 1879 | } | ||
M internal/ui/app_browser_launch_test.go +18 -6
| @@ -93,8 +93,14 @@ func testHasUsableGTKDisplay() bool { | |||
| 93 | 93 | if displayNum == "" { | |
| 94 | 94 | return false | |
| 95 | 95 | } | |
| 96 | - | if _, err := os.Stat(filepath.Join("/tmp/.X11-unix", "X"+displayNum)); err == nil { | |
| 97 | - | return true | |
| 96 | + | x11SocketCandidates := []string{filepath.Join(os.TempDir(), ".X11-unix", "X"+displayNum)} | |
| 97 | + | if fallback := "/tmp/.X11-unix/X" + displayNum; fallback != x11SocketCandidates[0] { | |
| 98 | + | x11SocketCandidates = append(x11SocketCandidates, fallback) | |
| 99 | + | } | |
| 100 | + | for _, candidate := range x11SocketCandidates { | |
| 101 | + | if _, err := os.Stat(candidate); err == nil { | |
| 102 | + | return true | |
| 103 | + | } | |
| 98 | 104 | } | |
| 99 | 105 | return false | |
| 100 | 106 | } | |
| @@ -891,15 +897,21 @@ func TestApp_MoveActivePaneToTabFromBrowserWindowActivatesTargetOwnerForCrossWin | |||
| 891 | 897 | t.Fatalf("moveActivePaneToTabFromBrowserWindow returned error: %v", err) | |
| 892 | 898 | } | |
| 893 | 899 | ||
| 894 | - | if tabs.Find(tab1.ID) == nil { | |
| 895 | - | t.Fatalf("tab-1 should remain when target owner is activated first") | |
| 900 | + | if tabs.Find(tab1.ID) != nil { | |
| 901 | + | t.Fatalf("tab-1 should be removed after its active pane moves away") | |
| 896 | 902 | } | |
| 897 | - | if tabs.Find(tab2.ID) != nil { | |
| 898 | - | t.Fatalf("tab-2 should have been moved from the target owner window") | |
| 903 | + | if tabs.Find(tab2.ID) == nil { | |
| 904 | + | t.Fatalf("tab-2 should remain in the target owner window") | |
| 899 | 905 | } | |
| 900 | 906 | if tabs.Find(tab3.ID) == nil { | |
| 901 | 907 | t.Fatalf("tab-3 should remain as the move target") | |
| 902 | 908 | } | |
| 909 | + | if app.tabs.ActiveTabID != tab3.ID { | |
| 910 | + | t.Fatalf("active tab = %q, want %q", app.tabs.ActiveTabID, tab3.ID) | |
| 911 | + | } | |
| 912 | + | if app.lastFocusedWindowID != second.id { | |
| 913 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", app.lastFocusedWindowID, second.id) | |
| 914 | + | } | |
| 903 | 915 | } | |
| 904 | 916 | ||
| 905 | 917 | func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) { | |
| @@ -93,8 +93,14 @@ func testHasUsableGTKDisplay() bool { | |||||
| 93 | if displayNum == "" { | 93 | if displayNum == "" { | ||
| 94 | return false | 94 | return false | ||
| 95 | } | 95 | } | ||
| 96 | - | if _, err := os.Stat(filepath.Join("/tmp/.X11-unix", "X"+displayNum)); err == nil { | 96 | + | x11SocketCandidates := []string{filepath.Join(os.TempDir(), ".X11-unix", "X"+displayNum)} |
| 97 | - | return true | 97 | + | if fallback := "/tmp/.X11-unix/X" + displayNum; fallback != x11SocketCandidates[0] { |
| 98 | + | x11SocketCandidates = append(x11SocketCandidates, fallback) | |||
| 99 | + | } | |||
| 100 | + | for _, candidate := range x11SocketCandidates { | |||
| 101 | + | if _, err := os.Stat(candidate); err == nil { | |||
| 102 | + | return true | |||
| 103 | + | } | |||
| 98 | } | 104 | } | ||
| 99 | return false | 105 | return false | ||
| 100 | } | 106 | } | ||
| @@ -891,15 +897,21 @@ func TestApp_MoveActivePaneToTabFromBrowserWindowActivatesTargetOwnerForCrossWin | |||||
| 891 | t.Fatalf("moveActivePaneToTabFromBrowserWindow returned error: %v", err) | 897 | t.Fatalf("moveActivePaneToTabFromBrowserWindow returned error: %v", err) | ||
| 892 | } | 898 | } | ||
| 893 | 899 | ||||
| 894 | - | if tabs.Find(tab1.ID) == nil { | 900 | + | if tabs.Find(tab1.ID) != nil { |
| 895 | - | t.Fatalf("tab-1 should remain when target owner is activated first") | 901 | + | t.Fatalf("tab-1 should be removed after its active pane moves away") |
| 896 | } | 902 | } | ||
| 897 | - | if tabs.Find(tab2.ID) != nil { | 903 | + | if tabs.Find(tab2.ID) == nil { |
| 898 | - | t.Fatalf("tab-2 should have been moved from the target owner window") | 904 | + | t.Fatalf("tab-2 should remain in the target owner window") |
| 899 | } | 905 | } | ||
| 900 | if tabs.Find(tab3.ID) == nil { | 906 | if tabs.Find(tab3.ID) == nil { | ||
| 901 | t.Fatalf("tab-3 should remain as the move target") | 907 | t.Fatalf("tab-3 should remain as the move target") | ||
| 902 | } | 908 | } | ||
| 909 | + | if app.tabs.ActiveTabID != tab3.ID { | |||
| 910 | + | t.Fatalf("active tab = %q, want %q", app.tabs.ActiveTabID, tab3.ID) | |||
| 911 | + | } | |||
| 912 | + | if app.lastFocusedWindowID != second.id { | |||
| 913 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", app.lastFocusedWindowID, second.id) | |||
| 914 | + | } | |||
| 903 | } | 915 | } | ||
| 904 | 916 | ||||
| 905 | func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) { | 917 | func TestApp_InitKeyboardHandlerDoesNotReattachExistingWindowInput(t *testing.T) { | ||
M internal/ui/component/omnibox.go +14 -2
| @@ -755,12 +755,17 @@ func (o *Omnibox) initList() error { | |||
| 755 | 755 | o.mu.RLock() | |
| 756 | 756 | mode := o.viewMode | |
| 757 | 757 | bangMode := o.bangMode | |
| 758 | + | bangSuggestions := o.bangSuggestions | |
| 758 | 759 | suggestions := o.suggestions | |
| 759 | 760 | favorites := o.favorites | |
| 760 | 761 | o.mu.RUnlock() | |
| 761 | 762 | ||
| 762 | 763 | // Don't navigate in bang mode - activating a bang should fill it in | |
| 763 | 764 | if bangMode { | |
| 765 | + | if bangText, ok := bangSuggestionTextAt(idx, bangSuggestions); ok && o.entry != nil { | |
| 766 | + | o.entry.SetText(bangText) | |
| 767 | + | o.entry.SetPosition(-1) | |
| 768 | + | } | |
| 764 | 769 | return | |
| 765 | 770 | } | |
| 766 | 771 | ||
| @@ -1458,6 +1463,13 @@ func selectedTargetURL(mode ViewMode, idx, maxVisible int, suggestions []Suggest | |||
| 1458 | 1463 | return resolveTargetURLForSelection(mode, idx, maxVisible, suggestions, favorites), true | |
| 1459 | 1464 | } | |
| 1460 | 1465 | ||
| 1466 | + | func bangSuggestionTextAt(idx int, bangSuggestions []BangSuggestion) (string, bool) { | |
| 1467 | + | if idx < 0 || idx >= len(bangSuggestions) { | |
| 1468 | + | return "", false | |
| 1469 | + | } | |
| 1470 | + | return "!" + bangSuggestions[idx].Key + " ", true | |
| 1471 | + | } | |
| 1472 | + | ||
| 1461 | 1473 | func visibleResultCount(total, maxVisible int) int { | |
| 1462 | 1474 | if total <= 0 { | |
| 1463 | 1475 | return 0 | |
| @@ -2293,8 +2305,8 @@ func (o *Omnibox) navigateToSelected() { | |||
| 2293 | 2305 | } | |
| 2294 | 2306 | ||
| 2295 | 2307 | // Otherwise, Enter autocompletes the selected bang. | |
| 2296 | - | if idx >= 0 && idx < len(bangSuggestions) { | |
| 2297 | - | o.entry.SetText("!" + bangSuggestions[idx].Key + " ") | |
| 2308 | + | if bangText, ok := bangSuggestionTextAt(idx, bangSuggestions); ok { | |
| 2309 | + | o.entry.SetText(bangText) | |
| 2298 | 2310 | o.entry.SetPosition(-1) | |
| 2299 | 2311 | return | |
| 2300 | 2312 | } | |
| @@ -755,12 +755,17 @@ func (o *Omnibox) initList() error { | |||||
| 755 | o.mu.RLock() | 755 | o.mu.RLock() | ||
| 756 | mode := o.viewMode | 756 | mode := o.viewMode | ||
| 757 | bangMode := o.bangMode | 757 | bangMode := o.bangMode | ||
| 758 | + | bangSuggestions := o.bangSuggestions | |||
| 758 | suggestions := o.suggestions | 759 | suggestions := o.suggestions | ||
| 759 | favorites := o.favorites | 760 | favorites := o.favorites | ||
| 760 | o.mu.RUnlock() | 761 | o.mu.RUnlock() | ||
| 761 | 762 | ||||
| 762 | // Don't navigate in bang mode - activating a bang should fill it in | 763 | // Don't navigate in bang mode - activating a bang should fill it in | ||
| 763 | if bangMode { | 764 | if bangMode { | ||
| 765 | + | if bangText, ok := bangSuggestionTextAt(idx, bangSuggestions); ok && o.entry != nil { | |||
| 766 | + | o.entry.SetText(bangText) | |||
| 767 | + | o.entry.SetPosition(-1) | |||
| 768 | + | } | |||
| 764 | return | 769 | return | ||
| 765 | } | 770 | } | ||
| 766 | 771 | ||||
| @@ -1458,6 +1463,13 @@ func selectedTargetURL(mode ViewMode, idx, maxVisible int, suggestions []Suggest | |||||
| 1458 | return resolveTargetURLForSelection(mode, idx, maxVisible, suggestions, favorites), true | 1463 | return resolveTargetURLForSelection(mode, idx, maxVisible, suggestions, favorites), true | ||
| 1459 | } | 1464 | } | ||
| 1460 | 1465 | ||||
| 1466 | + | func bangSuggestionTextAt(idx int, bangSuggestions []BangSuggestion) (string, bool) { | |||
| 1467 | + | if idx < 0 || idx >= len(bangSuggestions) { | |||
| 1468 | + | return "", false | |||
| 1469 | + | } | |||
| 1470 | + | return "!" + bangSuggestions[idx].Key + " ", true | |||
| 1471 | + | } | |||
| 1472 | + | ||||
| 1461 | func visibleResultCount(total, maxVisible int) int { | 1473 | func visibleResultCount(total, maxVisible int) int { | ||
| 1462 | if total <= 0 { | 1474 | if total <= 0 { | ||
| 1463 | return 0 | 1475 | return 0 | ||
| @@ -2293,8 +2305,8 @@ func (o *Omnibox) navigateToSelected() { | |||||
| 2293 | } | 2305 | } | ||
| 2294 | 2306 | ||||
| 2295 | // Otherwise, Enter autocompletes the selected bang. | 2307 | // Otherwise, Enter autocompletes the selected bang. | ||
| 2296 | - | if idx >= 0 && idx < len(bangSuggestions) { | 2308 | + | if bangText, ok := bangSuggestionTextAt(idx, bangSuggestions); ok { |
| 2297 | - | o.entry.SetText("!" + bangSuggestions[idx].Key + " ") | 2309 | + | o.entry.SetText(bangText) |
| 2298 | o.entry.SetPosition(-1) | 2310 | o.entry.SetPosition(-1) | ||
| 2299 | return | 2311 | return | ||
| 2300 | } | 2312 | } | ||
M internal/ui/component/omnibox_navigation_state_test.go +11 -0
| @@ -199,6 +199,17 @@ func TestSelectedTargetURL(t *testing.T) { | |||
| 199 | 199 | } | |
| 200 | 200 | } | |
| 201 | 201 | ||
| 202 | + | func TestBangSuggestionTextAt(t *testing.T) { | |
| 203 | + | bangSuggestions := []BangSuggestion{{Key: "g"}, {Key: "gh"}} | |
| 204 | + | ||
| 205 | + | if got, ok := bangSuggestionTextAt(1, bangSuggestions); !ok || got != "!gh " { | |
| 206 | + | t.Fatalf("bangSuggestionTextAt(1) = (%q, %v), want (%q, %v)", got, ok, "!gh ", true) | |
| 207 | + | } | |
| 208 | + | if got, ok := bangSuggestionTextAt(99, bangSuggestions); ok || got != "" { | |
| 209 | + | t.Fatalf("bangSuggestionTextAt(out of range) = (%q, %v), want (%q, %v)", got, ok, "", false) | |
| 210 | + | } | |
| 211 | + | } | |
| 212 | + | ||
| 202 | 213 | func TestVisibleGhostSuggestion(t *testing.T) { | |
| 203 | 214 | suggestions := []Suggestion{ | |
| 204 | 215 | {URL: "https://github.com/bnema/dumber"}, | |
| @@ -199,6 +199,17 @@ func TestSelectedTargetURL(t *testing.T) { | |||||
| 199 | } | 199 | } | ||
| 200 | } | 200 | } | ||
| 201 | 201 | ||||
| 202 | + | func TestBangSuggestionTextAt(t *testing.T) { | |||
| 203 | + | bangSuggestions := []BangSuggestion{{Key: "g"}, {Key: "gh"}} | |||
| 204 | + | ||||
| 205 | + | if got, ok := bangSuggestionTextAt(1, bangSuggestions); !ok || got != "!gh " { | |||
| 206 | + | t.Fatalf("bangSuggestionTextAt(1) = (%q, %v), want (%q, %v)", got, ok, "!gh ", true) | |||
| 207 | + | } | |||
| 208 | + | if got, ok := bangSuggestionTextAt(99, bangSuggestions); ok || got != "" { | |||
| 209 | + | t.Fatalf("bangSuggestionTextAt(out of range) = (%q, %v), want (%q, %v)", got, ok, "", false) | |||
| 210 | + | } | |||
| 211 | + | } | |||
| 212 | + | ||||
| 202 | func TestVisibleGhostSuggestion(t *testing.T) { | 213 | func TestVisibleGhostSuggestion(t *testing.T) { | ||
| 203 | suggestions := []Suggestion{ | 214 | suggestions := []Suggestion{ | ||
| 204 | {URL: "https://github.com/bnema/dumber"}, | 215 | {URL: "https://github.com/bnema/dumber"}, | ||