fix(omnibox): always open standalone links in new window
- commit
- 76e12e8aa7108af04b2ee419978c5dbff0ef0434
- author
- brice · · 2026-08-13 17:01 UTC
- parent
- 1ee1a70
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
12 files · +384 -36
M cmd/dumber/main.go +28 -3
| @@ -96,11 +96,28 @@ func launchModeFromArgs(args []string) (launchMode, string) { | |||
| 96 | 96 | } | |
| 97 | 97 | ||
| 98 | 98 | func tryForwardBrowseURLToRunningInstance(ctx context.Context, relay port.BrowserLaunchRelay, browseURL string) (bool, error) { | |
| 99 | + | return tryForwardBrowserURLToRunningInstance(ctx, relay, browseURL, func(ctx context.Context, url string) (bool, error) { | |
| 100 | + | return relay.DeliverOpenExternalURL(ctx, url) | |
| 101 | + | }) | |
| 102 | + | } | |
| 103 | + | ||
| 104 | + | func tryForwardFreshWindowURLToRunningInstance(ctx context.Context, relay port.BrowserLaunchRelay, browseURL string) (bool, error) { | |
| 105 | + | return tryForwardBrowserURLToRunningInstance(ctx, relay, browseURL, func(ctx context.Context, url string) (bool, error) { | |
| 106 | + | return relay.DeliverOpenFreshWindow(ctx, url) | |
| 107 | + | }) | |
| 108 | + | } | |
| 109 | + | ||
| 110 | + | func tryForwardBrowserURLToRunningInstance( | |
| 111 | + | ctx context.Context, | |
| 112 | + | relay port.BrowserLaunchRelay, | |
| 113 | + | browseURL string, | |
| 114 | + | deliver func(context.Context, string) (bool, error), | |
| 115 | + | ) (bool, error) { | |
| 99 | 116 | if relay == nil { | |
| 100 | 117 | return false, nil | |
| 101 | 118 | } | |
| 102 | 119 | ||
| 103 | - | delivered, err := relay.DeliverOpenExternalURL(ctx, browseURL) | |
| 120 | + | delivered, err := deliver(ctx, browseURL) | |
| 104 | 121 | if err != nil { | |
| 105 | 122 | if delivered && errors.Is(err, desktop.ErrBrowserLaunchRelayUnconfirmed) { | |
| 106 | 123 | return true, nil | |
| @@ -193,7 +210,15 @@ func main() { | |||
| 193 | 210 | timing.configComplete = time.Now() | |
| 194 | 211 | configureBrowserLaunchRelay(cfg) | |
| 195 | 212 | startupURL := domainurl.ResolveBrowserStartupURL(browseURL) | |
| 196 | - | if forwarded, err := tryForwardBrowseURLToRunningInstance(context.Background(), browserLaunchRelay, startupURL); err != nil { | |
| 213 | + | freshWindow := os.Getenv(desktop.FreshWindowLaunchEnvVar) == "1" | |
| 214 | + | if freshWindow { | |
| 215 | + | _ = os.Unsetenv(desktop.FreshWindowLaunchEnvVar) | |
| 216 | + | } | |
| 217 | + | forward := tryForwardBrowseURLToRunningInstance | |
| 218 | + | if freshWindow { | |
| 219 | + | forward = tryForwardFreshWindowURLToRunningInstance | |
| 220 | + | } | |
| 221 | + | if forwarded, err := forward(context.Background(), browserLaunchRelay, startupURL); err != nil { | |
| 197 | 222 | fmt.Fprintf( | |
| 198 | 223 | os.Stderr, | |
| 199 | 224 | "warning: failed to forward browse URL %q to a running instance, falling back to a new process: %v\n", | |
| @@ -961,7 +986,7 @@ func buildUIDependencies( | |||
| 961 | 986 | }, | |
| 962 | 987 | LaunchExternalURL: desktop.LaunchExternalURL, | |
| 963 | 988 | LaunchBrowserURL: func(navCtx context.Context, uri string) error { | |
| 964 | - | return launchStandaloneBrowserURL(navCtx, browserLauncher.LaunchURL, uri) | |
| 989 | + | return launchStandaloneBrowserURL(navCtx, browserLauncher.LaunchFreshWindowURL, uri) | |
| 965 | 990 | }, | |
| 966 | 991 | BrowserLaunchRelay: browserLaunchRelay, | |
| 967 | 992 | MigrationChecker: config.NewMigrator(), | |
| @@ -96,11 +96,28 @@ func launchModeFromArgs(args []string) (launchMode, string) { | |||||
| 96 | } | 96 | } | ||
| 97 | 97 | ||||
| 98 | func tryForwardBrowseURLToRunningInstance(ctx context.Context, relay port.BrowserLaunchRelay, browseURL string) (bool, error) { | 98 | func tryForwardBrowseURLToRunningInstance(ctx context.Context, relay port.BrowserLaunchRelay, browseURL string) (bool, error) { | ||
| 99 | + | return tryForwardBrowserURLToRunningInstance(ctx, relay, browseURL, func(ctx context.Context, url string) (bool, error) { | |||
| 100 | + | return relay.DeliverOpenExternalURL(ctx, url) | |||
| 101 | + | }) | |||
| 102 | + | } | |||
| 103 | + | ||||
| 104 | + | func tryForwardFreshWindowURLToRunningInstance(ctx context.Context, relay port.BrowserLaunchRelay, browseURL string) (bool, error) { | |||
| 105 | + | return tryForwardBrowserURLToRunningInstance(ctx, relay, browseURL, func(ctx context.Context, url string) (bool, error) { | |||
| 106 | + | return relay.DeliverOpenFreshWindow(ctx, url) | |||
| 107 | + | }) | |||
| 108 | + | } | |||
| 109 | + | ||||
| 110 | + | func tryForwardBrowserURLToRunningInstance( | |||
| 111 | + | ctx context.Context, | |||
| 112 | + | relay port.BrowserLaunchRelay, | |||
| 113 | + | browseURL string, | |||
| 114 | + | deliver func(context.Context, string) (bool, error), | |||
| 115 | + | ) (bool, error) { | |||
| 99 | if relay == nil { | 116 | if relay == nil { | ||
| 100 | return false, nil | 117 | return false, nil | ||
| 101 | } | 118 | } | ||
| 102 | 119 | ||||
| 103 | - | delivered, err := relay.DeliverOpenExternalURL(ctx, browseURL) | 120 | + | delivered, err := deliver(ctx, browseURL) |
| 104 | if err != nil { | 121 | if err != nil { | ||
| 105 | if delivered && errors.Is(err, desktop.ErrBrowserLaunchRelayUnconfirmed) { | 122 | if delivered && errors.Is(err, desktop.ErrBrowserLaunchRelayUnconfirmed) { | ||
| 106 | return true, nil | 123 | return true, nil | ||
| @@ -193,7 +210,15 @@ func main() { | |||||
| 193 | timing.configComplete = time.Now() | 210 | timing.configComplete = time.Now() | ||
| 194 | configureBrowserLaunchRelay(cfg) | 211 | configureBrowserLaunchRelay(cfg) | ||
| 195 | startupURL := domainurl.ResolveBrowserStartupURL(browseURL) | 212 | startupURL := domainurl.ResolveBrowserStartupURL(browseURL) | ||
| 196 | - | if forwarded, err := tryForwardBrowseURLToRunningInstance(context.Background(), browserLaunchRelay, startupURL); err != nil { | 213 | + | freshWindow := os.Getenv(desktop.FreshWindowLaunchEnvVar) == "1" |
| 214 | + | if freshWindow { | |||
| 215 | + | _ = os.Unsetenv(desktop.FreshWindowLaunchEnvVar) | |||
| 216 | + | } | |||
| 217 | + | forward := tryForwardBrowseURLToRunningInstance | |||
| 218 | + | if freshWindow { | |||
| 219 | + | forward = tryForwardFreshWindowURLToRunningInstance | |||
| 220 | + | } | |||
| 221 | + | if forwarded, err := forward(context.Background(), browserLaunchRelay, startupURL); err != nil { | |||
| 197 | fmt.Fprintf( | 222 | fmt.Fprintf( | ||
| 198 | os.Stderr, | 223 | os.Stderr, | ||
| 199 | "warning: failed to forward browse URL %q to a running instance, falling back to a new process: %v\n", | 224 | "warning: failed to forward browse URL %q to a running instance, falling back to a new process: %v\n", | ||
| @@ -961,7 +986,7 @@ func buildUIDependencies( | |||||
| 961 | }, | 986 | }, | ||
| 962 | LaunchExternalURL: desktop.LaunchExternalURL, | 987 | LaunchExternalURL: desktop.LaunchExternalURL, | ||
| 963 | LaunchBrowserURL: func(navCtx context.Context, uri string) error { | 988 | LaunchBrowserURL: func(navCtx context.Context, uri string) error { | ||
| 964 | - | return launchStandaloneBrowserURL(navCtx, browserLauncher.LaunchURL, uri) | 989 | + | return launchStandaloneBrowserURL(navCtx, browserLauncher.LaunchFreshWindowURL, uri) |
| 965 | }, | 990 | }, | ||
| 966 | BrowserLaunchRelay: browserLaunchRelay, | 991 | BrowserLaunchRelay: browserLaunchRelay, | ||
| 967 | MigrationChecker: config.NewMigrator(), | 992 | MigrationChecker: config.NewMigrator(), | ||
M cmd/dumber/main_test.go +14 -0
| @@ -256,6 +256,20 @@ func TestTryForwardBrowseURLToRunningInstance_ReturnsTrueOnRelayHit(t *testing.T | |||
| 256 | 256 | } | |
| 257 | 257 | } | |
| 258 | 258 | ||
| 259 | + | func TestTryForwardFreshWindowURLToRunningInstance_UsesFreshWindowRelayAction(t *testing.T) { | |
| 260 | + | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 261 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, nil) | |
| 262 | + | ||
| 263 | + | forwarded, err := tryForwardFreshWindowURLToRunningInstance(context.Background(), relay, "https://example.com") | |
| 264 | + | ||
| 265 | + | if err != nil { | |
| 266 | + | t.Fatalf("expected no error, got %v", err) | |
| 267 | + | } | |
| 268 | + | if !forwarded { | |
| 269 | + | t.Fatal("expected fresh-window relay delivery to be forwarded") | |
| 270 | + | } | |
| 271 | + | } | |
| 272 | + | ||
| 259 | 273 | func TestTryForwardBrowseURLToRunningInstance_ReturnsFalseOnRelayMiss(t *testing.T) { | |
| 260 | 274 | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 261 | 275 | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, nil) | |
| @@ -256,6 +256,20 @@ func TestTryForwardBrowseURLToRunningInstance_ReturnsTrueOnRelayHit(t *testing.T | |||||
| 256 | } | 256 | } | ||
| 257 | } | 257 | } | ||
| 258 | 258 | ||||
| 259 | + | func TestTryForwardFreshWindowURLToRunningInstance_UsesFreshWindowRelayAction(t *testing.T) { | |||
| 260 | + | relay := mocks.NewMockBrowserLaunchRelay(t) | |||
| 261 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, nil) | |||
| 262 | + | ||||
| 263 | + | forwarded, err := tryForwardFreshWindowURLToRunningInstance(context.Background(), relay, "https://example.com") | |||
| 264 | + | ||||
| 265 | + | if err != nil { | |||
| 266 | + | t.Fatalf("expected no error, got %v", err) | |||
| 267 | + | } | |||
| 268 | + | if !forwarded { | |||
| 269 | + | t.Fatal("expected fresh-window relay delivery to be forwarded") | |||
| 270 | + | } | |||
| 271 | + | } | |||
| 272 | + | ||||
| 259 | func TestTryForwardBrowseURLToRunningInstance_ReturnsFalseOnRelayMiss(t *testing.T) { | 273 | func TestTryForwardBrowseURLToRunningInstance_ReturnsFalseOnRelayMiss(t *testing.T) { | ||
| 260 | relay := mocks.NewMockBrowserLaunchRelay(t) | 274 | relay := mocks.NewMockBrowserLaunchRelay(t) | ||
| 261 | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, nil) | 275 | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, nil) | ||
M internal/application/port/browser_launch.go +6 -2
| @@ -5,12 +5,13 @@ import ( | |||
| 5 | 5 | "io" | |
| 6 | 6 | ) | |
| 7 | 7 | ||
| 8 | - | // BrowserWindowOpener opens an external URL. | |
| 8 | + | // BrowserWindowOpener opens browser URLs received through the launch relay. | |
| 9 | 9 | type BrowserWindowOpener interface { | |
| 10 | 10 | OpenExternalURL(ctx context.Context, url string) error | |
| 11 | + | OpenFreshWindow(ctx context.Context, url string) error | |
| 11 | 12 | } | |
| 12 | 13 | ||
| 13 | - | // BrowserLaunchRelay delivers external URL launch requests. | |
| 14 | + | // BrowserLaunchRelay delivers browser URL launch requests. | |
| 14 | 15 | type BrowserLaunchRelay interface { | |
| 15 | 16 | // DeliverOpenExternalURL attempts to deliver a request to open an external URL. | |
| 16 | 17 | // The bool reports whether the relay accepted the request; false means the | |
| @@ -18,6 +19,9 @@ type BrowserLaunchRelay interface { | |||
| 18 | 19 | // An error may still be returned with delivered=true when the relay accepted | |
| 19 | 20 | // the request but could not confirm completion before the caller timed out. | |
| 20 | 21 | DeliverOpenExternalURL(ctx context.Context, url string) (bool, error) | |
| 22 | + | // DeliverOpenFreshWindow attempts to deliver a request that must open in a | |
| 23 | + | // new browser window, independently of external-link workspace settings. | |
| 24 | + | DeliverOpenFreshWindow(ctx context.Context, url string) (bool, error) | |
| 21 | 25 | Listen(ctx context.Context, opener BrowserWindowOpener) (io.Closer, error) | |
| 22 | 26 | } | |
| 23 | 27 | ||
| @@ -5,12 +5,13 @@ import ( | |||||
| 5 | "io" | 5 | "io" | ||
| 6 | ) | 6 | ) | ||
| 7 | 7 | ||||
| 8 | - | // BrowserWindowOpener opens an external URL. | 8 | + | // BrowserWindowOpener opens browser URLs received through the launch relay. |
| 9 | type BrowserWindowOpener interface { | 9 | type BrowserWindowOpener interface { | ||
| 10 | OpenExternalURL(ctx context.Context, url string) error | 10 | OpenExternalURL(ctx context.Context, url string) error | ||
| 11 | + | OpenFreshWindow(ctx context.Context, url string) error | |||
| 11 | } | 12 | } | ||
| 12 | 13 | ||||
| 13 | - | // BrowserLaunchRelay delivers external URL launch requests. | 14 | + | // BrowserLaunchRelay delivers browser URL launch requests. |
| 14 | type BrowserLaunchRelay interface { | 15 | type BrowserLaunchRelay interface { | ||
| 15 | // DeliverOpenExternalURL attempts to deliver a request to open an external URL. | 16 | // DeliverOpenExternalURL attempts to deliver a request to open an external URL. | ||
| 16 | // The bool reports whether the relay accepted the request; false means the | 17 | // The bool reports whether the relay accepted the request; false means the | ||
| @@ -18,6 +19,9 @@ type BrowserLaunchRelay interface { | |||||
| 18 | // An error may still be returned with delivered=true when the relay accepted | 19 | // An error may still be returned with delivered=true when the relay accepted | ||
| 19 | // the request but could not confirm completion before the caller timed out. | 20 | // the request but could not confirm completion before the caller timed out. | ||
| 20 | DeliverOpenExternalURL(ctx context.Context, url string) (bool, error) | 21 | DeliverOpenExternalURL(ctx context.Context, url string) (bool, error) | ||
| 22 | + | // DeliverOpenFreshWindow attempts to deliver a request that must open in a | |||
| 23 | + | // new browser window, independently of external-link workspace settings. | |||
| 24 | + | DeliverOpenFreshWindow(ctx context.Context, url string) (bool, error) | |||
| 21 | Listen(ctx context.Context, opener BrowserWindowOpener) (io.Closer, error) | 25 | Listen(ctx context.Context, opener BrowserWindowOpener) (io.Closer, error) | ||
| 22 | } | 26 | } | ||
| 23 | 27 | ||||
M internal/application/port/mocks/browser_launch.go +123 -0
| @@ -96,6 +96,63 @@ func (_c *MockBrowserWindowOpener_OpenExternalURL_Call) RunAndReturn(run func(ct | |||
| 96 | 96 | return _c | |
| 97 | 97 | } | |
| 98 | 98 | ||
| 99 | + | // OpenFreshWindow provides a mock function for the type MockBrowserWindowOpener | |
| 100 | + | func (_mock *MockBrowserWindowOpener) OpenFreshWindow(ctx context.Context, url string) error { | |
| 101 | + | ret := _mock.Called(ctx, url) | |
| 102 | + | ||
| 103 | + | if len(ret) == 0 { | |
| 104 | + | panic("no return value specified for OpenFreshWindow") | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | var r0 error | |
| 108 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok { | |
| 109 | + | r0 = returnFunc(ctx, url) | |
| 110 | + | } else { | |
| 111 | + | r0 = ret.Error(0) | |
| 112 | + | } | |
| 113 | + | return r0 | |
| 114 | + | } | |
| 115 | + | ||
| 116 | + | // MockBrowserWindowOpener_OpenFreshWindow_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OpenFreshWindow' | |
| 117 | + | type MockBrowserWindowOpener_OpenFreshWindow_Call struct { | |
| 118 | + | *mock.Call | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | // OpenFreshWindow is a helper method to define mock.On call | |
| 122 | + | // - ctx context.Context | |
| 123 | + | // - url string | |
| 124 | + | func (_e *MockBrowserWindowOpener_Expecter) OpenFreshWindow(ctx any, url any) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |
| 125 | + | return &MockBrowserWindowOpener_OpenFreshWindow_Call{Call: _e.mock.On("OpenFreshWindow", ctx, url)} | |
| 126 | + | } | |
| 127 | + | ||
| 128 | + | func (_c *MockBrowserWindowOpener_OpenFreshWindow_Call) Run(run func(ctx context.Context, url string)) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |
| 129 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 130 | + | var arg0 context.Context | |
| 131 | + | if args[0] != nil { | |
| 132 | + | arg0 = args[0].(context.Context) | |
| 133 | + | } | |
| 134 | + | var arg1 string | |
| 135 | + | if args[1] != nil { | |
| 136 | + | arg1 = args[1].(string) | |
| 137 | + | } | |
| 138 | + | run( | |
| 139 | + | arg0, | |
| 140 | + | arg1, | |
| 141 | + | ) | |
| 142 | + | }) | |
| 143 | + | return _c | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | func (_c *MockBrowserWindowOpener_OpenFreshWindow_Call) Return(err error) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |
| 147 | + | _c.Call.Return(err) | |
| 148 | + | return _c | |
| 149 | + | } | |
| 150 | + | ||
| 151 | + | func (_c *MockBrowserWindowOpener_OpenFreshWindow_Call) RunAndReturn(run func(ctx context.Context, url string) error) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |
| 152 | + | _c.Call.Return(run) | |
| 153 | + | return _c | |
| 154 | + | } | |
| 155 | + | ||
| 99 | 156 | // NewMockBrowserLaunchRelay creates a new instance of MockBrowserLaunchRelay. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 100 | 157 | // The first argument is typically a *testing.T value. | |
| 101 | 158 | func NewMockBrowserLaunchRelay(t interface { | |
| @@ -189,6 +246,72 @@ func (_c *MockBrowserLaunchRelay_DeliverOpenExternalURL_Call) RunAndReturn(run f | |||
| 189 | 246 | return _c | |
| 190 | 247 | } | |
| 191 | 248 | ||
| 249 | + | // DeliverOpenFreshWindow provides a mock function for the type MockBrowserLaunchRelay | |
| 250 | + | func (_mock *MockBrowserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url string) (bool, error) { | |
| 251 | + | ret := _mock.Called(ctx, url) | |
| 252 | + | ||
| 253 | + | if len(ret) == 0 { | |
| 254 | + | panic("no return value specified for DeliverOpenFreshWindow") | |
| 255 | + | } | |
| 256 | + | ||
| 257 | + | var r0 bool | |
| 258 | + | var r1 error | |
| 259 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { | |
| 260 | + | return returnFunc(ctx, url) | |
| 261 | + | } | |
| 262 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) bool); ok { | |
| 263 | + | r0 = returnFunc(ctx, url) | |
| 264 | + | } else { | |
| 265 | + | r0 = ret.Get(0).(bool) | |
| 266 | + | } | |
| 267 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { | |
| 268 | + | r1 = returnFunc(ctx, url) | |
| 269 | + | } else { | |
| 270 | + | r1 = ret.Error(1) | |
| 271 | + | } | |
| 272 | + | return r0, r1 | |
| 273 | + | } | |
| 274 | + | ||
| 275 | + | // MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeliverOpenFreshWindow' | |
| 276 | + | type MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call struct { | |
| 277 | + | *mock.Call | |
| 278 | + | } | |
| 279 | + | ||
| 280 | + | // DeliverOpenFreshWindow is a helper method to define mock.On call | |
| 281 | + | // - ctx context.Context | |
| 282 | + | // - url string | |
| 283 | + | func (_e *MockBrowserLaunchRelay_Expecter) DeliverOpenFreshWindow(ctx any, url any) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |
| 284 | + | return &MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call{Call: _e.mock.On("DeliverOpenFreshWindow", ctx, url)} | |
| 285 | + | } | |
| 286 | + | ||
| 287 | + | func (_c *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call) Run(run func(ctx context.Context, url string)) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |
| 288 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 289 | + | var arg0 context.Context | |
| 290 | + | if args[0] != nil { | |
| 291 | + | arg0 = args[0].(context.Context) | |
| 292 | + | } | |
| 293 | + | var arg1 string | |
| 294 | + | if args[1] != nil { | |
| 295 | + | arg1 = args[1].(string) | |
| 296 | + | } | |
| 297 | + | run( | |
| 298 | + | arg0, | |
| 299 | + | arg1, | |
| 300 | + | ) | |
| 301 | + | }) | |
| 302 | + | return _c | |
| 303 | + | } | |
| 304 | + | ||
| 305 | + | func (_c *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call) Return(b bool, err error) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |
| 306 | + | _c.Call.Return(b, err) | |
| 307 | + | return _c | |
| 308 | + | } | |
| 309 | + | ||
| 310 | + | func (_c *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call) RunAndReturn(run func(ctx context.Context, url string) (bool, error)) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |
| 311 | + | _c.Call.Return(run) | |
| 312 | + | return _c | |
| 313 | + | } | |
| 314 | + | ||
| 192 | 315 | // Listen provides a mock function for the type MockBrowserLaunchRelay | |
| 193 | 316 | func (_mock *MockBrowserLaunchRelay) Listen(ctx context.Context, opener port.BrowserWindowOpener) (io.Closer, error) { | |
| 194 | 317 | ret := _mock.Called(ctx, opener) | |
| @@ -96,6 +96,63 @@ func (_c *MockBrowserWindowOpener_OpenExternalURL_Call) RunAndReturn(run func(ct | |||||
| 96 | return _c | 96 | return _c | ||
| 97 | } | 97 | } | ||
| 98 | 98 | ||||
| 99 | + | // OpenFreshWindow provides a mock function for the type MockBrowserWindowOpener | |||
| 100 | + | func (_mock *MockBrowserWindowOpener) OpenFreshWindow(ctx context.Context, url string) error { | |||
| 101 | + | ret := _mock.Called(ctx, url) | |||
| 102 | + | ||||
| 103 | + | if len(ret) == 0 { | |||
| 104 | + | panic("no return value specified for OpenFreshWindow") | |||
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | var r0 error | |||
| 108 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok { | |||
| 109 | + | r0 = returnFunc(ctx, url) | |||
| 110 | + | } else { | |||
| 111 | + | r0 = ret.Error(0) | |||
| 112 | + | } | |||
| 113 | + | return r0 | |||
| 114 | + | } | |||
| 115 | + | ||||
| 116 | + | // MockBrowserWindowOpener_OpenFreshWindow_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OpenFreshWindow' | |||
| 117 | + | type MockBrowserWindowOpener_OpenFreshWindow_Call struct { | |||
| 118 | + | *mock.Call | |||
| 119 | + | } | |||
| 120 | + | ||||
| 121 | + | // OpenFreshWindow is a helper method to define mock.On call | |||
| 122 | + | // - ctx context.Context | |||
| 123 | + | // - url string | |||
| 124 | + | func (_e *MockBrowserWindowOpener_Expecter) OpenFreshWindow(ctx any, url any) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |||
| 125 | + | return &MockBrowserWindowOpener_OpenFreshWindow_Call{Call: _e.mock.On("OpenFreshWindow", ctx, url)} | |||
| 126 | + | } | |||
| 127 | + | ||||
| 128 | + | func (_c *MockBrowserWindowOpener_OpenFreshWindow_Call) Run(run func(ctx context.Context, url string)) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |||
| 129 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 130 | + | var arg0 context.Context | |||
| 131 | + | if args[0] != nil { | |||
| 132 | + | arg0 = args[0].(context.Context) | |||
| 133 | + | } | |||
| 134 | + | var arg1 string | |||
| 135 | + | if args[1] != nil { | |||
| 136 | + | arg1 = args[1].(string) | |||
| 137 | + | } | |||
| 138 | + | run( | |||
| 139 | + | arg0, | |||
| 140 | + | arg1, | |||
| 141 | + | ) | |||
| 142 | + | }) | |||
| 143 | + | return _c | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | func (_c *MockBrowserWindowOpener_OpenFreshWindow_Call) Return(err error) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |||
| 147 | + | _c.Call.Return(err) | |||
| 148 | + | return _c | |||
| 149 | + | } | |||
| 150 | + | ||||
| 151 | + | func (_c *MockBrowserWindowOpener_OpenFreshWindow_Call) RunAndReturn(run func(ctx context.Context, url string) error) *MockBrowserWindowOpener_OpenFreshWindow_Call { | |||
| 152 | + | _c.Call.Return(run) | |||
| 153 | + | return _c | |||
| 154 | + | } | |||
| 155 | + | ||||
| 99 | // NewMockBrowserLaunchRelay creates a new instance of MockBrowserLaunchRelay. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | 156 | // NewMockBrowserLaunchRelay creates a new instance of MockBrowserLaunchRelay. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | ||
| 100 | // The first argument is typically a *testing.T value. | 157 | // The first argument is typically a *testing.T value. | ||
| 101 | func NewMockBrowserLaunchRelay(t interface { | 158 | func NewMockBrowserLaunchRelay(t interface { | ||
| @@ -189,6 +246,72 @@ func (_c *MockBrowserLaunchRelay_DeliverOpenExternalURL_Call) RunAndReturn(run f | |||||
| 189 | return _c | 246 | return _c | ||
| 190 | } | 247 | } | ||
| 191 | 248 | ||||
| 249 | + | // DeliverOpenFreshWindow provides a mock function for the type MockBrowserLaunchRelay | |||
| 250 | + | func (_mock *MockBrowserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url string) (bool, error) { | |||
| 251 | + | ret := _mock.Called(ctx, url) | |||
| 252 | + | ||||
| 253 | + | if len(ret) == 0 { | |||
| 254 | + | panic("no return value specified for DeliverOpenFreshWindow") | |||
| 255 | + | } | |||
| 256 | + | ||||
| 257 | + | var r0 bool | |||
| 258 | + | var r1 error | |||
| 259 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { | |||
| 260 | + | return returnFunc(ctx, url) | |||
| 261 | + | } | |||
| 262 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) bool); ok { | |||
| 263 | + | r0 = returnFunc(ctx, url) | |||
| 264 | + | } else { | |||
| 265 | + | r0 = ret.Get(0).(bool) | |||
| 266 | + | } | |||
| 267 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { | |||
| 268 | + | r1 = returnFunc(ctx, url) | |||
| 269 | + | } else { | |||
| 270 | + | r1 = ret.Error(1) | |||
| 271 | + | } | |||
| 272 | + | return r0, r1 | |||
| 273 | + | } | |||
| 274 | + | ||||
| 275 | + | // MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeliverOpenFreshWindow' | |||
| 276 | + | type MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call struct { | |||
| 277 | + | *mock.Call | |||
| 278 | + | } | |||
| 279 | + | ||||
| 280 | + | // DeliverOpenFreshWindow is a helper method to define mock.On call | |||
| 281 | + | // - ctx context.Context | |||
| 282 | + | // - url string | |||
| 283 | + | func (_e *MockBrowserLaunchRelay_Expecter) DeliverOpenFreshWindow(ctx any, url any) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |||
| 284 | + | return &MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call{Call: _e.mock.On("DeliverOpenFreshWindow", ctx, url)} | |||
| 285 | + | } | |||
| 286 | + | ||||
| 287 | + | func (_c *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call) Run(run func(ctx context.Context, url string)) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |||
| 288 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 289 | + | var arg0 context.Context | |||
| 290 | + | if args[0] != nil { | |||
| 291 | + | arg0 = args[0].(context.Context) | |||
| 292 | + | } | |||
| 293 | + | var arg1 string | |||
| 294 | + | if args[1] != nil { | |||
| 295 | + | arg1 = args[1].(string) | |||
| 296 | + | } | |||
| 297 | + | run( | |||
| 298 | + | arg0, | |||
| 299 | + | arg1, | |||
| 300 | + | ) | |||
| 301 | + | }) | |||
| 302 | + | return _c | |||
| 303 | + | } | |||
| 304 | + | ||||
| 305 | + | func (_c *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call) Return(b bool, err error) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |||
| 306 | + | _c.Call.Return(b, err) | |||
| 307 | + | return _c | |||
| 308 | + | } | |||
| 309 | + | ||||
| 310 | + | func (_c *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call) RunAndReturn(run func(ctx context.Context, url string) (bool, error)) *MockBrowserLaunchRelay_DeliverOpenFreshWindow_Call { | |||
| 311 | + | _c.Call.Return(run) | |||
| 312 | + | return _c | |||
| 313 | + | } | |||
| 314 | + | ||||
| 192 | // Listen provides a mock function for the type MockBrowserLaunchRelay | 315 | // Listen provides a mock function for the type MockBrowserLaunchRelay | ||
| 193 | func (_mock *MockBrowserLaunchRelay) Listen(ctx context.Context, opener port.BrowserWindowOpener) (io.Closer, error) { | 316 | func (_mock *MockBrowserLaunchRelay) Listen(ctx context.Context, opener port.BrowserWindowOpener) (io.Closer, error) { | ||
| 194 | ret := _mock.Called(ctx, opener) | 317 | ret := _mock.Called(ctx, opener) | ||
M internal/infrastructure/desktop/adapter.go +25 -1
| @@ -446,6 +446,10 @@ func NewSessionSpawner(ctx context.Context, spawnEnv port.SessionSpawnEnvironmen | |||
| 446 | 446 | // RestoreSessionEnvVar is the environment variable used to pass session ID for restoration. | |
| 447 | 447 | const RestoreSessionEnvVar = "DUMBER_RESTORE_SESSION" | |
| 448 | 448 | ||
| 449 | + | // FreshWindowLaunchEnvVar marks a browse child that must open a fresh window | |
| 450 | + | // instead of applying external-link workspace placement. | |
| 451 | + | const FreshWindowLaunchEnvVar = "DUMBER_BROWSER_FRESH_WINDOW" | |
| 452 | + | ||
| 449 | 453 | const layerShellPreloadToken = "libgtk4" + "-layer-shell" | |
| 450 | 454 | ||
| 451 | 455 | // LaunchExternalURL opens a URL with the system's default handler (xdg-open on Linux). | |
| @@ -466,6 +470,23 @@ func LaunchBrowserURL(uri string) { | |||
| 466 | 470 | } | |
| 467 | 471 | ||
| 468 | 472 | func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, error), spawnDetachedProcess func(*exec.Cmd) error) error { | |
| 473 | + | return launchBrowserURL(uri, false, resolveExecutablePath, spawnDetachedProcess) | |
| 474 | + | } | |
| 475 | + | ||
| 476 | + | func launchBrowserFreshWindowURL( | |
| 477 | + | uri string, | |
| 478 | + | resolveExecutablePath func() (string, error), | |
| 479 | + | spawnDetachedProcess func(*exec.Cmd) error, | |
| 480 | + | ) error { | |
| 481 | + | return launchBrowserURL(uri, true, resolveExecutablePath, spawnDetachedProcess) | |
| 482 | + | } | |
| 483 | + | ||
| 484 | + | func launchBrowserURL( | |
| 485 | + | uri string, | |
| 486 | + | freshWindow bool, | |
| 487 | + | resolveExecutablePath func() (string, error), | |
| 488 | + | spawnDetachedProcess func(*exec.Cmd) error, | |
| 489 | + | ) error { | |
| 469 | 490 | if resolveExecutablePath == nil { | |
| 470 | 491 | resolveExecutablePath = getExecutablePath | |
| 471 | 492 | } | |
| @@ -479,7 +500,10 @@ func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, er | |||
| 479 | 500 | } | |
| 480 | 501 | ||
| 481 | 502 | cmd := exec.Command(execPath, "browse", uri) | |
| 482 | - | cmd.Env = sanitizedChildEnv(os.Environ()) | |
| 503 | + | cmd.Env = withoutEnvKeys(sanitizedChildEnv(os.Environ()), FreshWindowLaunchEnvVar) | |
| 504 | + | if freshWindow { | |
| 505 | + | cmd.Env = append(cmd.Env, FreshWindowLaunchEnvVar+"=1") | |
| 506 | + | } | |
| 483 | 507 | if err := spawnDetachedProcess(cmd); err != nil { | |
| 484 | 508 | return fmt.Errorf("failed to launch dumber browse for requested URL: %w", err) | |
| 485 | 509 | } | |
| @@ -446,6 +446,10 @@ func NewSessionSpawner(ctx context.Context, spawnEnv port.SessionSpawnEnvironmen | |||||
| 446 | // RestoreSessionEnvVar is the environment variable used to pass session ID for restoration. | 446 | // RestoreSessionEnvVar is the environment variable used to pass session ID for restoration. | ||
| 447 | const RestoreSessionEnvVar = "DUMBER_RESTORE_SESSION" | 447 | const RestoreSessionEnvVar = "DUMBER_RESTORE_SESSION" | ||
| 448 | 448 | ||||
| 449 | + | // FreshWindowLaunchEnvVar marks a browse child that must open a fresh window | |||
| 450 | + | // instead of applying external-link workspace placement. | |||
| 451 | + | const FreshWindowLaunchEnvVar = "DUMBER_BROWSER_FRESH_WINDOW" | |||
| 452 | + | ||||
| 449 | const layerShellPreloadToken = "libgtk4" + "-layer-shell" | 453 | const layerShellPreloadToken = "libgtk4" + "-layer-shell" | ||
| 450 | 454 | ||||
| 451 | // LaunchExternalURL opens a URL with the system's default handler (xdg-open on Linux). | 455 | // LaunchExternalURL opens a URL with the system's default handler (xdg-open on Linux). | ||
| @@ -466,6 +470,23 @@ func LaunchBrowserURL(uri string) { | |||||
| 466 | } | 470 | } | ||
| 467 | 471 | ||||
| 468 | func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, error), spawnDetachedProcess func(*exec.Cmd) error) error { | 472 | func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, error), spawnDetachedProcess func(*exec.Cmd) error) error { | ||
| 473 | + | return launchBrowserURL(uri, false, resolveExecutablePath, spawnDetachedProcess) | |||
| 474 | + | } | |||
| 475 | + | ||||
| 476 | + | func launchBrowserFreshWindowURL( | |||
| 477 | + | uri string, | |||
| 478 | + | resolveExecutablePath func() (string, error), | |||
| 479 | + | spawnDetachedProcess func(*exec.Cmd) error, | |||
| 480 | + | ) error { | |||
| 481 | + | return launchBrowserURL(uri, true, resolveExecutablePath, spawnDetachedProcess) | |||
| 482 | + | } | |||
| 483 | + | ||||
| 484 | + | func launchBrowserURL( | |||
| 485 | + | uri string, | |||
| 486 | + | freshWindow bool, | |||
| 487 | + | resolveExecutablePath func() (string, error), | |||
| 488 | + | spawnDetachedProcess func(*exec.Cmd) error, | |||
| 489 | + | ) error { | |||
| 469 | if resolveExecutablePath == nil { | 490 | if resolveExecutablePath == nil { | ||
| 470 | resolveExecutablePath = getExecutablePath | 491 | resolveExecutablePath = getExecutablePath | ||
| 471 | } | 492 | } | ||
| @@ -479,7 +500,10 @@ func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, er | |||||
| 479 | } | 500 | } | ||
| 480 | 501 | ||||
| 481 | cmd := exec.Command(execPath, "browse", uri) | 502 | cmd := exec.Command(execPath, "browse", uri) | ||
| 482 | - | cmd.Env = sanitizedChildEnv(os.Environ()) | 503 | + | cmd.Env = withoutEnvKeys(sanitizedChildEnv(os.Environ()), FreshWindowLaunchEnvVar) |
| 504 | + | if freshWindow { | |||
| 505 | + | cmd.Env = append(cmd.Env, FreshWindowLaunchEnvVar+"=1") | |||
| 506 | + | } | |||
| 483 | if err := spawnDetachedProcess(cmd); err != nil { | 507 | if err := spawnDetachedProcess(cmd); err != nil { | ||
| 484 | return fmt.Errorf("failed to launch dumber browse for requested URL: %w", err) | 508 | return fmt.Errorf("failed to launch dumber browse for requested URL: %w", err) | ||
| 485 | } | 509 | } | ||
M internal/infrastructure/desktop/adapter_test.go +15 -0
| @@ -220,6 +220,21 @@ func TestLaunchBrowserBrowseURL_SpawnErrorDoesNotLeakURL(t *testing.T) { | |||
| 220 | 220 | } | |
| 221 | 221 | } | |
| 222 | 222 | ||
| 223 | + | func TestLaunchBrowserBrowseURL_ClearsFreshWindowMarker(t *testing.T) { | |
| 224 | + | t.Setenv(FreshWindowLaunchEnvVar, "1") | |
| 225 | + | ||
| 226 | + | err := launchBrowserBrowseURL( | |
| 227 | + | "https://example.com", | |
| 228 | + | func() (string, error) { return "/usr/bin/dumber", nil }, | |
| 229 | + | func(cmd *exec.Cmd) error { | |
| 230 | + | assert.NotContains(t, cmd.Env, FreshWindowLaunchEnvVar+"=1") | |
| 231 | + | return nil | |
| 232 | + | }, | |
| 233 | + | ) | |
| 234 | + | ||
| 235 | + | require.NoError(t, err) | |
| 236 | + | } | |
| 237 | + | ||
| 223 | 238 | func TestSetAsDefaultBrowserAlsoUpdatesMimeHandlers(t *testing.T) { | |
| 224 | 239 | t.Setenv("HOME", t.TempDir()) | |
| 225 | 240 | t.Setenv("XDG_DATA_HOME", filepath.Join(os.Getenv("HOME"), ".local", "share")) | |
| @@ -220,6 +220,21 @@ func TestLaunchBrowserBrowseURL_SpawnErrorDoesNotLeakURL(t *testing.T) { | |||||
| 220 | } | 220 | } | ||
| 221 | } | 221 | } | ||
| 222 | 222 | ||||
| 223 | + | func TestLaunchBrowserBrowseURL_ClearsFreshWindowMarker(t *testing.T) { | |||
| 224 | + | t.Setenv(FreshWindowLaunchEnvVar, "1") | |||
| 225 | + | ||||
| 226 | + | err := launchBrowserBrowseURL( | |||
| 227 | + | "https://example.com", | |||
| 228 | + | func() (string, error) { return "/usr/bin/dumber", nil }, | |||
| 229 | + | func(cmd *exec.Cmd) error { | |||
| 230 | + | assert.NotContains(t, cmd.Env, FreshWindowLaunchEnvVar+"=1") | |||
| 231 | + | return nil | |||
| 232 | + | }, | |||
| 233 | + | ) | |||
| 234 | + | ||||
| 235 | + | require.NoError(t, err) | |||
| 236 | + | } | |||
| 237 | + | ||||
| 223 | func TestSetAsDefaultBrowserAlsoUpdatesMimeHandlers(t *testing.T) { | 238 | func TestSetAsDefaultBrowserAlsoUpdatesMimeHandlers(t *testing.T) { | ||
| 224 | t.Setenv("HOME", t.TempDir()) | 239 | t.Setenv("HOME", t.TempDir()) | ||
| 225 | t.Setenv("XDG_DATA_HOME", filepath.Join(os.Getenv("HOME"), ".local", "share")) | 240 | t.Setenv("XDG_DATA_HOME", filepath.Join(os.Getenv("HOME"), ".local", "share")) | ||
M internal/infrastructure/desktop/browser_launch_relay.go +27 -7
| @@ -39,10 +39,15 @@ type browserLaunchRelay struct { | |||
| 39 | 39 | } | |
| 40 | 40 | ||
| 41 | 41 | type browserLaunchRequest struct { | |
| 42 | - | RequestID string `json:"request_id,omitempty"` | |
| 43 | - | URL string `json:"url"` | |
| 42 | + | RequestID string `json:"request_id,omitempty"` | |
| 43 | + | URL string `json:"url"` | |
| 44 | + | Action browserLaunchAction `json:"action,omitempty"` | |
| 44 | 45 | } | |
| 45 | 46 | ||
| 47 | + | type browserLaunchAction string | |
| 48 | + | ||
| 49 | + | const browserLaunchActionOpenFreshWindow browserLaunchAction = "open-fresh-window" | |
| 50 | + | ||
| 46 | 51 | type browserLaunchResponse struct { | |
| 47 | 52 | RequestID string `json:"request_id,omitempty"` | |
| 48 | 53 | Accepted bool `json:"accepted,omitempty"` | |
| @@ -69,6 +74,14 @@ func NewBrowserLaunchRelay(ipc runtimeprofile.IPCPaths) port.BrowserLaunchRelay | |||
| 69 | 74 | } | |
| 70 | 75 | ||
| 71 | 76 | func (r *browserLaunchRelay) DeliverOpenExternalURL(ctx context.Context, url string) (bool, error) { | |
| 77 | + | return r.deliver(ctx, url, "") | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url string) (bool, error) { | |
| 81 | + | return r.deliver(ctx, url, browserLaunchActionOpenFreshWindow) | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | func (r *browserLaunchRelay) deliver(ctx context.Context, url string, action browserLaunchAction) (bool, error) { | |
| 72 | 85 | socketPath, err := r.socketPath() | |
| 73 | 86 | if err != nil { | |
| 74 | 87 | return false, err | |
| @@ -93,7 +106,7 @@ func (r *browserLaunchRelay) DeliverOpenExternalURL(ctx context.Context, url str | |||
| 93 | 106 | if err := setBrowserLaunchConnDeadline(ctx, conn); err != nil { | |
| 94 | 107 | return false, err | |
| 95 | 108 | } | |
| 96 | - | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: requestID, URL: url}); err != nil { | |
| 109 | + | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: requestID, URL: url, Action: action}); err != nil { | |
| 97 | 110 | return false, err | |
| 98 | 111 | } | |
| 99 | 112 | ||
| @@ -353,20 +366,27 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||
| 353 | 366 | log.Debug(). | |
| 354 | 367 | Str("request_id", requestID). | |
| 355 | 368 | Str("url_host", safeURLHost(request.URL)). | |
| 356 | - | Msg("browser launch relay calling external URL opener") | |
| 357 | - | if err := opener.OpenExternalURL(ctx, request.URL); err != nil { | |
| 369 | + | Msg("browser launch relay calling browser URL opener") | |
| 370 | + | var err error | |
| 371 | + | switch request.Action { | |
| 372 | + | case browserLaunchActionOpenFreshWindow: | |
| 373 | + | err = opener.OpenFreshWindow(ctx, request.URL) | |
| 374 | + | default: | |
| 375 | + | err = opener.OpenExternalURL(ctx, request.URL) | |
| 376 | + | } | |
| 377 | + | if err != nil { | |
| 358 | 378 | log.Warn().Err(err). | |
| 359 | 379 | Str("request_id", requestID). | |
| 360 | 380 | Str("url_host", safeURLHost(request.URL)). | |
| 361 | 381 | Dur("elapsed", time.Since(started)). | |
| 362 | - | Msg("browser launch relay external URL opener failed") | |
| 382 | + | Msg("browser launch relay browser URL opener failed") | |
| 363 | 383 | return | |
| 364 | 384 | } | |
| 365 | 385 | log.Debug(). | |
| 366 | 386 | Str("request_id", requestID). | |
| 367 | 387 | Str("url_host", safeURLHost(request.URL)). | |
| 368 | 388 | Dur("elapsed", time.Since(started)). | |
| 369 | - | Msg("browser launch relay external URL opener returned success") | |
| 389 | + | Msg("browser launch relay browser URL opener returned success") | |
| 370 | 390 | }() | |
| 371 | 391 | } | |
| 372 | 392 | ||
| @@ -39,10 +39,15 @@ type browserLaunchRelay struct { | |||||
| 39 | } | 39 | } | ||
| 40 | 40 | ||||
| 41 | type browserLaunchRequest struct { | 41 | type browserLaunchRequest struct { | ||
| 42 | - | RequestID string `json:"request_id,omitempty"` | 42 | + | RequestID string `json:"request_id,omitempty"` |
| 43 | - | URL string `json:"url"` | 43 | + | URL string `json:"url"` |
| 44 | + | Action browserLaunchAction `json:"action,omitempty"` | |||
| 44 | } | 45 | } | ||
| 45 | 46 | ||||
| 47 | + | type browserLaunchAction string | |||
| 48 | + | ||||
| 49 | + | const browserLaunchActionOpenFreshWindow browserLaunchAction = "open-fresh-window" | |||
| 50 | + | ||||
| 46 | type browserLaunchResponse struct { | 51 | type browserLaunchResponse struct { | ||
| 47 | RequestID string `json:"request_id,omitempty"` | 52 | RequestID string `json:"request_id,omitempty"` | ||
| 48 | Accepted bool `json:"accepted,omitempty"` | 53 | Accepted bool `json:"accepted,omitempty"` | ||
| @@ -69,6 +74,14 @@ func NewBrowserLaunchRelay(ipc runtimeprofile.IPCPaths) port.BrowserLaunchRelay | |||||
| 69 | } | 74 | } | ||
| 70 | 75 | ||||
| 71 | func (r *browserLaunchRelay) DeliverOpenExternalURL(ctx context.Context, url string) (bool, error) { | 76 | func (r *browserLaunchRelay) DeliverOpenExternalURL(ctx context.Context, url string) (bool, error) { | ||
| 77 | + | return r.deliver(ctx, url, "") | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url string) (bool, error) { | |||
| 81 | + | return r.deliver(ctx, url, browserLaunchActionOpenFreshWindow) | |||
| 82 | + | } | |||
| 83 | + | ||||
| 84 | + | func (r *browserLaunchRelay) deliver(ctx context.Context, url string, action browserLaunchAction) (bool, error) { | |||
| 72 | socketPath, err := r.socketPath() | 85 | socketPath, err := r.socketPath() | ||
| 73 | if err != nil { | 86 | if err != nil { | ||
| 74 | return false, err | 87 | return false, err | ||
| @@ -93,7 +106,7 @@ func (r *browserLaunchRelay) DeliverOpenExternalURL(ctx context.Context, url str | |||||
| 93 | if err := setBrowserLaunchConnDeadline(ctx, conn); err != nil { | 106 | if err := setBrowserLaunchConnDeadline(ctx, conn); err != nil { | ||
| 94 | return false, err | 107 | return false, err | ||
| 95 | } | 108 | } | ||
| 96 | - | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: requestID, URL: url}); err != nil { | 109 | + | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: requestID, URL: url, Action: action}); err != nil { |
| 97 | return false, err | 110 | return false, err | ||
| 98 | } | 111 | } | ||
| 99 | 112 | ||||
| @@ -353,20 +366,27 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||||
| 353 | log.Debug(). | 366 | log.Debug(). | ||
| 354 | Str("request_id", requestID). | 367 | Str("request_id", requestID). | ||
| 355 | Str("url_host", safeURLHost(request.URL)). | 368 | Str("url_host", safeURLHost(request.URL)). | ||
| 356 | - | Msg("browser launch relay calling external URL opener") | 369 | + | Msg("browser launch relay calling browser URL opener") |
| 357 | - | if err := opener.OpenExternalURL(ctx, request.URL); err != nil { | 370 | + | var err error |
| 371 | + | switch request.Action { | |||
| 372 | + | case browserLaunchActionOpenFreshWindow: | |||
| 373 | + | err = opener.OpenFreshWindow(ctx, request.URL) | |||
| 374 | + | default: | |||
| 375 | + | err = opener.OpenExternalURL(ctx, request.URL) | |||
| 376 | + | } | |||
| 377 | + | if err != nil { | |||
| 358 | log.Warn().Err(err). | 378 | log.Warn().Err(err). | ||
| 359 | Str("request_id", requestID). | 379 | Str("request_id", requestID). | ||
| 360 | Str("url_host", safeURLHost(request.URL)). | 380 | Str("url_host", safeURLHost(request.URL)). | ||
| 361 | Dur("elapsed", time.Since(started)). | 381 | Dur("elapsed", time.Since(started)). | ||
| 362 | - | Msg("browser launch relay external URL opener failed") | 382 | + | Msg("browser launch relay browser URL opener failed") |
| 363 | return | 383 | return | ||
| 364 | } | 384 | } | ||
| 365 | log.Debug(). | 385 | log.Debug(). | ||
| 366 | Str("request_id", requestID). | 386 | Str("request_id", requestID). | ||
| 367 | Str("url_host", safeURLHost(request.URL)). | 387 | Str("url_host", safeURLHost(request.URL)). | ||
| 368 | Dur("elapsed", time.Since(started)). | 388 | Dur("elapsed", time.Since(started)). | ||
| 369 | - | Msg("browser launch relay external URL opener returned success") | 389 | + | Msg("browser launch relay browser URL opener returned success") |
| 370 | }() | 390 | }() | ||
| 371 | } | 391 | } | ||
| 372 | 392 | ||||
M internal/infrastructure/desktop/browser_launch_relay_test.go +59 -4
| @@ -21,6 +21,23 @@ func (f browserWindowOpenerFunc) OpenExternalURL(ctx context.Context, url string | |||
| 21 | 21 | return f(ctx, url) | |
| 22 | 22 | } | |
| 23 | 23 | ||
| 24 | + | func (f browserWindowOpenerFunc) OpenFreshWindow(ctx context.Context, url string) error { | |
| 25 | + | return f(ctx, url) | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | type browserWindowOpener struct { | |
| 29 | + | openExternalURL func(context.Context, string) error | |
| 30 | + | openFreshWindow func(context.Context, string) error | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | func (o browserWindowOpener) OpenExternalURL(ctx context.Context, url string) error { | |
| 34 | + | return o.openExternalURL(ctx, url) | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | func (o browserWindowOpener) OpenFreshWindow(ctx context.Context, url string) error { | |
| 38 | + | return o.openFreshWindow(ctx, url) | |
| 39 | + | } | |
| 40 | + | ||
| 24 | 41 | func waitForSocket(t *testing.T, path string) { | |
| 25 | 42 | t.Helper() | |
| 26 | 43 | ||
| @@ -230,10 +247,16 @@ func TestBrowserLaunchRelay_DeliverOpenExternalURL_RoundTrip(t *testing.T) { | |||
| 230 | 247 | received := make(chan string, 1) | |
| 231 | 248 | ctx := t.Context() | |
| 232 | 249 | ||
| 233 | - | closer, err := relay.Listen(ctx, browserWindowOpenerFunc(func(_ context.Context, url string) error { | |
| 234 | - | received <- url | |
| 235 | - | return nil | |
| 236 | - | })) | |
| 250 | + | closer, err := relay.Listen(ctx, browserWindowOpener{ | |
| 251 | + | openExternalURL: func(_ context.Context, url string) error { | |
| 252 | + | received <- url | |
| 253 | + | return nil | |
| 254 | + | }, | |
| 255 | + | openFreshWindow: func(context.Context, string) error { | |
| 256 | + | t.Fatal("external URL relay request must preserve configured placement") | |
| 257 | + | return nil | |
| 258 | + | }, | |
| 259 | + | }) | |
| 237 | 260 | require.NoError(t, err) | |
| 238 | 261 | defer closer.Close() | |
| 239 | 262 | ||
| @@ -252,6 +275,38 @@ func TestBrowserLaunchRelay_DeliverOpenExternalURL_RoundTrip(t *testing.T) { | |||
| 252 | 275 | } | |
| 253 | 276 | } | |
| 254 | 277 | ||
| 278 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RoundTrip(t *testing.T) { | |
| 279 | + | ipc := testIPC(shortTempDir(t)) | |
| 280 | + | relay := NewBrowserLaunchRelay(ipc) | |
| 281 | + | ||
| 282 | + | freshWindows := make(chan string, 1) | |
| 283 | + | ctx := t.Context() | |
| 284 | + | closer, err := relay.Listen(ctx, browserWindowOpener{ | |
| 285 | + | openExternalURL: func(context.Context, string) error { | |
| 286 | + | t.Fatal("external-link placement must not handle standalone omnibox navigation") | |
| 287 | + | return nil | |
| 288 | + | }, | |
| 289 | + | openFreshWindow: func(_ context.Context, url string) error { | |
| 290 | + | freshWindows <- url | |
| 291 | + | return nil | |
| 292 | + | }, | |
| 293 | + | }) | |
| 294 | + | require.NoError(t, err) | |
| 295 | + | defer closer.Close() | |
| 296 | + | ||
| 297 | + | waitForSocket(t, ipc.BrowserLaunchSocket) | |
| 298 | + | delivered, err := relay.DeliverOpenFreshWindow(context.Background(), "https://example.com/omnibox") | |
| 299 | + | ||
| 300 | + | require.NoError(t, err) | |
| 301 | + | require.True(t, delivered) | |
| 302 | + | select { | |
| 303 | + | case got := <-freshWindows: | |
| 304 | + | assert.Equal(t, "https://example.com/omnibox", got) | |
| 305 | + | case <-time.After(time.Second): | |
| 306 | + | t.Fatal("expected fresh-window opener to receive the URL") | |
| 307 | + | } | |
| 308 | + | } | |
| 309 | + | ||
| 255 | 310 | func TestBrowserLaunchRelay_RequiresBrowserLaunchSocketPath(t *testing.T) { | |
| 256 | 311 | relay := NewBrowserLaunchRelay(runtimeprofile.IPCPaths{}) | |
| 257 | 312 | ||
| @@ -21,6 +21,23 @@ func (f browserWindowOpenerFunc) OpenExternalURL(ctx context.Context, url string | |||||
| 21 | return f(ctx, url) | 21 | return f(ctx, url) | ||
| 22 | } | 22 | } | ||
| 23 | 23 | ||||
| 24 | + | func (f browserWindowOpenerFunc) OpenFreshWindow(ctx context.Context, url string) error { | |||
| 25 | + | return f(ctx, url) | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | type browserWindowOpener struct { | |||
| 29 | + | openExternalURL func(context.Context, string) error | |||
| 30 | + | openFreshWindow func(context.Context, string) error | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | func (o browserWindowOpener) OpenExternalURL(ctx context.Context, url string) error { | |||
| 34 | + | return o.openExternalURL(ctx, url) | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | func (o browserWindowOpener) OpenFreshWindow(ctx context.Context, url string) error { | |||
| 38 | + | return o.openFreshWindow(ctx, url) | |||
| 39 | + | } | |||
| 40 | + | ||||
| 24 | func waitForSocket(t *testing.T, path string) { | 41 | func waitForSocket(t *testing.T, path string) { | ||
| 25 | t.Helper() | 42 | t.Helper() | ||
| 26 | 43 | ||||
| @@ -230,10 +247,16 @@ func TestBrowserLaunchRelay_DeliverOpenExternalURL_RoundTrip(t *testing.T) { | |||||
| 230 | received := make(chan string, 1) | 247 | received := make(chan string, 1) | ||
| 231 | ctx := t.Context() | 248 | ctx := t.Context() | ||
| 232 | 249 | ||||
| 233 | - | closer, err := relay.Listen(ctx, browserWindowOpenerFunc(func(_ context.Context, url string) error { | 250 | + | closer, err := relay.Listen(ctx, browserWindowOpener{ |
| 234 | - | received <- url | 251 | + | openExternalURL: func(_ context.Context, url string) error { |
| 235 | - | return nil | 252 | + | received <- url |
| 236 | - | })) | 253 | + | return nil |
| 254 | + | }, | |||
| 255 | + | openFreshWindow: func(context.Context, string) error { | |||
| 256 | + | t.Fatal("external URL relay request must preserve configured placement") | |||
| 257 | + | return nil | |||
| 258 | + | }, | |||
| 259 | + | }) | |||
| 237 | require.NoError(t, err) | 260 | require.NoError(t, err) | ||
| 238 | defer closer.Close() | 261 | defer closer.Close() | ||
| 239 | 262 | ||||
| @@ -252,6 +275,38 @@ func TestBrowserLaunchRelay_DeliverOpenExternalURL_RoundTrip(t *testing.T) { | |||||
| 252 | } | 275 | } | ||
| 253 | } | 276 | } | ||
| 254 | 277 | ||||
| 278 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RoundTrip(t *testing.T) { | |||
| 279 | + | ipc := testIPC(shortTempDir(t)) | |||
| 280 | + | relay := NewBrowserLaunchRelay(ipc) | |||
| 281 | + | ||||
| 282 | + | freshWindows := make(chan string, 1) | |||
| 283 | + | ctx := t.Context() | |||
| 284 | + | closer, err := relay.Listen(ctx, browserWindowOpener{ | |||
| 285 | + | openExternalURL: func(context.Context, string) error { | |||
| 286 | + | t.Fatal("external-link placement must not handle standalone omnibox navigation") | |||
| 287 | + | return nil | |||
| 288 | + | }, | |||
| 289 | + | openFreshWindow: func(_ context.Context, url string) error { | |||
| 290 | + | freshWindows <- url | |||
| 291 | + | return nil | |||
| 292 | + | }, | |||
| 293 | + | }) | |||
| 294 | + | require.NoError(t, err) | |||
| 295 | + | defer closer.Close() | |||
| 296 | + | ||||
| 297 | + | waitForSocket(t, ipc.BrowserLaunchSocket) | |||
| 298 | + | delivered, err := relay.DeliverOpenFreshWindow(context.Background(), "https://example.com/omnibox") | |||
| 299 | + | ||||
| 300 | + | require.NoError(t, err) | |||
| 301 | + | require.True(t, delivered) | |||
| 302 | + | select { | |||
| 303 | + | case got := <-freshWindows: | |||
| 304 | + | assert.Equal(t, "https://example.com/omnibox", got) | |||
| 305 | + | case <-time.After(time.Second): | |||
| 306 | + | t.Fatal("expected fresh-window opener to receive the URL") | |||
| 307 | + | } | |||
| 308 | + | } | |||
| 309 | + | ||||
| 255 | func TestBrowserLaunchRelay_RequiresBrowserLaunchSocketPath(t *testing.T) { | 310 | func TestBrowserLaunchRelay_RequiresBrowserLaunchSocketPath(t *testing.T) { | ||
| 256 | relay := NewBrowserLaunchRelay(runtimeprofile.IPCPaths{}) | 311 | relay := NewBrowserLaunchRelay(runtimeprofile.IPCPaths{}) | ||
| 257 | 312 | ||||
M internal/infrastructure/desktop/browser_launcher.go +7 -5
| @@ -13,7 +13,7 @@ import ( | |||
| 13 | 13 | // received the request, but the caller could not confirm that in time. | |
| 14 | 14 | var ErrBrowserLaunchUnconfirmed = errors.New("browser launch could not be confirmed") | |
| 15 | 15 | ||
| 16 | - | // BrowserLauncher opens URLs in a relay-first dumber browser instance. | |
| 16 | + | // BrowserLauncher opens standalone-omnibox URLs in a relay-first dumber browser instance. | |
| 17 | 17 | type BrowserLauncher struct { | |
| 18 | 18 | relay port.BrowserLaunchRelay | |
| 19 | 19 | resolveExecutablePath func() (string, error) | |
| @@ -29,14 +29,16 @@ func NewBrowserLauncher(relay port.BrowserLaunchRelay) *BrowserLauncher { | |||
| 29 | 29 | } | |
| 30 | 30 | } | |
| 31 | 31 | ||
| 32 | - | // LaunchURL forwards the URL to a running instance when possible, otherwise spawns a new one. | |
| 33 | - | func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error { | |
| 32 | + | // LaunchFreshWindowURL forwards the URL to a running instance when possible, | |
| 33 | + | // always requesting a new browser window. This keeps standalone omnibox | |
| 34 | + | // navigation independent from external-link workspace placement settings. | |
| 35 | + | func (l *BrowserLauncher) LaunchFreshWindowURL(ctx context.Context, url string) error { | |
| 34 | 36 | if l == nil { | |
| 35 | 37 | return errors.New("browser launcher is unavailable") | |
| 36 | 38 | } | |
| 37 | 39 | ||
| 38 | 40 | if l.relay != nil { | |
| 39 | - | delivered, err := l.relay.DeliverOpenExternalURL(ctx, url) | |
| 41 | + | delivered, err := l.relay.DeliverOpenFreshWindow(ctx, url) | |
| 40 | 42 | if err != nil { | |
| 41 | 43 | if delivered && errors.Is(err, ErrBrowserLaunchRelayUnconfirmed) { | |
| 42 | 44 | return ErrBrowserLaunchUnconfirmed | |
| @@ -48,7 +50,7 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error { | |||
| 48 | 50 | } | |
| 49 | 51 | } | |
| 50 | 52 | ||
| 51 | - | if err := launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil { | |
| 53 | + | if err := launchBrowserFreshWindowURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil { | |
| 52 | 54 | return fmt.Errorf("launch dumber browse: %w", err) | |
| 53 | 55 | } | |
| 54 | 56 | return nil | |
| @@ -13,7 +13,7 @@ import ( | |||||
| 13 | // received the request, but the caller could not confirm that in time. | 13 | // received the request, but the caller could not confirm that in time. | ||
| 14 | var ErrBrowserLaunchUnconfirmed = errors.New("browser launch could not be confirmed") | 14 | var ErrBrowserLaunchUnconfirmed = errors.New("browser launch could not be confirmed") | ||
| 15 | 15 | ||||
| 16 | - | // BrowserLauncher opens URLs in a relay-first dumber browser instance. | 16 | + | // BrowserLauncher opens standalone-omnibox URLs in a relay-first dumber browser instance. |
| 17 | type BrowserLauncher struct { | 17 | type BrowserLauncher struct { | ||
| 18 | relay port.BrowserLaunchRelay | 18 | relay port.BrowserLaunchRelay | ||
| 19 | resolveExecutablePath func() (string, error) | 19 | resolveExecutablePath func() (string, error) | ||
| @@ -29,14 +29,16 @@ func NewBrowserLauncher(relay port.BrowserLaunchRelay) *BrowserLauncher { | |||||
| 29 | } | 29 | } | ||
| 30 | } | 30 | } | ||
| 31 | 31 | ||||
| 32 | - | // LaunchURL forwards the URL to a running instance when possible, otherwise spawns a new one. | 32 | + | // LaunchFreshWindowURL forwards the URL to a running instance when possible, |
| 33 | - | func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error { | 33 | + | // always requesting a new browser window. This keeps standalone omnibox |
| 34 | + | // navigation independent from external-link workspace placement settings. | |||
| 35 | + | func (l *BrowserLauncher) LaunchFreshWindowURL(ctx context.Context, url string) error { | |||
| 34 | if l == nil { | 36 | if l == nil { | ||
| 35 | return errors.New("browser launcher is unavailable") | 37 | return errors.New("browser launcher is unavailable") | ||
| 36 | } | 38 | } | ||
| 37 | 39 | ||||
| 38 | if l.relay != nil { | 40 | if l.relay != nil { | ||
| 39 | - | delivered, err := l.relay.DeliverOpenExternalURL(ctx, url) | 41 | + | delivered, err := l.relay.DeliverOpenFreshWindow(ctx, url) |
| 40 | if err != nil { | 42 | if err != nil { | ||
| 41 | if delivered && errors.Is(err, ErrBrowserLaunchRelayUnconfirmed) { | 43 | if delivered && errors.Is(err, ErrBrowserLaunchRelayUnconfirmed) { | ||
| 42 | return ErrBrowserLaunchUnconfirmed | 44 | return ErrBrowserLaunchUnconfirmed | ||
| @@ -48,7 +50,7 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) error { | |||||
| 48 | } | 50 | } | ||
| 49 | } | 51 | } | ||
| 50 | 52 | ||||
| 51 | - | if err := launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil { | 53 | + | if err := launchBrowserFreshWindowURL(url, l.resolveExecutablePath, l.startDetachedProcess); err != nil { |
| 52 | return fmt.Errorf("launch dumber browse: %w", err) | 54 | return fmt.Errorf("launch dumber browse: %w", err) | ||
| 53 | } | 55 | } | ||
| 54 | return nil | 56 | return nil | ||
M internal/infrastructure/desktop/browser_launcher_test.go +15 -14
| @@ -11,9 +11,9 @@ import ( | |||
| 11 | 11 | "github.com/stretchr/testify/require" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | - | func TestBrowserLauncher_LaunchURL_ReturnsWithoutSpawningWhenRelayDelivers(t *testing.T) { | |
| 14 | + | func TestBrowserLauncher_LaunchFreshWindowURL_ReturnsWithoutSpawningWhenRelayDelivers(t *testing.T) { | |
| 15 | 15 | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 16 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(true, nil) | |
| 16 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, nil) | |
| 17 | 17 | ||
| 18 | 18 | launcher := NewBrowserLauncher(relay) | |
| 19 | 19 | spawned := false | |
| @@ -26,15 +26,15 @@ func TestBrowserLauncher_LaunchURL_ReturnsWithoutSpawningWhenRelayDelivers(t *te | |||
| 26 | 26 | return nil | |
| 27 | 27 | } | |
| 28 | 28 | ||
| 29 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | |
| 29 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") | |
| 30 | 30 | ||
| 31 | 31 | require.NoError(t, err) | |
| 32 | 32 | assert.False(t, spawned) | |
| 33 | 33 | } | |
| 34 | 34 | ||
| 35 | - | func TestBrowserLauncher_LaunchURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) { | |
| 35 | + | func TestBrowserLauncher_LaunchFreshWindowURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) { | |
| 36 | 36 | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 37 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, nil) | |
| 37 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(false, nil) | |
| 38 | 38 | ||
| 39 | 39 | launcher := NewBrowserLauncher(relay) | |
| 40 | 40 | launcher.resolveExecutablePath = func() (string, error) { | |
| @@ -47,17 +47,18 @@ func TestBrowserLauncher_LaunchURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) | |||
| 47 | 47 | return nil | |
| 48 | 48 | } | |
| 49 | 49 | ||
| 50 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | |
| 50 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") | |
| 51 | 51 | ||
| 52 | 52 | require.NoError(t, err) | |
| 53 | 53 | require.NotNil(t, gotCmd) | |
| 54 | 54 | assert.Equal(t, "/usr/bin/dumber", gotCmd.Path) | |
| 55 | 55 | assert.Equal(t, []string{"/usr/bin/dumber", "browse", "https://example.com"}, gotCmd.Args) | |
| 56 | + | assert.Contains(t, gotCmd.Env, FreshWindowLaunchEnvVar+"=1") | |
| 56 | 57 | } | |
| 57 | 58 | ||
| 58 | - | func TestBrowserLauncher_LaunchURL_ReturnsUnconfirmedErrorWhenRelayDeliveryIsAmbiguous(t *testing.T) { | |
| 59 | + | func TestBrowserLauncher_LaunchFreshWindowURL_ReturnsUnconfirmedErrorWhenRelayDeliveryIsAmbiguous(t *testing.T) { | |
| 59 | 60 | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 60 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(true, ErrBrowserLaunchRelayUnconfirmed) | |
| 61 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, ErrBrowserLaunchRelayUnconfirmed) | |
| 61 | 62 | ||
| 62 | 63 | launcher := NewBrowserLauncher(relay) | |
| 63 | 64 | launcher.resolveExecutablePath = func() (string, error) { | |
| @@ -69,16 +70,16 @@ func TestBrowserLauncher_LaunchURL_ReturnsUnconfirmedErrorWhenRelayDeliveryIsAmb | |||
| 69 | 70 | return nil | |
| 70 | 71 | } | |
| 71 | 72 | ||
| 72 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | |
| 73 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") | |
| 73 | 74 | ||
| 74 | 75 | require.Error(t, err) | |
| 75 | 76 | assert.ErrorIs(t, err, ErrBrowserLaunchUnconfirmed) | |
| 76 | 77 | } | |
| 77 | 78 | ||
| 78 | - | func TestBrowserLauncher_LaunchURL_PropagatesRelayError(t *testing.T) { | |
| 79 | + | func TestBrowserLauncher_LaunchFreshWindowURL_PropagatesRelayError(t *testing.T) { | |
| 79 | 80 | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 80 | 81 | wantErr := errors.New("relay exploded") | |
| 81 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, wantErr) | |
| 82 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(false, wantErr) | |
| 82 | 83 | ||
| 83 | 84 | launcher := NewBrowserLauncher(relay) | |
| 84 | 85 | launcher.resolveExecutablePath = func() (string, error) { | |
| @@ -90,17 +91,17 @@ func TestBrowserLauncher_LaunchURL_PropagatesRelayError(t *testing.T) { | |||
| 90 | 91 | return nil | |
| 91 | 92 | } | |
| 92 | 93 | ||
| 93 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | |
| 94 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") | |
| 94 | 95 | ||
| 95 | 96 | require.Error(t, err) | |
| 96 | 97 | require.ErrorIs(t, err, wantErr) | |
| 97 | 98 | assert.NotContains(t, err.Error(), "https://example.com") | |
| 98 | 99 | } | |
| 99 | 100 | ||
| 100 | - | func TestBrowserLauncher_LaunchURL_NilLauncherReturnsError(t *testing.T) { | |
| 101 | + | func TestBrowserLauncher_LaunchFreshWindowURL_NilLauncherReturnsError(t *testing.T) { | |
| 101 | 102 | var launcher *BrowserLauncher | |
| 102 | 103 | ||
| 103 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | |
| 104 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") | |
| 104 | 105 | ||
| 105 | 106 | require.Error(t, err) | |
| 106 | 107 | assert.Contains(t, err.Error(), "unavailable") | |
| @@ -11,9 +11,9 @@ import ( | |||||
| 11 | "github.com/stretchr/testify/require" | 11 | "github.com/stretchr/testify/require" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | - | func TestBrowserLauncher_LaunchURL_ReturnsWithoutSpawningWhenRelayDelivers(t *testing.T) { | 14 | + | func TestBrowserLauncher_LaunchFreshWindowURL_ReturnsWithoutSpawningWhenRelayDelivers(t *testing.T) { |
| 15 | relay := mocks.NewMockBrowserLaunchRelay(t) | 15 | relay := mocks.NewMockBrowserLaunchRelay(t) | ||
| 16 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(true, nil) | 16 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, nil) |
| 17 | 17 | ||||
| 18 | launcher := NewBrowserLauncher(relay) | 18 | launcher := NewBrowserLauncher(relay) | ||
| 19 | spawned := false | 19 | spawned := false | ||
| @@ -26,15 +26,15 @@ func TestBrowserLauncher_LaunchURL_ReturnsWithoutSpawningWhenRelayDelivers(t *te | |||||
| 26 | return nil | 26 | return nil | ||
| 27 | } | 27 | } | ||
| 28 | 28 | ||||
| 29 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | 29 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") |
| 30 | 30 | ||||
| 31 | require.NoError(t, err) | 31 | require.NoError(t, err) | ||
| 32 | assert.False(t, spawned) | 32 | assert.False(t, spawned) | ||
| 33 | } | 33 | } | ||
| 34 | 34 | ||||
| 35 | - | func TestBrowserLauncher_LaunchURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) { | 35 | + | func TestBrowserLauncher_LaunchFreshWindowURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) { |
| 36 | relay := mocks.NewMockBrowserLaunchRelay(t) | 36 | relay := mocks.NewMockBrowserLaunchRelay(t) | ||
| 37 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, nil) | 37 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(false, nil) |
| 38 | 38 | ||||
| 39 | launcher := NewBrowserLauncher(relay) | 39 | launcher := NewBrowserLauncher(relay) | ||
| 40 | launcher.resolveExecutablePath = func() (string, error) { | 40 | launcher.resolveExecutablePath = func() (string, error) { | ||
| @@ -47,17 +47,18 @@ func TestBrowserLauncher_LaunchURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) | |||||
| 47 | return nil | 47 | return nil | ||
| 48 | } | 48 | } | ||
| 49 | 49 | ||||
| 50 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | 50 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") |
| 51 | 51 | ||||
| 52 | require.NoError(t, err) | 52 | require.NoError(t, err) | ||
| 53 | require.NotNil(t, gotCmd) | 53 | require.NotNil(t, gotCmd) | ||
| 54 | assert.Equal(t, "/usr/bin/dumber", gotCmd.Path) | 54 | assert.Equal(t, "/usr/bin/dumber", gotCmd.Path) | ||
| 55 | assert.Equal(t, []string{"/usr/bin/dumber", "browse", "https://example.com"}, gotCmd.Args) | 55 | assert.Equal(t, []string{"/usr/bin/dumber", "browse", "https://example.com"}, gotCmd.Args) | ||
| 56 | + | assert.Contains(t, gotCmd.Env, FreshWindowLaunchEnvVar+"=1") | |||
| 56 | } | 57 | } | ||
| 57 | 58 | ||||
| 58 | - | func TestBrowserLauncher_LaunchURL_ReturnsUnconfirmedErrorWhenRelayDeliveryIsAmbiguous(t *testing.T) { | 59 | + | func TestBrowserLauncher_LaunchFreshWindowURL_ReturnsUnconfirmedErrorWhenRelayDeliveryIsAmbiguous(t *testing.T) { |
| 59 | relay := mocks.NewMockBrowserLaunchRelay(t) | 60 | relay := mocks.NewMockBrowserLaunchRelay(t) | ||
| 60 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(true, ErrBrowserLaunchRelayUnconfirmed) | 61 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, ErrBrowserLaunchRelayUnconfirmed) |
| 61 | 62 | ||||
| 62 | launcher := NewBrowserLauncher(relay) | 63 | launcher := NewBrowserLauncher(relay) | ||
| 63 | launcher.resolveExecutablePath = func() (string, error) { | 64 | launcher.resolveExecutablePath = func() (string, error) { | ||
| @@ -69,16 +70,16 @@ func TestBrowserLauncher_LaunchURL_ReturnsUnconfirmedErrorWhenRelayDeliveryIsAmb | |||||
| 69 | return nil | 70 | return nil | ||
| 70 | } | 71 | } | ||
| 71 | 72 | ||||
| 72 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | 73 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") |
| 73 | 74 | ||||
| 74 | require.Error(t, err) | 75 | require.Error(t, err) | ||
| 75 | assert.ErrorIs(t, err, ErrBrowserLaunchUnconfirmed) | 76 | assert.ErrorIs(t, err, ErrBrowserLaunchUnconfirmed) | ||
| 76 | } | 77 | } | ||
| 77 | 78 | ||||
| 78 | - | func TestBrowserLauncher_LaunchURL_PropagatesRelayError(t *testing.T) { | 79 | + | func TestBrowserLauncher_LaunchFreshWindowURL_PropagatesRelayError(t *testing.T) { |
| 79 | relay := mocks.NewMockBrowserLaunchRelay(t) | 80 | relay := mocks.NewMockBrowserLaunchRelay(t) | ||
| 80 | wantErr := errors.New("relay exploded") | 81 | wantErr := errors.New("relay exploded") | ||
| 81 | - | relay.EXPECT().DeliverOpenExternalURL(context.Background(), "https://example.com").Return(false, wantErr) | 82 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(false, wantErr) |
| 82 | 83 | ||||
| 83 | launcher := NewBrowserLauncher(relay) | 84 | launcher := NewBrowserLauncher(relay) | ||
| 84 | launcher.resolveExecutablePath = func() (string, error) { | 85 | launcher.resolveExecutablePath = func() (string, error) { | ||
| @@ -90,17 +91,17 @@ func TestBrowserLauncher_LaunchURL_PropagatesRelayError(t *testing.T) { | |||||
| 90 | return nil | 91 | return nil | ||
| 91 | } | 92 | } | ||
| 92 | 93 | ||||
| 93 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | 94 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") |
| 94 | 95 | ||||
| 95 | require.Error(t, err) | 96 | require.Error(t, err) | ||
| 96 | require.ErrorIs(t, err, wantErr) | 97 | require.ErrorIs(t, err, wantErr) | ||
| 97 | assert.NotContains(t, err.Error(), "https://example.com") | 98 | assert.NotContains(t, err.Error(), "https://example.com") | ||
| 98 | } | 99 | } | ||
| 99 | 100 | ||||
| 100 | - | func TestBrowserLauncher_LaunchURL_NilLauncherReturnsError(t *testing.T) { | 101 | + | func TestBrowserLauncher_LaunchFreshWindowURL_NilLauncherReturnsError(t *testing.T) { |
| 101 | var launcher *BrowserLauncher | 102 | var launcher *BrowserLauncher | ||
| 102 | 103 | ||||
| 103 | - | err := launcher.LaunchURL(context.Background(), "https://example.com") | 104 | + | err := launcher.LaunchFreshWindowURL(context.Background(), "https://example.com") |
| 104 | 105 | ||||
| 105 | require.Error(t, err) | 106 | require.Error(t, err) | ||
| 106 | assert.Contains(t, err.Error(), "unavailable") | 107 | assert.Contains(t, err.Error(), "unavailable") | ||
M internal/ui/app_browser_launch_test.go +4 -0
| @@ -51,6 +51,10 @@ func (r *testBrowserLaunchRelay) DeliverOpenExternalURL(context.Context, string) | |||
| 51 | 51 | return false, nil | |
| 52 | 52 | } | |
| 53 | 53 | ||
| 54 | + | func (r *testBrowserLaunchRelay) DeliverOpenFreshWindow(context.Context, string) (bool, error) { | |
| 55 | + | return false, nil | |
| 56 | + | } | |
| 57 | + | ||
| 54 | 58 | func (r *testBrowserLaunchRelay) Listen(_ context.Context, opener port.BrowserWindowOpener) (io.Closer, error) { | |
| 55 | 59 | r.listenCalls++ | |
| 56 | 60 | _ = opener | |
| @@ -51,6 +51,10 @@ func (r *testBrowserLaunchRelay) DeliverOpenExternalURL(context.Context, string) | |||||
| 51 | return false, nil | 51 | return false, nil | ||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
| 54 | + | func (r *testBrowserLaunchRelay) DeliverOpenFreshWindow(context.Context, string) (bool, error) { | |||
| 55 | + | return false, nil | |||
| 56 | + | } | |||
| 57 | + | ||||
| 54 | func (r *testBrowserLaunchRelay) Listen(_ context.Context, opener port.BrowserWindowOpener) (io.Closer, error) { | 58 | func (r *testBrowserLaunchRelay) Listen(_ context.Context, opener port.BrowserWindowOpener) (io.Closer, error) { | ||
| 55 | r.listenCalls++ | 59 | r.listenCalls++ | ||
| 56 | _ = opener | 60 | _ = opener | ||
M internal/ui/browser_window.go +61 -0
| @@ -610,6 +610,67 @@ func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |||
| 610 | 610 | return nil | |
| 611 | 611 | } | |
| 612 | 612 | ||
| 613 | + | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | |
| 614 | + | log := logging.FromContext(ctx) | |
| 615 | + | log.Debug(). | |
| 616 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 617 | + | Msg("ui: open fresh window dispatch requested") | |
| 618 | + | ||
| 619 | + | dispatch := a.dispatchOnMainThread | |
| 620 | + | if dispatch == nil { | |
| 621 | + | dispatch = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 622 | + | if fn != nil { | |
| 623 | + | fn() | |
| 624 | + | } | |
| 625 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |
| 626 | + | } | |
| 627 | + | } | |
| 628 | + | ||
| 629 | + | var openErr error | |
| 630 | + | var windowCountBefore int | |
| 631 | + | var windowCountAfter int | |
| 632 | + | var hasTabCoord bool | |
| 633 | + | var hasTabsUC bool | |
| 634 | + | result := dispatch("ui.open_fresh_window", func() { | |
| 635 | + | windowCountBefore = len(a.browserWindows) | |
| 636 | + | hasTabCoord = a.tabCoord != nil | |
| 637 | + | hasTabsUC = a.tabsUC != nil | |
| 638 | + | log.Debug(). | |
| 639 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 640 | + | Int("window_count_before", windowCountBefore). | |
| 641 | + | Bool("has_tab_coord", hasTabCoord). | |
| 642 | + | Bool("has_tabs_uc", hasTabsUC). | |
| 643 | + | Msg("ui: open fresh window main-thread work started") | |
| 644 | + | openErr = a.openFreshWindow(ctx, url) | |
| 645 | + | windowCountAfter = len(a.browserWindows) | |
| 646 | + | }) | |
| 647 | + | if !result.Completed() { | |
| 648 | + | log.Warn(). | |
| 649 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 650 | + | Dur("elapsed", result.Elapsed). | |
| 651 | + | Str("dispatch_status", string(result.Status)). | |
| 652 | + | Msg("ui: open fresh window skipped after main-thread dispatch did not complete") | |
| 653 | + | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |
| 654 | + | } | |
| 655 | + | if openErr != nil { | |
| 656 | + | log.Warn().Err(openErr). | |
| 657 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 658 | + | Dur("elapsed", result.Elapsed). | |
| 659 | + | Str("dispatch_status", string(result.Status)). | |
| 660 | + | Int("window_count_after", windowCountAfter). | |
| 661 | + | Msg("ui: open fresh window failed") | |
| 662 | + | return openErr | |
| 663 | + | } | |
| 664 | + | ||
| 665 | + | log.Debug(). | |
| 666 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 667 | + | Dur("elapsed", result.Elapsed). | |
| 668 | + | Str("dispatch_status", string(result.Status)). | |
| 669 | + | Int("window_count_after", windowCountAfter). | |
| 670 | + | Msg("ui: open fresh window completed") | |
| 671 | + | return nil | |
| 672 | + | } | |
| 673 | + | ||
| 613 | 674 | func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | |
| 614 | 675 | externalLinks := cfg.UI.Workspace.ExternalLinks | |
| 615 | 676 | if externalLinks.Behavior == entity.ExternalLinkBehaviorWindowed || externalLinks.Behavior == "" { | |
| @@ -610,6 +610,67 @@ func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |||||
| 610 | return nil | 610 | return nil | ||
| 611 | } | 611 | } | ||
| 612 | 612 | ||||
| 613 | + | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | |||
| 614 | + | log := logging.FromContext(ctx) | |||
| 615 | + | log.Debug(). | |||
| 616 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 617 | + | Msg("ui: open fresh window dispatch requested") | |||
| 618 | + | ||||
| 619 | + | dispatch := a.dispatchOnMainThread | |||
| 620 | + | if dispatch == nil { | |||
| 621 | + | dispatch = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 622 | + | if fn != nil { | |||
| 623 | + | fn() | |||
| 624 | + | } | |||
| 625 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |||
| 626 | + | } | |||
| 627 | + | } | |||
| 628 | + | ||||
| 629 | + | var openErr error | |||
| 630 | + | var windowCountBefore int | |||
| 631 | + | var windowCountAfter int | |||
| 632 | + | var hasTabCoord bool | |||
| 633 | + | var hasTabsUC bool | |||
| 634 | + | result := dispatch("ui.open_fresh_window", func() { | |||
| 635 | + | windowCountBefore = len(a.browserWindows) | |||
| 636 | + | hasTabCoord = a.tabCoord != nil | |||
| 637 | + | hasTabsUC = a.tabsUC != nil | |||
| 638 | + | log.Debug(). | |||
| 639 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 640 | + | Int("window_count_before", windowCountBefore). | |||
| 641 | + | Bool("has_tab_coord", hasTabCoord). | |||
| 642 | + | Bool("has_tabs_uc", hasTabsUC). | |||
| 643 | + | Msg("ui: open fresh window main-thread work started") | |||
| 644 | + | openErr = a.openFreshWindow(ctx, url) | |||
| 645 | + | windowCountAfter = len(a.browserWindows) | |||
| 646 | + | }) | |||
| 647 | + | if !result.Completed() { | |||
| 648 | + | log.Warn(). | |||
| 649 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 650 | + | Dur("elapsed", result.Elapsed). | |||
| 651 | + | Str("dispatch_status", string(result.Status)). | |||
| 652 | + | Msg("ui: open fresh window skipped after main-thread dispatch did not complete") | |||
| 653 | + | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |||
| 654 | + | } | |||
| 655 | + | if openErr != nil { | |||
| 656 | + | log.Warn().Err(openErr). | |||
| 657 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 658 | + | Dur("elapsed", result.Elapsed). | |||
| 659 | + | Str("dispatch_status", string(result.Status)). | |||
| 660 | + | Int("window_count_after", windowCountAfter). | |||
| 661 | + | Msg("ui: open fresh window failed") | |||
| 662 | + | return openErr | |||
| 663 | + | } | |||
| 664 | + | ||||
| 665 | + | log.Debug(). | |||
| 666 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 667 | + | Dur("elapsed", result.Elapsed). | |||
| 668 | + | Str("dispatch_status", string(result.Status)). | |||
| 669 | + | Int("window_count_after", windowCountAfter). | |||
| 670 | + | Msg("ui: open fresh window completed") | |||
| 671 | + | return nil | |||
| 672 | + | } | |||
| 673 | + | ||||
| 613 | func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | 674 | func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | ||
| 614 | externalLinks := cfg.UI.Workspace.ExternalLinks | 675 | externalLinks := cfg.UI.Workspace.ExternalLinks | ||
| 615 | if externalLinks.Behavior == entity.ExternalLinkBehaviorWindowed || externalLinks.Behavior == "" { | 676 | if externalLinks.Behavior == entity.ExternalLinkBehaviorWindowed || externalLinks.Behavior == "" { | ||