Merge pull request 'fix(navigation): unblock OAuth handoff to desktop apps and clean popup leftovers' (#11) from fix/popup-oauth-external-scheme into main
Reviewed-on: https://git.bnema.dev/bnema/dumber/pulls/11
21 files · +1125 -70
A internal/application/usecase/relay_pane_reuse.go +69 -0
| @@ -0,0 +1,69 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "time" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 7 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | type relayPaneRecord struct { | |
| 11 | + | paneID entity.PaneID | |
| 12 | + | openedAt time.Time | |
| 13 | + | } | |
| 14 | + | ||
| 15 | + | // RelayPaneReuse remembers panes opened for externally relayed URLs so a | |
| 16 | + | // repeated identical request (desktop apps retry OAuth URLs) reuses the pane | |
| 17 | + | // instead of creating duplicates. It is not safe for concurrent use; callers | |
| 18 | + | // confine it to the UI main thread. The zero value is ready to use. | |
| 19 | + | type RelayPaneReuse struct { | |
| 20 | + | entries map[string]relayPaneRecord | |
| 21 | + | // Now overrides the clock in tests; nil means time.Now. | |
| 22 | + | Now func() time.Time | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func (r *RelayPaneReuse) clock() time.Time { | |
| 26 | + | if r.Now != nil { | |
| 27 | + | return r.Now() | |
| 28 | + | } | |
| 29 | + | return time.Now() | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | // Lookup returns the pane to reuse for url. Entries that expired or whose pane | |
| 33 | + | // no longer exists (per exists) are forgotten first. | |
| 34 | + | func (r *RelayPaneReuse) Lookup(url string, exists func(entity.PaneID) bool) (entity.PaneID, bool) { | |
| 35 | + | now := r.clock() | |
| 36 | + | for key, rec := range r.entries { | |
| 37 | + | if now.Sub(rec.openedAt) > domainurl.RelayReuseWindow || (exists != nil && !exists(rec.paneID)) { | |
| 38 | + | delete(r.entries, key) | |
| 39 | + | } | |
| 40 | + | } | |
| 41 | + | key := domainurl.RelayURLKey(url) | |
| 42 | + | rec, ok := r.entries[key] | |
| 43 | + | if !ok || !domainurl.ShouldReuseRelayPane(key, rec.openedAt, url, now, domainurl.RelayReuseWindow) { | |
| 44 | + | return "", false | |
| 45 | + | } | |
| 46 | + | return rec.paneID, true | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | // Record remembers that paneID was created for url. | |
| 50 | + | func (r *RelayPaneReuse) Record(url string, paneID entity.PaneID) { | |
| 51 | + | key := domainurl.RelayURLKey(url) | |
| 52 | + | if key == "" || paneID == "" { | |
| 53 | + | return | |
| 54 | + | } | |
| 55 | + | if r.entries == nil { | |
| 56 | + | r.entries = make(map[string]relayPaneRecord) | |
| 57 | + | } | |
| 58 | + | r.entries[key] = relayPaneRecord{paneID: paneID, openedAt: r.clock()} | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | // Forget drops the record for url. | |
| 62 | + | func (r *RelayPaneReuse) Forget(url string) { | |
| 63 | + | delete(r.entries, domainurl.RelayURLKey(url)) | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | // Len reports how many relay panes are remembered. | |
| 67 | + | func (r *RelayPaneReuse) Len() int { | |
| 68 | + | return len(r.entries) | |
| 69 | + | } | |
| @@ -0,0 +1,69 @@ | |||||
| 1 | + | package usecase | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "time" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 7 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | type relayPaneRecord struct { | |||
| 11 | + | paneID entity.PaneID | |||
| 12 | + | openedAt time.Time | |||
| 13 | + | } | |||
| 14 | + | ||||
| 15 | + | // RelayPaneReuse remembers panes opened for externally relayed URLs so a | |||
| 16 | + | // repeated identical request (desktop apps retry OAuth URLs) reuses the pane | |||
| 17 | + | // instead of creating duplicates. It is not safe for concurrent use; callers | |||
| 18 | + | // confine it to the UI main thread. The zero value is ready to use. | |||
| 19 | + | type RelayPaneReuse struct { | |||
| 20 | + | entries map[string]relayPaneRecord | |||
| 21 | + | // Now overrides the clock in tests; nil means time.Now. | |||
| 22 | + | Now func() time.Time | |||
| 23 | + | } | |||
| 24 | + | ||||
| 25 | + | func (r *RelayPaneReuse) clock() time.Time { | |||
| 26 | + | if r.Now != nil { | |||
| 27 | + | return r.Now() | |||
| 28 | + | } | |||
| 29 | + | return time.Now() | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | // Lookup returns the pane to reuse for url. Entries that expired or whose pane | |||
| 33 | + | // no longer exists (per exists) are forgotten first. | |||
| 34 | + | func (r *RelayPaneReuse) Lookup(url string, exists func(entity.PaneID) bool) (entity.PaneID, bool) { | |||
| 35 | + | now := r.clock() | |||
| 36 | + | for key, rec := range r.entries { | |||
| 37 | + | if now.Sub(rec.openedAt) > domainurl.RelayReuseWindow || (exists != nil && !exists(rec.paneID)) { | |||
| 38 | + | delete(r.entries, key) | |||
| 39 | + | } | |||
| 40 | + | } | |||
| 41 | + | key := domainurl.RelayURLKey(url) | |||
| 42 | + | rec, ok := r.entries[key] | |||
| 43 | + | if !ok || !domainurl.ShouldReuseRelayPane(key, rec.openedAt, url, now, domainurl.RelayReuseWindow) { | |||
| 44 | + | return "", false | |||
| 45 | + | } | |||
| 46 | + | return rec.paneID, true | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | // Record remembers that paneID was created for url. | |||
| 50 | + | func (r *RelayPaneReuse) Record(url string, paneID entity.PaneID) { | |||
| 51 | + | key := domainurl.RelayURLKey(url) | |||
| 52 | + | if key == "" || paneID == "" { | |||
| 53 | + | return | |||
| 54 | + | } | |||
| 55 | + | if r.entries == nil { | |||
| 56 | + | r.entries = make(map[string]relayPaneRecord) | |||
| 57 | + | } | |||
| 58 | + | r.entries[key] = relayPaneRecord{paneID: paneID, openedAt: r.clock()} | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | // Forget drops the record for url. | |||
| 62 | + | func (r *RelayPaneReuse) Forget(url string) { | |||
| 63 | + | delete(r.entries, domainurl.RelayURLKey(url)) | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | // Len reports how many relay panes are remembered. | |||
| 67 | + | func (r *RelayPaneReuse) Len() int { | |||
| 68 | + | return len(r.entries) | |||
| 69 | + | } | |||
A internal/application/usecase/relay_pane_reuse_test.go +32 -0
| @@ -0,0 +1,32 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | "time" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 10 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | func TestRelayPaneReuse_PrunesExpiredAndMissingPanes(t *testing.T) { | |
| 14 | + | now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) | |
| 15 | + | reg := RelayPaneReuse{Now: func() time.Time { return now }} | |
| 16 | + | reg.Record("https://a.example/", "pane-a") | |
| 17 | + | reg.Record("https://b.example/", "pane-b") | |
| 18 | + | ||
| 19 | + | exists := func(id entity.PaneID) bool { return id != "pane-b" } | |
| 20 | + | _, ok := reg.Lookup("https://b.example/", exists) | |
| 21 | + | assert.False(t, ok) | |
| 22 | + | assert.Equal(t, 1, reg.Len()) | |
| 23 | + | ||
| 24 | + | id, ok := reg.Lookup("https://a.example/", exists) | |
| 25 | + | assert.True(t, ok) | |
| 26 | + | assert.Equal(t, entity.PaneID("pane-a"), id) | |
| 27 | + | ||
| 28 | + | now = now.Add(domainurl.RelayReuseWindow + time.Second) | |
| 29 | + | _, ok = reg.Lookup("https://a.example/", exists) | |
| 30 | + | assert.False(t, ok) | |
| 31 | + | assert.Equal(t, 0, reg.Len()) | |
| 32 | + | } | |
| @@ -0,0 +1,32 @@ | |||||
| 1 | + | package usecase | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | "time" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/assert" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 10 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | func TestRelayPaneReuse_PrunesExpiredAndMissingPanes(t *testing.T) { | |||
| 14 | + | now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) | |||
| 15 | + | reg := RelayPaneReuse{Now: func() time.Time { return now }} | |||
| 16 | + | reg.Record("https://a.example/", "pane-a") | |||
| 17 | + | reg.Record("https://b.example/", "pane-b") | |||
| 18 | + | ||||
| 19 | + | exists := func(id entity.PaneID) bool { return id != "pane-b" } | |||
| 20 | + | _, ok := reg.Lookup("https://b.example/", exists) | |||
| 21 | + | assert.False(t, ok) | |||
| 22 | + | assert.Equal(t, 1, reg.Len()) | |||
| 23 | + | ||||
| 24 | + | id, ok := reg.Lookup("https://a.example/", exists) | |||
| 25 | + | assert.True(t, ok) | |||
| 26 | + | assert.Equal(t, entity.PaneID("pane-a"), id) | |||
| 27 | + | ||||
| 28 | + | now = now.Add(domainurl.RelayReuseWindow + time.Second) | |||
| 29 | + | _, ok = reg.Lookup("https://a.example/", exists) | |||
| 30 | + | assert.False(t, ok) | |||
| 31 | + | assert.Equal(t, 0, reg.Len()) | |||
| 32 | + | } | |||
A internal/domain/url/relay_reuse.go +33 -0
| @@ -0,0 +1,33 @@ | |||
| 1 | + | package url | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "strings" | |
| 5 | + | "time" | |
| 6 | + | ) | |
| 7 | + | ||
| 8 | + | // RelayReuseWindow is how long a pane opened for an externally relayed URL | |
| 9 | + | // remains eligible to be reused when the same URL is relayed again. | |
| 10 | + | const RelayReuseWindow = 2 * time.Minute | |
| 11 | + | ||
| 12 | + | // RelayURLKey returns the canonical key identifying a relayed URL for reuse | |
| 13 | + | // decisions. | |
| 14 | + | func RelayURLKey(raw string) string { | |
| 15 | + | return Normalize(strings.TrimSpace(raw)) | |
| 16 | + | } | |
| 17 | + | ||
| 18 | + | // ShouldReuseRelayPane reports whether a pane that was opened for recordedURL | |
| 19 | + | // at openedAt should be reused for requestedURL at now, instead of creating a | |
| 20 | + | // new pane. It is true only when both URLs are non-empty and identical after | |
| 21 | + | // normalization and the pane is no older than window. A pane recorded in the | |
| 22 | + | // future (clock anomaly) or with a non-positive window is never reused. | |
| 23 | + | func ShouldReuseRelayPane(recordedURL string, openedAt time.Time, requestedURL string, now time.Time, window time.Duration) bool { | |
| 24 | + | if window <= 0 { | |
| 25 | + | return false | |
| 26 | + | } | |
| 27 | + | requested := RelayURLKey(requestedURL) | |
| 28 | + | if requested == "" || requested != RelayURLKey(recordedURL) { | |
| 29 | + | return false | |
| 30 | + | } | |
| 31 | + | age := now.Sub(openedAt) | |
| 32 | + | return age >= 0 && age <= window | |
| 33 | + | } | |
| @@ -0,0 +1,33 @@ | |||||
| 1 | + | package url | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "strings" | |||
| 5 | + | "time" | |||
| 6 | + | ) | |||
| 7 | + | ||||
| 8 | + | // RelayReuseWindow is how long a pane opened for an externally relayed URL | |||
| 9 | + | // remains eligible to be reused when the same URL is relayed again. | |||
| 10 | + | const RelayReuseWindow = 2 * time.Minute | |||
| 11 | + | ||||
| 12 | + | // RelayURLKey returns the canonical key identifying a relayed URL for reuse | |||
| 13 | + | // decisions. | |||
| 14 | + | func RelayURLKey(raw string) string { | |||
| 15 | + | return Normalize(strings.TrimSpace(raw)) | |||
| 16 | + | } | |||
| 17 | + | ||||
| 18 | + | // ShouldReuseRelayPane reports whether a pane that was opened for recordedURL | |||
| 19 | + | // at openedAt should be reused for requestedURL at now, instead of creating a | |||
| 20 | + | // new pane. It is true only when both URLs are non-empty and identical after | |||
| 21 | + | // normalization and the pane is no older than window. A pane recorded in the | |||
| 22 | + | // future (clock anomaly) or with a non-positive window is never reused. | |||
| 23 | + | func ShouldReuseRelayPane(recordedURL string, openedAt time.Time, requestedURL string, now time.Time, window time.Duration) bool { | |||
| 24 | + | if window <= 0 { | |||
| 25 | + | return false | |||
| 26 | + | } | |||
| 27 | + | requested := RelayURLKey(requestedURL) | |||
| 28 | + | if requested == "" || requested != RelayURLKey(recordedURL) { | |||
| 29 | + | return false | |||
| 30 | + | } | |||
| 31 | + | age := now.Sub(openedAt) | |||
| 32 | + | return age >= 0 && age <= window | |||
| 33 | + | } | |||
A internal/domain/url/relay_reuse_test.go +45 -0
| @@ -0,0 +1,45 @@ | |||
| 1 | + | package url | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | "time" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestShouldReuseRelayPane(t *testing.T) { | |
| 11 | + | const u = "https://claude.com/cai/login/app-google-auth?open_in_browser=1&hop_nonce=XYZ" | |
| 12 | + | opened := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) | |
| 13 | + | ||
| 14 | + | tests := []struct { | |
| 15 | + | name string | |
| 16 | + | recorded string | |
| 17 | + | requested string | |
| 18 | + | now time.Time | |
| 19 | + | window time.Duration | |
| 20 | + | want bool | |
| 21 | + | }{ | |
| 22 | + | {"same url within window", u, u, opened.Add(30 * time.Second), RelayReuseWindow, true}, | |
| 23 | + | {"same url at exact window boundary", u, u, opened.Add(RelayReuseWindow), RelayReuseWindow, true}, | |
| 24 | + | {"same url just after window", u, u, opened.Add(RelayReuseWindow + time.Nanosecond), RelayReuseWindow, false}, | |
| 25 | + | {"different nonce", u, "https://claude.com/cai/login/app-google-auth?open_in_browser=1&hop_nonce=ABC", opened.Add(time.Second), RelayReuseWindow, false}, | |
| 26 | + | {"different host", u, "https://example.com/", opened.Add(time.Second), RelayReuseWindow, false}, | |
| 27 | + | {"equivalent after normalization", "example.com/path", "https://example.com/path", opened.Add(time.Second), RelayReuseWindow, true}, | |
| 28 | + | {"surrounding whitespace ignored", u, " " + u + "\n", opened.Add(time.Second), RelayReuseWindow, true}, | |
| 29 | + | {"empty requested", "", "", opened, RelayReuseWindow, false}, | |
| 30 | + | {"empty recorded", "", u, opened, RelayReuseWindow, false}, | |
| 31 | + | {"recorded in the future", u, u, opened.Add(-time.Second), RelayReuseWindow, false}, | |
| 32 | + | {"zero window", u, u, opened, 0, false}, | |
| 33 | + | {"negative window", u, u, opened, -time.Second, false}, | |
| 34 | + | } | |
| 35 | + | for _, tt := range tests { | |
| 36 | + | t.Run(tt.name, func(t *testing.T) { | |
| 37 | + | assert.Equal(t, tt.want, ShouldReuseRelayPane(tt.recorded, opened, tt.requested, tt.now, tt.window)) | |
| 38 | + | }) | |
| 39 | + | } | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | func TestRelayURLKey_Normalizes(t *testing.T) { | |
| 43 | + | assert.Equal(t, "https://example.com", RelayURLKey(" example.com ")) | |
| 44 | + | assert.Empty(t, RelayURLKey(" ")) | |
| 45 | + | } | |
| @@ -0,0 +1,45 @@ | |||||
| 1 | + | package url | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | "time" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/assert" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func TestShouldReuseRelayPane(t *testing.T) { | |||
| 11 | + | const u = "https://claude.com/cai/login/app-google-auth?open_in_browser=1&hop_nonce=XYZ" | |||
| 12 | + | opened := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) | |||
| 13 | + | ||||
| 14 | + | tests := []struct { | |||
| 15 | + | name string | |||
| 16 | + | recorded string | |||
| 17 | + | requested string | |||
| 18 | + | now time.Time | |||
| 19 | + | window time.Duration | |||
| 20 | + | want bool | |||
| 21 | + | }{ | |||
| 22 | + | {"same url within window", u, u, opened.Add(30 * time.Second), RelayReuseWindow, true}, | |||
| 23 | + | {"same url at exact window boundary", u, u, opened.Add(RelayReuseWindow), RelayReuseWindow, true}, | |||
| 24 | + | {"same url just after window", u, u, opened.Add(RelayReuseWindow + time.Nanosecond), RelayReuseWindow, false}, | |||
| 25 | + | {"different nonce", u, "https://claude.com/cai/login/app-google-auth?open_in_browser=1&hop_nonce=ABC", opened.Add(time.Second), RelayReuseWindow, false}, | |||
| 26 | + | {"different host", u, "https://example.com/", opened.Add(time.Second), RelayReuseWindow, false}, | |||
| 27 | + | {"equivalent after normalization", "example.com/path", "https://example.com/path", opened.Add(time.Second), RelayReuseWindow, true}, | |||
| 28 | + | {"surrounding whitespace ignored", u, " " + u + "\n", opened.Add(time.Second), RelayReuseWindow, true}, | |||
| 29 | + | {"empty requested", "", "", opened, RelayReuseWindow, false}, | |||
| 30 | + | {"empty recorded", "", u, opened, RelayReuseWindow, false}, | |||
| 31 | + | {"recorded in the future", u, u, opened.Add(-time.Second), RelayReuseWindow, false}, | |||
| 32 | + | {"zero window", u, u, opened, 0, false}, | |||
| 33 | + | {"negative window", u, u, opened, -time.Second, false}, | |||
| 34 | + | } | |||
| 35 | + | for _, tt := range tests { | |||
| 36 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 37 | + | assert.Equal(t, tt.want, ShouldReuseRelayPane(tt.recorded, opened, tt.requested, tt.now, tt.window)) | |||
| 38 | + | }) | |||
| 39 | + | } | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | func TestRelayURLKey_Normalizes(t *testing.T) { | |||
| 43 | + | assert.Equal(t, "https://example.com", RelayURLKey(" example.com ")) | |||
| 44 | + | assert.Empty(t, RelayURLKey(" ")) | |||
| 45 | + | } | |||
M internal/infrastructure/cef/webview.go +24 -0
| @@ -1214,6 +1214,30 @@ func (wv *WebView) handleNativePopupAborted() { | |||
| 1214 | 1214 | wv.mu.Unlock() | |
| 1215 | 1215 | } | |
| 1216 | 1216 | ||
| 1217 | + | // releaseUnrecoverableAbortedPopup destroys a pane-hosted popup shell whose | |
| 1218 | + | // CEF popup was aborted and that can no longer obtain a browser. While | |
| 1219 | + | // pendingCreate is set the shell still recovers through direct browser | |
| 1220 | + | // creation (see preparePopupShellDirectBrowserCreation), so it is left alone. | |
| 1221 | + | // Destroy runs the lifecycle close callback registered by the content layer, | |
| 1222 | + | // which drops its pending-popup and OAuth state, closes the hidden pane and | |
| 1223 | + | // releases the view. Shells owned by a native popup host (nativePopupHostAbort | |
| 1224 | + | // installed) or without a content-layer close callback are never touched: the | |
| 1225 | + | // host owns their teardown. | |
| 1226 | + | func (wv *WebView) releaseUnrecoverableAbortedPopup() bool { | |
| 1227 | + | if wv == nil { | |
| 1228 | + | return false | |
| 1229 | + | } | |
| 1230 | + | wv.mu.RLock() | |
| 1231 | + | release := wv.nativePopupHostAbort == nil && wv.popupLifecycleClose != nil && | |
| 1232 | + | wv.browser == nil && wv.pendingCreate == nil | |
| 1233 | + | wv.mu.RUnlock() | |
| 1234 | + | if !release { | |
| 1235 | + | return false | |
| 1236 | + | } | |
| 1237 | + | wv.Destroy() | |
| 1238 | + | return true | |
| 1239 | + | } | |
| 1240 | + | ||
| 1217 | 1241 | // --------------------------------------------------------------------------- | |
| 1218 | 1242 | // JavaScript / Appearance | |
| 1219 | 1243 | // --------------------------------------------------------------------------- | |
| @@ -1214,6 +1214,30 @@ func (wv *WebView) handleNativePopupAborted() { | |||||
| 1214 | wv.mu.Unlock() | 1214 | wv.mu.Unlock() | ||
| 1215 | } | 1215 | } | ||
| 1216 | 1216 | ||||
| 1217 | + | // releaseUnrecoverableAbortedPopup destroys a pane-hosted popup shell whose | |||
| 1218 | + | // CEF popup was aborted and that can no longer obtain a browser. While | |||
| 1219 | + | // pendingCreate is set the shell still recovers through direct browser | |||
| 1220 | + | // creation (see preparePopupShellDirectBrowserCreation), so it is left alone. | |||
| 1221 | + | // Destroy runs the lifecycle close callback registered by the content layer, | |||
| 1222 | + | // which drops its pending-popup and OAuth state, closes the hidden pane and | |||
| 1223 | + | // releases the view. Shells owned by a native popup host (nativePopupHostAbort | |||
| 1224 | + | // installed) or without a content-layer close callback are never touched: the | |||
| 1225 | + | // host owns their teardown. | |||
| 1226 | + | func (wv *WebView) releaseUnrecoverableAbortedPopup() bool { | |||
| 1227 | + | if wv == nil { | |||
| 1228 | + | return false | |||
| 1229 | + | } | |||
| 1230 | + | wv.mu.RLock() | |||
| 1231 | + | release := wv.nativePopupHostAbort == nil && wv.popupLifecycleClose != nil && | |||
| 1232 | + | wv.browser == nil && wv.pendingCreate == nil | |||
| 1233 | + | wv.mu.RUnlock() | |||
| 1234 | + | if !release { | |||
| 1235 | + | return false | |||
| 1236 | + | } | |||
| 1237 | + | wv.Destroy() | |||
| 1238 | + | return true | |||
| 1239 | + | } | |||
| 1240 | + | ||||
| 1217 | // --------------------------------------------------------------------------- | 1241 | // --------------------------------------------------------------------------- | ||
| 1218 | // JavaScript / Appearance | 1242 | // JavaScript / Appearance | ||
| 1219 | // --------------------------------------------------------------------------- | 1243 | // --------------------------------------------------------------------------- | ||
M internal/infrastructure/cef/webview_handlers.go +15 -2
| @@ -903,9 +903,22 @@ func (h *handlerSet) handlePopupHostDecision( | |||
| 903 | 903 | } | |
| 904 | 904 | } | |
| 905 | 905 | ||
| 906 | + | // OnBeforePopupAborted is keyed by CEF's popup_id, which only maps to a popup | |
| 907 | + | // shell through the parent's pendingNativePopups. The content layer keys its | |
| 908 | + | // pending popup and OAuth state by the shell's WebViewID, so cleanup of a shell | |
| 909 | + | // that can no longer get a browser goes through its lifecycle close callback | |
| 910 | + | // (content handlePopupClose) rather than by id. Recoverable shells keep using | |
| 911 | + | // the direct-creation fallback. | |
| 906 | 912 | func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) { | |
| 907 | - | if popup := h.wv.takePendingNativePopup(popupID); popup != nil { | |
| 908 | - | popup.handleNativePopupAborted() | |
| 913 | + | popup := h.wv.takePendingNativePopup(popupID) | |
| 914 | + | if popup == nil { | |
| 915 | + | return | |
| 916 | + | } | |
| 917 | + | popup.handleNativePopupAborted() | |
| 918 | + | if popup.releaseUnrecoverableAbortedPopup() { | |
| 919 | + | logging.FromContext(h.currentContext()).Debug(). | |
| 920 | + | Int32("popup_id", popupID). | |
| 921 | + | Msg("cef: popup aborted and unrecoverable, released popup shell") | |
| 909 | 922 | } | |
| 910 | 923 | } | |
| 911 | 924 | ||
| @@ -903,9 +903,22 @@ func (h *handlerSet) handlePopupHostDecision( | |||||
| 903 | } | 903 | } | ||
| 904 | } | 904 | } | ||
| 905 | 905 | ||||
| 906 | + | // OnBeforePopupAborted is keyed by CEF's popup_id, which only maps to a popup | |||
| 907 | + | // shell through the parent's pendingNativePopups. The content layer keys its | |||
| 908 | + | // pending popup and OAuth state by the shell's WebViewID, so cleanup of a shell | |||
| 909 | + | // that can no longer get a browser goes through its lifecycle close callback | |||
| 910 | + | // (content handlePopupClose) rather than by id. Recoverable shells keep using | |||
| 911 | + | // the direct-creation fallback. | |||
| 906 | func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) { | 912 | func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) { | ||
| 907 | - | if popup := h.wv.takePendingNativePopup(popupID); popup != nil { | 913 | + | popup := h.wv.takePendingNativePopup(popupID) |
| 908 | - | popup.handleNativePopupAborted() | 914 | + | if popup == nil { |
| 915 | + | return | |||
| 916 | + | } | |||
| 917 | + | popup.handleNativePopupAborted() | |||
| 918 | + | if popup.releaseUnrecoverableAbortedPopup() { | |||
| 919 | + | logging.FromContext(h.currentContext()).Debug(). | |||
| 920 | + | Int32("popup_id", popupID). | |||
| 921 | + | Msg("cef: popup aborted and unrecoverable, released popup shell") | |||
| 909 | } | 922 | } | ||
| 910 | } | 923 | } | ||
| 911 | 924 | ||||
M internal/infrastructure/cef/webview_popup_lifecycle_test.go +50 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package cef | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/stretchr/testify/assert" | |
| @@ -24,3 +25,52 @@ func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) { | |||
| 24 | 25 | assert.Zero(t, lifecycleCloseCalls) | |
| 25 | 26 | assert.Equal(t, 1, oauthCloseCalls) | |
| 26 | 27 | } | |
| 28 | + | ||
| 29 | + | func TestOnBeforePopupAborted_ReleasesUnrecoverablePaneHostedPopup(t *testing.T) { | |
| 30 | + | parent := &WebView{ctx: context.Background()} | |
| 31 | + | popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 77} | |
| 32 | + | closeCalls := 0 | |
| 33 | + | popup.SetOnClose(func() { closeCalls++ }) | |
| 34 | + | parent.trackPendingNativePopup(77, popup) | |
| 35 | + | ||
| 36 | + | (&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 77) | |
| 37 | + | ||
| 38 | + | assert.Equal(t, 1, closeCalls) | |
| 39 | + | assert.True(t, popup.IsDestroyed()) | |
| 40 | + | assert.Nil(t, parent.takePendingNativePopup(77)) | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | func TestOnBeforePopupAborted_KeepsRecoverablePopupForDirectFallback(t *testing.T) { | |
| 44 | + | parent := &WebView{ctx: context.Background()} | |
| 45 | + | popup := &WebView{ | |
| 46 | + | ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 78, | |
| 47 | + | pendingCreate: &pendingBrowserCreate{}, | |
| 48 | + | } | |
| 49 | + | closeCalls := 0 | |
| 50 | + | popup.SetOnClose(func() { closeCalls++ }) | |
| 51 | + | parent.trackPendingNativePopup(78, popup) | |
| 52 | + | ||
| 53 | + | (&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 78) | |
| 54 | + | ||
| 55 | + | assert.Zero(t, closeCalls) | |
| 56 | + | assert.False(t, popup.IsDestroyed()) | |
| 57 | + | assert.False(t, popup.isNativePopupCandidate()) | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | func TestOnBeforePopupAborted_LeavesNativeHostedPopupToHost(t *testing.T) { | |
| 61 | + | parent := &WebView{ctx: context.Background()} | |
| 62 | + | popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 79} | |
| 63 | + | closeCalls := 0 | |
| 64 | + | popup.SetOnClose(func() { closeCalls++ }) | |
| 65 | + | popup.SetNativePopupHostAbort(func() {}) | |
| 66 | + | parent.trackPendingNativePopup(79, popup) | |
| 67 | + | ||
| 68 | + | (&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 79) | |
| 69 | + | ||
| 70 | + | assert.Zero(t, closeCalls) | |
| 71 | + | assert.False(t, popup.IsDestroyed()) | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | func TestOnBeforePopupAborted_UnknownPopupIsNoop(_ *testing.T) { | |
| 75 | + | (&handlerSet{wv: &WebView{ctx: context.Background()}}).OnBeforePopupAborted(nil, 123) | |
| 76 | + | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package cef | 1 | package cef | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/stretchr/testify/assert" | 7 | "github.com/stretchr/testify/assert" | ||
| @@ -24,3 +25,52 @@ func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) { | |||||
| 24 | assert.Zero(t, lifecycleCloseCalls) | 25 | assert.Zero(t, lifecycleCloseCalls) | ||
| 25 | assert.Equal(t, 1, oauthCloseCalls) | 26 | assert.Equal(t, 1, oauthCloseCalls) | ||
| 26 | } | 27 | } | ||
| 28 | + | ||||
| 29 | + | func TestOnBeforePopupAborted_ReleasesUnrecoverablePaneHostedPopup(t *testing.T) { | |||
| 30 | + | parent := &WebView{ctx: context.Background()} | |||
| 31 | + | popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 77} | |||
| 32 | + | closeCalls := 0 | |||
| 33 | + | popup.SetOnClose(func() { closeCalls++ }) | |||
| 34 | + | parent.trackPendingNativePopup(77, popup) | |||
| 35 | + | ||||
| 36 | + | (&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 77) | |||
| 37 | + | ||||
| 38 | + | assert.Equal(t, 1, closeCalls) | |||
| 39 | + | assert.True(t, popup.IsDestroyed()) | |||
| 40 | + | assert.Nil(t, parent.takePendingNativePopup(77)) | |||
| 41 | + | } | |||
| 42 | + | ||||
| 43 | + | func TestOnBeforePopupAborted_KeepsRecoverablePopupForDirectFallback(t *testing.T) { | |||
| 44 | + | parent := &WebView{ctx: context.Background()} | |||
| 45 | + | popup := &WebView{ | |||
| 46 | + | ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 78, | |||
| 47 | + | pendingCreate: &pendingBrowserCreate{}, | |||
| 48 | + | } | |||
| 49 | + | closeCalls := 0 | |||
| 50 | + | popup.SetOnClose(func() { closeCalls++ }) | |||
| 51 | + | parent.trackPendingNativePopup(78, popup) | |||
| 52 | + | ||||
| 53 | + | (&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 78) | |||
| 54 | + | ||||
| 55 | + | assert.Zero(t, closeCalls) | |||
| 56 | + | assert.False(t, popup.IsDestroyed()) | |||
| 57 | + | assert.False(t, popup.isNativePopupCandidate()) | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | func TestOnBeforePopupAborted_LeavesNativeHostedPopupToHost(t *testing.T) { | |||
| 61 | + | parent := &WebView{ctx: context.Background()} | |||
| 62 | + | popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 79} | |||
| 63 | + | closeCalls := 0 | |||
| 64 | + | popup.SetOnClose(func() { closeCalls++ }) | |||
| 65 | + | popup.SetNativePopupHostAbort(func() {}) | |||
| 66 | + | parent.trackPendingNativePopup(79, popup) | |||
| 67 | + | ||||
| 68 | + | (&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 79) | |||
| 69 | + | ||||
| 70 | + | assert.Zero(t, closeCalls) | |||
| 71 | + | assert.False(t, popup.IsDestroyed()) | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | func TestOnBeforePopupAborted_UnknownPopupIsNoop(_ *testing.T) { | |||
| 75 | + | (&handlerSet{wv: &WebView{ctx: context.Background()}}).OnBeforePopupAborted(nil, 123) | |||
| 76 | + | } | |||
M internal/ui/app.go +24 -2
| @@ -90,8 +90,9 @@ type App struct { | |||
| 90 | 90 | runtimeConfig *runtimeConfigState | |
| 91 | 91 | ||
| 92 | 92 | browserWindows map[string]*browserWindow | |
| 93 | - | browserWindowOrder []string // registration order of window IDs | |
| 94 | - | instanceWindows map[string]string // logical instance name -> browser window ID | |
| 93 | + | browserWindowOrder []string // registration order of window IDs | |
| 94 | + | instanceWindows map[string]string // logical instance name -> browser window ID | |
| 95 | + | externalRelayPanes usecase.RelayPaneReuse // panes opened by the external URL relay (main thread only) | |
| 95 | 96 | lastFocusedWindowID string | |
| 96 | 97 | nativePopupWindows map[port.WebViewID]*nativePopupWindow | |
| 97 | 98 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | |
| @@ -3207,6 +3208,7 @@ func (a *App) initContentCoordinator( | |||
| 3207 | 3208 | // Wire external URL launcher (e.g. xdg-open for vscode://, spotify://) | |
| 3208 | 3209 | if a.deps.LaunchExternalURL != nil { | |
| 3209 | 3210 | a.contentCoord.SetOnLaunchExternalURL(a.deps.LaunchExternalURL) | |
| 3211 | + | a.contentCoord.SetOnConfirmExternalLaunch(a.confirmExternalLaunch) | |
| 3210 | 3212 | } | |
| 3211 | 3213 | ||
| 3212 | 3214 | a.contentCoord.SetOnTouchpadNavigationGesture(func(paneID entity.PaneID, gesture entity.TouchpadNavigationGesture) { | |
| @@ -3601,6 +3603,26 @@ func (a *App) wireKeyboardActions() { | |||
| 3601 | 3603 | }) | |
| 3602 | 3604 | } | |
| 3603 | 3605 | ||
| 3606 | + | // externalLaunchConfirmer is implemented by the window permission dialog. | |
| 3607 | + | type externalLaunchConfirmer interface { | |
| 3608 | + | ShowExternalLaunchConfirm(ctx context.Context, origin, uri string, callback func(allowed bool)) | |
| 3609 | + | } | |
| 3610 | + | ||
| 3611 | + | // confirmExternalLaunch asks, in the window owning paneID, whether uri may be | |
| 3612 | + | // opened by an external application. Without a window it denies. | |
| 3613 | + | func (a *App) confirmExternalLaunch(paneID entity.PaneID, origin, uri string, respond func(allowed bool)) { | |
| 3614 | + | ctx := context.Background() | |
| 3615 | + | if a.deps != nil && a.deps.Ctx != nil { | |
| 3616 | + | ctx = a.deps.Ctx | |
| 3617 | + | } | |
| 3618 | + | confirmer, ok := a.permissionDialogForPane(paneID).(externalLaunchConfirmer) | |
| 3619 | + | if !ok { | |
| 3620 | + | respond(false) | |
| 3621 | + | return | |
| 3622 | + | } | |
| 3623 | + | confirmer.ShowExternalLaunchConfirm(ctx, origin, uri, respond) | |
| 3624 | + | } | |
| 3625 | + | ||
| 3604 | 3626 | // permissionDialogForPane returns the permission dialog of the window owning the | |
| 3605 | 3627 | // pane (including floating panes), or nil when no window or dialog is available. | |
| 3606 | 3628 | func (a *App) permissionDialogForPane(paneID entity.PaneID) port.PermissionDialogPresenter { | |
| @@ -90,8 +90,9 @@ type App struct { | |||||
| 90 | runtimeConfig *runtimeConfigState | 90 | runtimeConfig *runtimeConfigState | ||
| 91 | 91 | ||||
| 92 | browserWindows map[string]*browserWindow | 92 | browserWindows map[string]*browserWindow | ||
| 93 | - | browserWindowOrder []string // registration order of window IDs | 93 | + | browserWindowOrder []string // registration order of window IDs |
| 94 | - | instanceWindows map[string]string // logical instance name -> browser window ID | 94 | + | instanceWindows map[string]string // logical instance name -> browser window ID |
| 95 | + | externalRelayPanes usecase.RelayPaneReuse // panes opened by the external URL relay (main thread only) | |||
| 95 | lastFocusedWindowID string | 96 | lastFocusedWindowID string | ||
| 96 | nativePopupWindows map[port.WebViewID]*nativePopupWindow | 97 | nativePopupWindows map[port.WebViewID]*nativePopupWindow | ||
| 97 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | 98 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | ||
| @@ -3207,6 +3208,7 @@ func (a *App) initContentCoordinator( | |||||
| 3207 | // Wire external URL launcher (e.g. xdg-open for vscode://, spotify://) | 3208 | // Wire external URL launcher (e.g. xdg-open for vscode://, spotify://) | ||
| 3208 | if a.deps.LaunchExternalURL != nil { | 3209 | if a.deps.LaunchExternalURL != nil { | ||
| 3209 | a.contentCoord.SetOnLaunchExternalURL(a.deps.LaunchExternalURL) | 3210 | a.contentCoord.SetOnLaunchExternalURL(a.deps.LaunchExternalURL) | ||
| 3211 | + | a.contentCoord.SetOnConfirmExternalLaunch(a.confirmExternalLaunch) | |||
| 3210 | } | 3212 | } | ||
| 3211 | 3213 | ||||
| 3212 | a.contentCoord.SetOnTouchpadNavigationGesture(func(paneID entity.PaneID, gesture entity.TouchpadNavigationGesture) { | 3214 | a.contentCoord.SetOnTouchpadNavigationGesture(func(paneID entity.PaneID, gesture entity.TouchpadNavigationGesture) { | ||
| @@ -3601,6 +3603,26 @@ func (a *App) wireKeyboardActions() { | |||||
| 3601 | }) | 3603 | }) | ||
| 3602 | } | 3604 | } | ||
| 3603 | 3605 | ||||
| 3606 | + | // externalLaunchConfirmer is implemented by the window permission dialog. | |||
| 3607 | + | type externalLaunchConfirmer interface { | |||
| 3608 | + | ShowExternalLaunchConfirm(ctx context.Context, origin, uri string, callback func(allowed bool)) | |||
| 3609 | + | } | |||
| 3610 | + | ||||
| 3611 | + | // confirmExternalLaunch asks, in the window owning paneID, whether uri may be | |||
| 3612 | + | // opened by an external application. Without a window it denies. | |||
| 3613 | + | func (a *App) confirmExternalLaunch(paneID entity.PaneID, origin, uri string, respond func(allowed bool)) { | |||
| 3614 | + | ctx := context.Background() | |||
| 3615 | + | if a.deps != nil && a.deps.Ctx != nil { | |||
| 3616 | + | ctx = a.deps.Ctx | |||
| 3617 | + | } | |||
| 3618 | + | confirmer, ok := a.permissionDialogForPane(paneID).(externalLaunchConfirmer) | |||
| 3619 | + | if !ok { | |||
| 3620 | + | respond(false) | |||
| 3621 | + | return | |||
| 3622 | + | } | |||
| 3623 | + | confirmer.ShowExternalLaunchConfirm(ctx, origin, uri, respond) | |||
| 3624 | + | } | |||
| 3625 | + | ||||
| 3604 | // permissionDialogForPane returns the permission dialog of the window owning the | 3626 | // permissionDialogForPane returns the permission dialog of the window owning the | ||
| 3605 | // pane (including floating panes), or nil when no window or dialog is available. | 3627 | // pane (including floating panes), or nil when no window or dialog is available. | ||
| 3606 | func (a *App) permissionDialogForPane(paneID entity.PaneID) port.PermissionDialogPresenter { | 3628 | func (a *App) permissionDialogForPane(paneID entity.PaneID) port.PermissionDialogPresenter { | ||
M internal/ui/browser_window.go +1 -1
| @@ -634,7 +634,7 @@ func (a *App) dispatchWindowURLWork(ctx context.Context, label, url string, work | |||
| 634 | 634 | func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |
| 635 | 635 | url = domainurl.Normalize(url) | |
| 636 | 636 | return a.dispatchWindowURLWork(ctx, "ui.open_external_url", url, func(ctx context.Context) error { | |
| 637 | - | return a.openExternalURLOnMainThread(ctx, url, a.runtimeConfigSnapshot()) | |
| 637 | + | return a.openRelayURLOnMainThread(ctx, url, a.runtimeConfigSnapshot()) | |
| 638 | 638 | }) | |
| 639 | 639 | } | |
| 640 | 640 | ||
| @@ -634,7 +634,7 @@ func (a *App) dispatchWindowURLWork(ctx context.Context, label, url string, work | |||||
| 634 | func (a *App) OpenExternalURL(ctx context.Context, url string) error { | 634 | func (a *App) OpenExternalURL(ctx context.Context, url string) error { | ||
| 635 | url = domainurl.Normalize(url) | 635 | url = domainurl.Normalize(url) | ||
| 636 | return a.dispatchWindowURLWork(ctx, "ui.open_external_url", url, func(ctx context.Context) error { | 636 | return a.dispatchWindowURLWork(ctx, "ui.open_external_url", url, func(ctx context.Context) error { | ||
| 637 | - | return a.openExternalURLOnMainThread(ctx, url, a.runtimeConfigSnapshot()) | 637 | + | return a.openRelayURLOnMainThread(ctx, url, a.runtimeConfigSnapshot()) |
| 638 | }) | 638 | }) | ||
| 639 | } | 639 | } | ||
| 640 | 640 | ||||
M internal/ui/coordinator/content/callbacks.go +1 -1
| @@ -89,7 +89,7 @@ func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.P | |||
| 89 | 89 | c.onProgressChanged(paneID, wv, identity, progress) | |
| 90 | 90 | }, | |
| 91 | 91 | OnURIChanged: func(uri string) { | |
| 92 | - | c.handleURIChanged(ctx, paneID, wv, uri) | |
| 92 | + | c.handleURIChanged(ctx, paneID, wv, identity, uri) | |
| 93 | 93 | }, | |
| 94 | 94 | OnLinkHover: func(uri string) { | |
| 95 | 95 | c.onLinkHover(paneID, uri) | |
| @@ -89,7 +89,7 @@ func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.P | |||||
| 89 | c.onProgressChanged(paneID, wv, identity, progress) | 89 | c.onProgressChanged(paneID, wv, identity, progress) | ||
| 90 | }, | 90 | }, | ||
| 91 | OnURIChanged: func(uri string) { | 91 | OnURIChanged: func(uri string) { | ||
| 92 | - | c.handleURIChanged(ctx, paneID, wv, uri) | 92 | + | c.handleURIChanged(ctx, paneID, wv, identity, uri) |
| 93 | }, | 93 | }, | ||
| 94 | OnLinkHover: func(uri string) { | 94 | OnLinkHover: func(uri string) { | ||
| 95 | c.onLinkHover(paneID, uri) | 95 | c.onLinkHover(paneID, uri) | ||
M internal/ui/coordinator/content/coordinator.go +17 -0
| @@ -134,6 +134,17 @@ type Coordinator struct { | |||
| 134 | 134 | // Callback to open a URL with the system's default handler (e.g. xdg-open). | |
| 135 | 135 | // Used for external URL schemes like vscode://, spotify://, etc. | |
| 136 | 136 | onLaunchExternalURL func(uri string) | |
| 137 | + | // Callback asking the user to confirm an external scheme launch for a pane. | |
| 138 | + | // respond must be called exactly once on the GTK thread. | |
| 139 | + | // origin is the host of the requesting page, empty when unknown. | |
| 140 | + | onConfirmExternalLaunch func(paneID entity.PaneID, origin, uri string, respond func(allowed bool)) | |
| 141 | + | ||
| 142 | + | // externalPromptMu guards the external-launch prompt state: outstanding | |
| 143 | + | // prompts per pane, the token sequence, and the last web host per pane. | |
| 144 | + | externalPromptMu sync.Mutex | |
| 145 | + | externalPrompts map[entity.PaneID]externalPrompt | |
| 146 | + | externalPromptSeq uint64 | |
| 147 | + | externalPromptOrigins map[entity.PaneID]string | |
| 137 | 148 | } | |
| 138 | 149 | ||
| 139 | 150 | type pendingThemeUpdate struct { | |
| @@ -307,6 +318,12 @@ func (c *Coordinator) SetOnLaunchExternalURL(fn func(uri string)) { | |||
| 307 | 318 | c.onLaunchExternalURL = fn | |
| 308 | 319 | } | |
| 309 | 320 | ||
| 321 | + | // SetOnConfirmExternalLaunch sets the callback that asks the user whether an | |
| 322 | + | // external URL scheme reached by page navigation may be opened. | |
| 323 | + | func (c *Coordinator) SetOnConfirmExternalLaunch(fn func(paneID entity.PaneID, origin, uri string, respond func(allowed bool))) { | |
| 324 | + | c.onConfirmExternalLaunch = fn | |
| 325 | + | } | |
| 326 | + | ||
| 310 | 327 | // SetSettingsApplier sets the engine settings applier for config hot-reload. | |
| 311 | 328 | func (c *Coordinator) SetSettingsApplier(sa port.SettingsApplier) { | |
| 312 | 329 | c.settingsApplier = sa | |
| @@ -134,6 +134,17 @@ type Coordinator struct { | |||||
| 134 | // Callback to open a URL with the system's default handler (e.g. xdg-open). | 134 | // Callback to open a URL with the system's default handler (e.g. xdg-open). | ||
| 135 | // Used for external URL schemes like vscode://, spotify://, etc. | 135 | // Used for external URL schemes like vscode://, spotify://, etc. | ||
| 136 | onLaunchExternalURL func(uri string) | 136 | onLaunchExternalURL func(uri string) | ||
| 137 | + | // Callback asking the user to confirm an external scheme launch for a pane. | |||
| 138 | + | // respond must be called exactly once on the GTK thread. | |||
| 139 | + | // origin is the host of the requesting page, empty when unknown. | |||
| 140 | + | onConfirmExternalLaunch func(paneID entity.PaneID, origin, uri string, respond func(allowed bool)) | |||
| 141 | + | ||||
| 142 | + | // externalPromptMu guards the external-launch prompt state: outstanding | |||
| 143 | + | // prompts per pane, the token sequence, and the last web host per pane. | |||
| 144 | + | externalPromptMu sync.Mutex | |||
| 145 | + | externalPrompts map[entity.PaneID]externalPrompt | |||
| 146 | + | externalPromptSeq uint64 | |||
| 147 | + | externalPromptOrigins map[entity.PaneID]string | |||
| 137 | } | 148 | } | ||
| 138 | 149 | ||||
| 139 | type pendingThemeUpdate struct { | 150 | type pendingThemeUpdate struct { | ||
| @@ -307,6 +318,12 @@ func (c *Coordinator) SetOnLaunchExternalURL(fn func(uri string)) { | |||||
| 307 | c.onLaunchExternalURL = fn | 318 | c.onLaunchExternalURL = fn | ||
| 308 | } | 319 | } | ||
| 309 | 320 | ||||
| 321 | + | // SetOnConfirmExternalLaunch sets the callback that asks the user whether an | |||
| 322 | + | // external URL scheme reached by page navigation may be opened. | |||
| 323 | + | func (c *Coordinator) SetOnConfirmExternalLaunch(fn func(paneID entity.PaneID, origin, uri string, respond func(allowed bool))) { | |||
| 324 | + | c.onConfirmExternalLaunch = fn | |||
| 325 | + | } | |||
| 326 | + | ||||
| 310 | // SetSettingsApplier sets the engine settings applier for config hot-reload. | 327 | // SetSettingsApplier sets the engine settings applier for config hot-reload. | ||
| 311 | func (c *Coordinator) SetSettingsApplier(sa port.SettingsApplier) { | 328 | func (c *Coordinator) SetSettingsApplier(sa port.SettingsApplier) { | ||
| 312 | c.settingsApplier = sa | 329 | c.settingsApplier = sa | ||
M internal/ui/coordinator/content/lifecycle.go +1 -0
| @@ -49,6 +49,7 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||
| 49 | 49 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | |
| 50 | 50 | log := logging.FromContext(ctx) | |
| 51 | 51 | c.ensurePopupManager().cancelDeferredPopupsForParent(paneID) | |
| 52 | + | c.clearExternalPrompt(paneID) | |
| 52 | 53 | ||
| 53 | 54 | // deleteWebViewLocked clears reveal state before returning, even when a | |
| 54 | 55 | // previous cleanup already removed this pane's mapping. | |
| @@ -49,6 +49,7 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||||
| 49 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | 49 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | ||
| 50 | log := logging.FromContext(ctx) | 50 | log := logging.FromContext(ctx) | ||
| 51 | c.ensurePopupManager().cancelDeferredPopupsForParent(paneID) | 51 | c.ensurePopupManager().cancelDeferredPopupsForParent(paneID) | ||
| 52 | + | c.clearExternalPrompt(paneID) | |||
| 52 | 53 | ||||
| 53 | // deleteWebViewLocked clears reveal state before returning, even when a | 54 | // deleteWebViewLocked clears reveal state before returning, even when a | ||
| 54 | // previous cleanup already removed this pane's mapping. | 55 | // previous cleanup already removed this pane's mapping. | ||
M internal/ui/coordinator/content/navigation.go +102 -13
| @@ -2,6 +2,7 @@ package content | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | neturl "net/url" | |
| 5 | 6 | "strings" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/dumber/internal/application/port" | |
| @@ -387,10 +388,100 @@ func (c *Coordinator) onLinkHover(paneID entity.PaneID, uri string) { | |||
| 387 | 388 | } | |
| 388 | 389 | } | |
| 389 | 390 | ||
| 391 | + | // externalPrompt is a pane's outstanding external-launch confirmation. | |
| 392 | + | type externalPrompt struct { | |
| 393 | + | token uint64 | |
| 394 | + | identity webViewIdentity | |
| 395 | + | } | |
| 396 | + | ||
| 397 | + | // promptExternalLaunch asks the user whether the external URI may be handed to | |
| 398 | + | // the system handler. A pane has at most one outstanding prompt: further | |
| 399 | + | // external URIs from that pane are dropped until it is answered, so a page | |
| 400 | + | // cannot flood the window's prompt queue. | |
| 401 | + | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, identity webViewIdentity, uri string) { | |
| 402 | + | log := logging.FromContext(ctx) | |
| 403 | + | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | |
| 404 | + | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | |
| 405 | + | return | |
| 406 | + | } | |
| 407 | + | ||
| 408 | + | c.externalPromptMu.Lock() | |
| 409 | + | if _, pending := c.externalPrompts[paneID]; pending { | |
| 410 | + | c.externalPromptMu.Unlock() | |
| 411 | + | return | |
| 412 | + | } | |
| 413 | + | if c.externalPrompts == nil { | |
| 414 | + | c.externalPrompts = make(map[entity.PaneID]externalPrompt) | |
| 415 | + | } | |
| 416 | + | c.externalPromptSeq++ | |
| 417 | + | token := c.externalPromptSeq | |
| 418 | + | c.externalPrompts[paneID] = externalPrompt{token: token, identity: identity} | |
| 419 | + | origin := c.externalPromptOrigins[paneID] | |
| 420 | + | c.externalPromptMu.Unlock() | |
| 421 | + | ||
| 422 | + | c.onConfirmExternalLaunch(paneID, origin, uri, func(allowed bool) { | |
| 423 | + | c.externalPromptMu.Lock() | |
| 424 | + | prompt, pending := c.externalPrompts[paneID] | |
| 425 | + | // The prompt is stale when its pane was released, rebound, or | |
| 426 | + | // re-prompted while it was queued: never launch for it. | |
| 427 | + | current := pending && prompt.token == token | |
| 428 | + | if current { | |
| 429 | + | delete(c.externalPrompts, paneID) | |
| 430 | + | } | |
| 431 | + | c.externalPromptMu.Unlock() | |
| 432 | + | current = current && c.isBoundWebView(paneID, identity) | |
| 433 | + | ||
| 434 | + | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). | |
| 435 | + | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | |
| 436 | + | Msg("external scheme launch decision") | |
| 437 | + | if allowed && current { | |
| 438 | + | c.onLaunchExternalURL(uri) | |
| 439 | + | } | |
| 440 | + | }) | |
| 441 | + | } | |
| 442 | + | ||
| 443 | + | // rememberExternalPromptOrigin records the host of the http(s) document shown | |
| 444 | + | // in the pane, so a later external-launch prompt can name the requesting site. | |
| 445 | + | // Any other document clears it: the prompt then says "A page" rather than | |
| 446 | + | // attributing the request to a previous site. | |
| 447 | + | func (c *Coordinator) rememberExternalPromptOrigin(paneID entity.PaneID, uri string) { | |
| 448 | + | host := "" | |
| 449 | + | if parsed, err := neturl.Parse(uri); err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") { | |
| 450 | + | host = parsed.Hostname() | |
| 451 | + | } | |
| 452 | + | c.externalPromptMu.Lock() | |
| 453 | + | defer c.externalPromptMu.Unlock() | |
| 454 | + | if host == "" { | |
| 455 | + | delete(c.externalPromptOrigins, paneID) | |
| 456 | + | return | |
| 457 | + | } | |
| 458 | + | if c.externalPromptOrigins == nil { | |
| 459 | + | c.externalPromptOrigins = make(map[entity.PaneID]string) | |
| 460 | + | } | |
| 461 | + | c.externalPromptOrigins[paneID] = host | |
| 462 | + | } | |
| 463 | + | ||
| 464 | + | // isBoundWebView reports whether identity is still the WebView bound to the | |
| 465 | + | // pane. Engine callbacks are posted asynchronously and can arrive after the | |
| 466 | + | // pane was released or rebound. | |
| 467 | + | func (c *Coordinator) isBoundWebView(paneID entity.PaneID, identity webViewIdentity) bool { | |
| 468 | + | bound, ok := identityForWebView(c.GetWebView(paneID)) | |
| 469 | + | return ok && bound == identity | |
| 470 | + | } | |
| 471 | + | ||
| 472 | + | // clearExternalPrompt invalidates the pane's outstanding external-launch | |
| 473 | + | // prompt so a late answer cannot launch anything, and forgets its origin. | |
| 474 | + | func (c *Coordinator) clearExternalPrompt(paneID entity.PaneID) { | |
| 475 | + | c.externalPromptMu.Lock() | |
| 476 | + | delete(c.externalPrompts, paneID) | |
| 477 | + | delete(c.externalPromptOrigins, paneID) | |
| 478 | + | c.externalPromptMu.Unlock() | |
| 479 | + | } | |
| 480 | + | ||
| 390 | 481 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | |
| 391 | 482 | // and SPA navigation tracking. | |
| 392 | - | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | |
| 393 | - | if uri == "" { | |
| 483 | + | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri string) { | |
| 484 | + | if uri == "" || !c.isBoundWebView(paneID, identity) { | |
| 394 | 485 | return | |
| 395 | 486 | } | |
| 396 | 487 | ||
| @@ -401,24 +492,22 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||
| 401 | 492 | isExternal := urlutil.IsExternalScheme(uri) | |
| 402 | 493 | ||
| 403 | 494 | if isExternal { | |
| 404 | - | log.Info().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external scheme URI change blocked") | |
| 495 | + | log.Info().Str("pane_id", string(paneID)).Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | |
| 496 | + | Msg("external scheme URI change intercepted") | |
| 405 | 497 | ||
| 406 | - | // External schemes must only be launched from WebKit's navigation-policy path, | |
| 407 | - | // where NavigationAction.IsUserGesture is available and verified. URI changes can | |
| 408 | - | // be driven by page script or redirects, so this path only cleans up WebKit state. | |
| 409 | 498 | if err := wv.Stop(ctx); err != nil { | |
| 410 | - | log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for blocked external URL") | |
| 499 | + | log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("stop webview for external URL") | |
| 411 | 500 | } | |
| 412 | 501 | ||
| 413 | - | // Navigate back to avoid stale URI in omnibox/history. | |
| 414 | - | if wv.CanGoBack() { | |
| 415 | - | if err := wv.GoBack(ctx); err != nil { | |
| 416 | - | log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("GoBack after blocked external URL") | |
| 417 | - | } | |
| 418 | - | } | |
| 502 | + | // URI changes can be driven by page script or redirects (OAuth flows end | |
| 503 | + | // with a redirect to claude://, vscode://, ...), so the launch is never | |
| 504 | + | // automatic: the user must confirm it. Do not navigate back: going back | |
| 505 | + | // to an OAuth endpoint re-issues the redirect and loops. | |
| 506 | + | c.promptExternalLaunch(ctx, paneID, identity, uri) | |
| 419 | 507 | return | |
| 420 | 508 | } | |
| 421 | 509 | ||
| 510 | + | c.rememberExternalPromptOrigin(paneID, uri) | |
| 422 | 511 | if !wv.IsLoading() { | |
| 423 | 512 | c.onSPANavigation(ctx, paneID, uri) | |
| 424 | 513 | } | |
| @@ -2,6 +2,7 @@ package content | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | neturl "net/url" | |||
| 5 | "strings" | 6 | "strings" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/dumber/internal/application/port" | 8 | "github.com/bnema/dumber/internal/application/port" | ||
| @@ -387,10 +388,100 @@ func (c *Coordinator) onLinkHover(paneID entity.PaneID, uri string) { | |||||
| 387 | } | 388 | } | ||
| 388 | } | 389 | } | ||
| 389 | 390 | ||||
| 391 | + | // externalPrompt is a pane's outstanding external-launch confirmation. | |||
| 392 | + | type externalPrompt struct { | |||
| 393 | + | token uint64 | |||
| 394 | + | identity webViewIdentity | |||
| 395 | + | } | |||
| 396 | + | ||||
| 397 | + | // promptExternalLaunch asks the user whether the external URI may be handed to | |||
| 398 | + | // the system handler. A pane has at most one outstanding prompt: further | |||
| 399 | + | // external URIs from that pane are dropped until it is answered, so a page | |||
| 400 | + | // cannot flood the window's prompt queue. | |||
| 401 | + | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, identity webViewIdentity, uri string) { | |||
| 402 | + | log := logging.FromContext(ctx) | |||
| 403 | + | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | |||
| 404 | + | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | |||
| 405 | + | return | |||
| 406 | + | } | |||
| 407 | + | ||||
| 408 | + | c.externalPromptMu.Lock() | |||
| 409 | + | if _, pending := c.externalPrompts[paneID]; pending { | |||
| 410 | + | c.externalPromptMu.Unlock() | |||
| 411 | + | return | |||
| 412 | + | } | |||
| 413 | + | if c.externalPrompts == nil { | |||
| 414 | + | c.externalPrompts = make(map[entity.PaneID]externalPrompt) | |||
| 415 | + | } | |||
| 416 | + | c.externalPromptSeq++ | |||
| 417 | + | token := c.externalPromptSeq | |||
| 418 | + | c.externalPrompts[paneID] = externalPrompt{token: token, identity: identity} | |||
| 419 | + | origin := c.externalPromptOrigins[paneID] | |||
| 420 | + | c.externalPromptMu.Unlock() | |||
| 421 | + | ||||
| 422 | + | c.onConfirmExternalLaunch(paneID, origin, uri, func(allowed bool) { | |||
| 423 | + | c.externalPromptMu.Lock() | |||
| 424 | + | prompt, pending := c.externalPrompts[paneID] | |||
| 425 | + | // The prompt is stale when its pane was released, rebound, or | |||
| 426 | + | // re-prompted while it was queued: never launch for it. | |||
| 427 | + | current := pending && prompt.token == token | |||
| 428 | + | if current { | |||
| 429 | + | delete(c.externalPrompts, paneID) | |||
| 430 | + | } | |||
| 431 | + | c.externalPromptMu.Unlock() | |||
| 432 | + | current = current && c.isBoundWebView(paneID, identity) | |||
| 433 | + | ||||
| 434 | + | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). | |||
| 435 | + | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | |||
| 436 | + | Msg("external scheme launch decision") | |||
| 437 | + | if allowed && current { | |||
| 438 | + | c.onLaunchExternalURL(uri) | |||
| 439 | + | } | |||
| 440 | + | }) | |||
| 441 | + | } | |||
| 442 | + | ||||
| 443 | + | // rememberExternalPromptOrigin records the host of the http(s) document shown | |||
| 444 | + | // in the pane, so a later external-launch prompt can name the requesting site. | |||
| 445 | + | // Any other document clears it: the prompt then says "A page" rather than | |||
| 446 | + | // attributing the request to a previous site. | |||
| 447 | + | func (c *Coordinator) rememberExternalPromptOrigin(paneID entity.PaneID, uri string) { | |||
| 448 | + | host := "" | |||
| 449 | + | if parsed, err := neturl.Parse(uri); err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") { | |||
| 450 | + | host = parsed.Hostname() | |||
| 451 | + | } | |||
| 452 | + | c.externalPromptMu.Lock() | |||
| 453 | + | defer c.externalPromptMu.Unlock() | |||
| 454 | + | if host == "" { | |||
| 455 | + | delete(c.externalPromptOrigins, paneID) | |||
| 456 | + | return | |||
| 457 | + | } | |||
| 458 | + | if c.externalPromptOrigins == nil { | |||
| 459 | + | c.externalPromptOrigins = make(map[entity.PaneID]string) | |||
| 460 | + | } | |||
| 461 | + | c.externalPromptOrigins[paneID] = host | |||
| 462 | + | } | |||
| 463 | + | ||||
| 464 | + | // isBoundWebView reports whether identity is still the WebView bound to the | |||
| 465 | + | // pane. Engine callbacks are posted asynchronously and can arrive after the | |||
| 466 | + | // pane was released or rebound. | |||
| 467 | + | func (c *Coordinator) isBoundWebView(paneID entity.PaneID, identity webViewIdentity) bool { | |||
| 468 | + | bound, ok := identityForWebView(c.GetWebView(paneID)) | |||
| 469 | + | return ok && bound == identity | |||
| 470 | + | } | |||
| 471 | + | ||||
| 472 | + | // clearExternalPrompt invalidates the pane's outstanding external-launch | |||
| 473 | + | // prompt so a late answer cannot launch anything, and forgets its origin. | |||
| 474 | + | func (c *Coordinator) clearExternalPrompt(paneID entity.PaneID) { | |||
| 475 | + | c.externalPromptMu.Lock() | |||
| 476 | + | delete(c.externalPrompts, paneID) | |||
| 477 | + | delete(c.externalPromptOrigins, paneID) | |||
| 478 | + | c.externalPromptMu.Unlock() | |||
| 479 | + | } | |||
| 480 | + | ||||
| 390 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | 481 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | ||
| 391 | // and SPA navigation tracking. | 482 | // and SPA navigation tracking. | ||
| 392 | - | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | 483 | + | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri string) { |
| 393 | - | if uri == "" { | 484 | + | if uri == "" || !c.isBoundWebView(paneID, identity) { |
| 394 | return | 485 | return | ||
| 395 | } | 486 | } | ||
| 396 | 487 | ||||
| @@ -401,24 +492,22 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||||
| 401 | isExternal := urlutil.IsExternalScheme(uri) | 492 | isExternal := urlutil.IsExternalScheme(uri) | ||
| 402 | 493 | ||||
| 403 | if isExternal { | 494 | if isExternal { | ||
| 404 | - | log.Info().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external scheme URI change blocked") | 495 | + | log.Info().Str("pane_id", string(paneID)).Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). |
| 496 | + | Msg("external scheme URI change intercepted") | |||
| 405 | 497 | ||||
| 406 | - | // External schemes must only be launched from WebKit's navigation-policy path, | |||
| 407 | - | // where NavigationAction.IsUserGesture is available and verified. URI changes can | |||
| 408 | - | // be driven by page script or redirects, so this path only cleans up WebKit state. | |||
| 409 | if err := wv.Stop(ctx); err != nil { | 498 | if err := wv.Stop(ctx); err != nil { | ||
| 410 | - | log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for blocked external URL") | 499 | + | log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("stop webview for external URL") |
| 411 | } | 500 | } | ||
| 412 | 501 | ||||
| 413 | - | // Navigate back to avoid stale URI in omnibox/history. | 502 | + | // URI changes can be driven by page script or redirects (OAuth flows end |
| 414 | - | if wv.CanGoBack() { | 503 | + | // with a redirect to claude://, vscode://, ...), so the launch is never |
| 415 | - | if err := wv.GoBack(ctx); err != nil { | 504 | + | // automatic: the user must confirm it. Do not navigate back: going back |
| 416 | - | log.Warn().Str("pane_id", string(paneID)).Err(err).Msg("GoBack after blocked external URL") | 505 | + | // to an OAuth endpoint re-issues the redirect and loops. |
| 417 | - | } | 506 | + | c.promptExternalLaunch(ctx, paneID, identity, uri) |
| 418 | - | } | |||
| 419 | return | 507 | return | ||
| 420 | } | 508 | } | ||
| 421 | 509 | ||||
| 510 | + | c.rememberExternalPromptOrigin(paneID, uri) | |||
| 422 | if !wv.IsLoading() { | 511 | if !wv.IsLoading() { | ||
| 423 | c.onSPANavigation(ctx, paneID, uri) | 512 | c.onSPANavigation(ctx, paneID, uri) | ||
| 424 | } | 513 | } | ||
M internal/ui/coordinator/content/navigation_security_test.go +248 -47
| @@ -6,72 +6,169 @@ import ( | |||
| 6 | 6 | "testing" | |
| 7 | 7 | ||
| 8 | 8 | "github.com/stretchr/testify/assert" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 9 | 11 | ||
| 12 | + | "github.com/bnema/dumber/internal/application/port" | |
| 10 | 13 | "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | 14 | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | 15 | ) | |
| 13 | 16 | ||
| 14 | - | func TestHandleURIChanged_BlocksExternalSchemeWithoutLaunching(t *testing.T) { | |
| 17 | + | type externalLaunchRecorder struct { | |
| 18 | + | prompts []string | |
| 19 | + | origins []string | |
| 20 | + | respond func(bool) | |
| 21 | + | launched []string | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | const testPaneID = entity.PaneID("pane-1") | |
| 25 | + | ||
| 26 | + | // uriChanged delivers a URI change to testPaneID as the engine callback | |
| 27 | + | // installed for wv. | |
| 28 | + | func uriChanged(ctx context.Context, c *Coordinator, wv port.WebView, uri string) { | |
| 29 | + | identity, _ := identityForWebView(wv) | |
| 30 | + | c.handleURIChanged(ctx, testPaneID, wv, identity, uri) | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | // externalWebView returns a WebView mock with ID 1 and the given generation. | |
| 34 | + | func externalWebView(t *testing.T, generation uint64) *mocks.MockWebView { | |
| 35 | + | wv := mocks.NewMockWebView(t) | |
| 36 | + | wv.EXPECT().ID().Return(port.WebViewID(1)).Maybe() | |
| 37 | + | wv.EXPECT().Generation().Return(generation).Maybe() | |
| 38 | + | return wv | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | func (r *externalLaunchRecorder) install(c *Coordinator) { | |
| 42 | + | c.onLaunchExternalURL = func(uri string) { | |
| 43 | + | r.launched = append(r.launched, uri) | |
| 44 | + | } | |
| 45 | + | c.onConfirmExternalLaunch = func(_ entity.PaneID, origin, uri string, respond func(bool)) { | |
| 46 | + | r.prompts = append(r.prompts, uri) | |
| 47 | + | r.origins = append(r.origins, origin) | |
| 48 | + | r.respond = respond | |
| 49 | + | } | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | func TestHandleURIChanged_ExternalSchemeWithoutConfirmerIsBlocked(t *testing.T) { | |
| 15 | 53 | t.Parallel() | |
| 16 | 54 | ||
| 17 | 55 | ctx := context.Background() | |
| 18 | - | paneID := entity.PaneID("pane-1") | |
| 19 | - | wv := mocks.NewMockWebView(t) | |
| 56 | + | wv := externalWebView(t, 1) | |
| 20 | 57 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 21 | - | wv.EXPECT().CanGoBack().Return(false).Once() | |
| 22 | 58 | ||
| 23 | 59 | launchCount := 0 | |
| 24 | 60 | c := &Coordinator{ | |
| 25 | - | onLaunchExternalURL: func(uri string) { | |
| 26 | - | launchCount++ | |
| 27 | - | }, | |
| 61 | + | webViews: map[entity.PaneID]port.WebView{"pane-1": wv}, | |
| 62 | + | onLaunchExternalURL: func(string) { launchCount++ }, | |
| 28 | 63 | } | |
| 29 | 64 | ||
| 30 | - | c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd") | |
| 65 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") | |
| 31 | 66 | ||
| 32 | 67 | assert.Equal(t, 0, launchCount) | |
| 33 | 68 | } | |
| 34 | 69 | ||
| 35 | - | func TestHandleURIChanged_BlocksExternalSchemeRedirectAndRestoresPreviousPage(t *testing.T) { | |
| 70 | + | func TestHandleURIChanged_ExternalSchemeLaunchesOnlyAfterConfirmation(t *testing.T) { | |
| 36 | 71 | t.Parallel() | |
| 37 | 72 | ||
| 38 | 73 | ctx := context.Background() | |
| 39 | - | paneID := entity.PaneID("pane-1") | |
| 40 | - | wv := mocks.NewMockWebView(t) | |
| 74 | + | wv := externalWebView(t, 1) | |
| 41 | 75 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 42 | - | wv.EXPECT().CanGoBack().Return(true).Once() | |
| 43 | - | wv.EXPECT().GoBack(ctx).Return(nil).Once() | |
| 44 | 76 | ||
| 45 | - | launchCount := 0 | |
| 46 | - | c := &Coordinator{ | |
| 47 | - | onLaunchExternalURL: func(uri string) { | |
| 48 | - | launchCount++ | |
| 49 | - | }, | |
| 50 | - | } | |
| 77 | + | rec := &externalLaunchRecorder{} | |
| 78 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 79 | + | rec.install(c) | |
| 51 | 80 | ||
| 52 | - | c.handleURIChanged(ctx, paneID, wv, "spotify://track/123") | |
| 81 | + | uri := "claude://login/google-auth?code=abc" | |
| 82 | + | uriChanged(ctx, c, wv, uri) | |
| 53 | 83 | ||
| 54 | - | assert.Equal(t, 0, launchCount) | |
| 84 | + | assert.Equal(t, []string{uri}, rec.prompts) | |
| 85 | + | assert.Empty(t, rec.launched) | |
| 86 | + | ||
| 87 | + | rec.respond(true) | |
| 88 | + | assert.Equal(t, []string{uri}, rec.launched) | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T) { | |
| 92 | + | t.Parallel() | |
| 93 | + | ||
| 94 | + | ctx := context.Background() | |
| 95 | + | wv := externalWebView(t, 1) | |
| 96 | + | // GoBack must not be called: going back to an OAuth endpoint loops. | |
| 97 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 98 | + | ||
| 99 | + | rec := &externalLaunchRecorder{} | |
| 100 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 101 | + | rec.install(c) | |
| 102 | + | ||
| 103 | + | uriChanged(ctx, c, wv, "spotify://track/123") | |
| 104 | + | rec.respond(false) | |
| 105 | + | ||
| 106 | + | assert.Empty(t, rec.launched) | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | func TestHandleURIChanged_OneOutstandingPromptPerPane(t *testing.T) { | |
| 110 | + | t.Parallel() | |
| 111 | + | ||
| 112 | + | ctx := context.Background() | |
| 113 | + | wv := externalWebView(t, 1) | |
| 114 | + | wv.EXPECT().Stop(ctx).Return(nil).Times(3) | |
| 115 | + | ||
| 116 | + | rec := &externalLaunchRecorder{} | |
| 117 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 118 | + | rec.install(c) | |
| 119 | + | ||
| 120 | + | // Alternating URIs must not queue more prompts while one is pending. | |
| 121 | + | uriChanged(ctx, c, wv, "claude://a") | |
| 122 | + | uriChanged(ctx, c, wv, "claude://b") | |
| 123 | + | uriChanged(ctx, c, wv, "claude://a") | |
| 124 | + | assert.Equal(t, []string{"claude://a"}, rec.prompts) | |
| 125 | + | ||
| 126 | + | // Once answered, the pane may prompt again. | |
| 127 | + | rec.respond(false) | |
| 128 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 129 | + | uriChanged(ctx, c, wv, "claude://b") | |
| 130 | + | assert.Len(t, rec.prompts, 2) | |
| 131 | + | } | |
| 132 | + | ||
| 133 | + | func TestHandleURIChanged_StalePromptAfterReleaseDoesNotLaunch(t *testing.T) { | |
| 134 | + | t.Parallel() | |
| 135 | + | ||
| 136 | + | ctx := context.Background() | |
| 137 | + | wv := externalWebView(t, 1) | |
| 138 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 139 | + | ||
| 140 | + | rec := &externalLaunchRecorder{} | |
| 141 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 142 | + | rec.install(c) | |
| 143 | + | ||
| 144 | + | uriChanged(ctx, c, wv, "claude://login?code=x") | |
| 145 | + | // Pane closed while the prompt was queued. | |
| 146 | + | c.clearExternalPrompt("pane-1") | |
| 147 | + | delete(c.webViews, "pane-1") | |
| 148 | + | ||
| 149 | + | rec.respond(true) | |
| 150 | + | assert.Empty(t, rec.launched) | |
| 55 | 151 | } | |
| 56 | 152 | ||
| 57 | 153 | func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | |
| 58 | 154 | t.Parallel() | |
| 59 | 155 | ||
| 60 | 156 | ctx := context.Background() | |
| 61 | - | paneID := entity.PaneID("pane-1") | |
| 62 | - | wv := mocks.NewMockWebView(t) | |
| 157 | + | paneID := testPaneID | |
| 158 | + | wv := externalWebView(t, 1) | |
| 63 | 159 | wv.EXPECT().IsLoading().Return(false).Once() | |
| 64 | 160 | ||
| 65 | 161 | var recordedPane entity.PaneID | |
| 66 | 162 | var recordedURI string | |
| 67 | 163 | c := &Coordinator{ | |
| 164 | + | webViews: map[entity.PaneID]port.WebView{paneID: wv}, | |
| 68 | 165 | onPaneURIUpdated: func(paneID entity.PaneID, uri string) { | |
| 69 | 166 | recordedPane = paneID | |
| 70 | 167 | recordedURI = uri | |
| 71 | 168 | }, | |
| 72 | 169 | } | |
| 73 | 170 | ||
| 74 | - | c.handleURIChanged(ctx, paneID, wv, "https://example.com/app#state") | |
| 171 | + | uriChanged(ctx, c, wv, "https://example.com/app#state") | |
| 75 | 172 | ||
| 76 | 173 | assert.Equal(t, paneID, recordedPane) | |
| 77 | 174 | assert.Equal(t, "https://example.com/app#state", recordedURI) | |
| @@ -81,41 +178,145 @@ func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) { | |||
| 81 | 178 | t.Parallel() | |
| 82 | 179 | ||
| 83 | 180 | ctx := context.Background() | |
| 84 | - | paneID := entity.PaneID("pane-1") | |
| 85 | - | wv := mocks.NewMockWebView(t) | |
| 181 | + | wv := externalWebView(t, 1) | |
| 86 | 182 | wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once() | |
| 87 | - | wv.EXPECT().CanGoBack().Return(false).Once() | |
| 88 | 183 | ||
| 89 | - | launchCount := 0 | |
| 90 | - | c := &Coordinator{ | |
| 91 | - | onLaunchExternalURL: func(uri string) { | |
| 92 | - | launchCount++ | |
| 93 | - | }, | |
| 94 | - | } | |
| 184 | + | rec := &externalLaunchRecorder{} | |
| 185 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 186 | + | rec.install(c) | |
| 95 | 187 | ||
| 96 | - | c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd") | |
| 188 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") | |
| 97 | 189 | ||
| 98 | - | assert.Equal(t, 0, launchCount) | |
| 190 | + | assert.Len(t, rec.prompts, 1) | |
| 191 | + | assert.Empty(t, rec.launched) | |
| 99 | 192 | } | |
| 100 | 193 | ||
| 101 | - | func TestHandleURIChanged_GoBackErrorDoesNotPanic(t *testing.T) { | |
| 194 | + | func TestHandleURIChanged_OldAnswerCannotLaunchReplacementPrompt(t *testing.T) { | |
| 102 | 195 | t.Parallel() | |
| 103 | 196 | ||
| 104 | 197 | ctx := context.Background() | |
| 105 | - | paneID := entity.PaneID("pane-1") | |
| 106 | - | wv := mocks.NewMockWebView(t) | |
| 198 | + | wv := externalWebView(t, 1) | |
| 199 | + | wv.EXPECT().Stop(ctx).Return(nil).Twice() | |
| 200 | + | ||
| 201 | + | rec := &externalLaunchRecorder{} | |
| 202 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 203 | + | rec.install(c) | |
| 204 | + | ||
| 205 | + | uriChanged(ctx, c, wv, "claude://old") | |
| 206 | + | oldRespond := rec.respond | |
| 207 | + | // The pane is invalidated, then the same WebView asks again. | |
| 208 | + | c.clearExternalPrompt("pane-1") | |
| 209 | + | uriChanged(ctx, c, wv, "claude://new") | |
| 210 | + | newRespond := rec.respond | |
| 211 | + | ||
| 212 | + | oldRespond(true) | |
| 213 | + | assert.Empty(t, rec.launched, "old answer must not launch") | |
| 214 | + | ||
| 215 | + | newRespond(true) | |
| 216 | + | assert.Equal(t, []string{"claude://new"}, rec.launched) | |
| 217 | + | } | |
| 218 | + | ||
| 219 | + | func TestHandleURIChanged_RebindToNewGenerationMakesPromptStale(t *testing.T) { | |
| 220 | + | t.Parallel() | |
| 221 | + | ||
| 222 | + | ctx := context.Background() | |
| 223 | + | wv := externalWebView(t, 1) | |
| 107 | 224 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 108 | - | wv.EXPECT().CanGoBack().Return(true).Once() | |
| 109 | - | wv.EXPECT().GoBack(ctx).Return(errors.New("goback failed")).Once() | |
| 225 | + | rebound := externalWebView(t, 2) | |
| 110 | 226 | ||
| 111 | - | launchCount := 0 | |
| 112 | - | c := &Coordinator{ | |
| 113 | - | onLaunchExternalURL: func(uri string) { | |
| 114 | - | launchCount++ | |
| 115 | - | }, | |
| 227 | + | rec := &externalLaunchRecorder{} | |
| 228 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 229 | + | rec.install(c) | |
| 230 | + | ||
| 231 | + | uriChanged(ctx, c, wv, "claude://login?code=x") | |
| 232 | + | c.webViews["pane-1"] = rebound | |
| 233 | + | ||
| 234 | + | rec.respond(true) | |
| 235 | + | assert.Empty(t, rec.launched) | |
| 236 | + | } | |
| 237 | + | ||
| 238 | + | func TestHandleURIChanged_PromptNamesLastWebOrigin(t *testing.T) { | |
| 239 | + | t.Parallel() | |
| 240 | + | ||
| 241 | + | ctx := context.Background() | |
| 242 | + | wv := externalWebView(t, 1) | |
| 243 | + | wv.EXPECT().IsLoading().Return(true).Once() | |
| 244 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 245 | + | ||
| 246 | + | rec := &externalLaunchRecorder{} | |
| 247 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 248 | + | rec.install(c) | |
| 249 | + | ||
| 250 | + | uriChanged(ctx, c, wv, "https://claude.ai/login/popup-google-auth?code=x") | |
| 251 | + | uriChanged(ctx, c, wv, "claude://login") | |
| 252 | + | ||
| 253 | + | assert.Equal(t, []string{"claude.ai"}, rec.origins) | |
| 254 | + | } | |
| 255 | + | ||
| 256 | + | func TestHandleURIChanged_IgnoresCallbackFromReleasedWebView(t *testing.T) { | |
| 257 | + | t.Parallel() | |
| 258 | + | ||
| 259 | + | ctx := context.Background() | |
| 260 | + | wv := externalWebView(t, 1) | |
| 261 | + | ||
| 262 | + | rec := &externalLaunchRecorder{} | |
| 263 | + | c := &Coordinator{} | |
| 264 | + | rec.install(c) | |
| 265 | + | ||
| 266 | + | // The callback was queued before the pane released its WebView. | |
| 267 | + | uriChanged(ctx, c, wv, "https://site.example/") | |
| 268 | + | uriChanged(ctx, c, wv, "claude://login") | |
| 269 | + | ||
| 270 | + | assert.Empty(t, rec.prompts) | |
| 271 | + | assert.Empty(t, c.externalPromptOrigins) | |
| 272 | + | } | |
| 273 | + | ||
| 274 | + | func TestHandleURIChanged_NonHTTPDocumentClearsOrigin(t *testing.T) { | |
| 275 | + | t.Parallel() | |
| 276 | + | ||
| 277 | + | for _, doc := range []string{"data:text/html,hi", "file://trusted.example/page.html"} { | |
| 278 | + | ctx := context.Background() | |
| 279 | + | wv := externalWebView(t, 1) | |
| 280 | + | wv.EXPECT().IsLoading().Return(true).Times(2) | |
| 281 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 282 | + | ||
| 283 | + | rec := &externalLaunchRecorder{} | |
| 284 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 285 | + | rec.install(c) | |
| 286 | + | ||
| 287 | + | uriChanged(ctx, c, wv, "https://trusted.example/") | |
| 288 | + | uriChanged(ctx, c, wv, doc) | |
| 289 | + | uriChanged(ctx, c, wv, "claude://login") | |
| 290 | + | ||
| 291 | + | assert.Equal(t, []string{""}, rec.origins, doc) | |
| 116 | 292 | } | |
| 293 | + | } | |
| 117 | 294 | ||
| 118 | - | c.handleURIChanged(ctx, paneID, wv, "spotify://track/123") | |
| 295 | + | func TestURIChangedCallback_IgnoredAfterSameWebViewIsReusedFromPool(t *testing.T) { | |
| 296 | + | ctx := context.Background() | |
| 297 | + | c := newMinimalCoordinator() | |
| 298 | + | rec := &externalLaunchRecorder{} | |
| 299 | + | rec.install(c) | |
| 300 | + | spaCalls := 0 | |
| 301 | + | c.onPaneURIUpdated = func(entity.PaneID, string) { spaCalls++ } | |
| 119 | 302 | ||
| 120 | - | assert.Equal(t, 0, launchCount) | |
| 303 | + | generation := uint64(1) | |
| 304 | + | wv := mocks.NewMockWebView(t) | |
| 305 | + | wv.EXPECT().ID().Return(port.WebViewID(1)).Maybe() | |
| 306 | + | wv.EXPECT().Generation().RunAndReturn(func() uint64 { return generation }).Maybe() | |
| 307 | + | var callbacks *port.WebViewCallbacks | |
| 308 | + | wv.EXPECT().SetCallbacks(mock.Anything).Run(func(cb *port.WebViewCallbacks) { callbacks = cb }).Once() | |
| 309 | + | ||
| 310 | + | c.setWebViewLocked(testPaneID, wv) | |
| 311 | + | c.setupWebViewCallbacks(ctx, testPaneID, wv) | |
| 312 | + | require.NotNil(t, callbacks) | |
| 313 | + | ||
| 314 | + | // The same object is reused from the pool for a new binding. | |
| 315 | + | generation = 2 | |
| 316 | + | callbacks.OnURIChanged("https://site.example/") | |
| 317 | + | callbacks.OnURIChanged("claude://login") | |
| 318 | + | ||
| 319 | + | assert.Empty(t, rec.prompts) | |
| 320 | + | assert.Empty(t, c.externalPromptOrigins) | |
| 321 | + | assert.Zero(t, spaCalls) | |
| 121 | 322 | } | |
| @@ -6,72 +6,169 @@ import ( | |||||
| 6 | "testing" | 6 | "testing" | ||
| 7 | 7 | ||||
| 8 | "github.com/stretchr/testify/assert" | 8 | "github.com/stretchr/testify/assert" | ||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 9 | 11 | ||||
| 12 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 10 | "github.com/bnema/dumber/internal/application/port/mocks" | 13 | "github.com/bnema/dumber/internal/application/port/mocks" | ||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 14 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 12 | ) | 15 | ) | ||
| 13 | 16 | ||||
| 14 | - | func TestHandleURIChanged_BlocksExternalSchemeWithoutLaunching(t *testing.T) { | 17 | + | type externalLaunchRecorder struct { |
| 18 | + | prompts []string | |||
| 19 | + | origins []string | |||
| 20 | + | respond func(bool) | |||
| 21 | + | launched []string | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | const testPaneID = entity.PaneID("pane-1") | |||
| 25 | + | ||||
| 26 | + | // uriChanged delivers a URI change to testPaneID as the engine callback | |||
| 27 | + | // installed for wv. | |||
| 28 | + | func uriChanged(ctx context.Context, c *Coordinator, wv port.WebView, uri string) { | |||
| 29 | + | identity, _ := identityForWebView(wv) | |||
| 30 | + | c.handleURIChanged(ctx, testPaneID, wv, identity, uri) | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | // externalWebView returns a WebView mock with ID 1 and the given generation. | |||
| 34 | + | func externalWebView(t *testing.T, generation uint64) *mocks.MockWebView { | |||
| 35 | + | wv := mocks.NewMockWebView(t) | |||
| 36 | + | wv.EXPECT().ID().Return(port.WebViewID(1)).Maybe() | |||
| 37 | + | wv.EXPECT().Generation().Return(generation).Maybe() | |||
| 38 | + | return wv | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | func (r *externalLaunchRecorder) install(c *Coordinator) { | |||
| 42 | + | c.onLaunchExternalURL = func(uri string) { | |||
| 43 | + | r.launched = append(r.launched, uri) | |||
| 44 | + | } | |||
| 45 | + | c.onConfirmExternalLaunch = func(_ entity.PaneID, origin, uri string, respond func(bool)) { | |||
| 46 | + | r.prompts = append(r.prompts, uri) | |||
| 47 | + | r.origins = append(r.origins, origin) | |||
| 48 | + | r.respond = respond | |||
| 49 | + | } | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | func TestHandleURIChanged_ExternalSchemeWithoutConfirmerIsBlocked(t *testing.T) { | |||
| 15 | t.Parallel() | 53 | t.Parallel() | ||
| 16 | 54 | ||||
| 17 | ctx := context.Background() | 55 | ctx := context.Background() | ||
| 18 | - | paneID := entity.PaneID("pane-1") | 56 | + | wv := externalWebView(t, 1) |
| 19 | - | wv := mocks.NewMockWebView(t) | |||
| 20 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 57 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 21 | - | wv.EXPECT().CanGoBack().Return(false).Once() | |||
| 22 | 58 | ||||
| 23 | launchCount := 0 | 59 | launchCount := 0 | ||
| 24 | c := &Coordinator{ | 60 | c := &Coordinator{ | ||
| 25 | - | onLaunchExternalURL: func(uri string) { | 61 | + | webViews: map[entity.PaneID]port.WebView{"pane-1": wv}, |
| 26 | - | launchCount++ | 62 | + | onLaunchExternalURL: func(string) { launchCount++ }, |
| 27 | - | }, | |||
| 28 | } | 63 | } | ||
| 29 | 64 | ||||
| 30 | - | c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd") | 65 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") |
| 31 | 66 | ||||
| 32 | assert.Equal(t, 0, launchCount) | 67 | assert.Equal(t, 0, launchCount) | ||
| 33 | } | 68 | } | ||
| 34 | 69 | ||||
| 35 | - | func TestHandleURIChanged_BlocksExternalSchemeRedirectAndRestoresPreviousPage(t *testing.T) { | 70 | + | func TestHandleURIChanged_ExternalSchemeLaunchesOnlyAfterConfirmation(t *testing.T) { |
| 36 | t.Parallel() | 71 | t.Parallel() | ||
| 37 | 72 | ||||
| 38 | ctx := context.Background() | 73 | ctx := context.Background() | ||
| 39 | - | paneID := entity.PaneID("pane-1") | 74 | + | wv := externalWebView(t, 1) |
| 40 | - | wv := mocks.NewMockWebView(t) | |||
| 41 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 75 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 42 | - | wv.EXPECT().CanGoBack().Return(true).Once() | |||
| 43 | - | wv.EXPECT().GoBack(ctx).Return(nil).Once() | |||
| 44 | 76 | ||||
| 45 | - | launchCount := 0 | 77 | + | rec := &externalLaunchRecorder{} |
| 46 | - | c := &Coordinator{ | 78 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 47 | - | onLaunchExternalURL: func(uri string) { | 79 | + | rec.install(c) |
| 48 | - | launchCount++ | |||
| 49 | - | }, | |||
| 50 | - | } | |||
| 51 | 80 | ||||
| 52 | - | c.handleURIChanged(ctx, paneID, wv, "spotify://track/123") | 81 | + | uri := "claude://login/google-auth?code=abc" |
| 82 | + | uriChanged(ctx, c, wv, uri) | |||
| 53 | 83 | ||||
| 54 | - | assert.Equal(t, 0, launchCount) | 84 | + | assert.Equal(t, []string{uri}, rec.prompts) |
| 85 | + | assert.Empty(t, rec.launched) | |||
| 86 | + | ||||
| 87 | + | rec.respond(true) | |||
| 88 | + | assert.Equal(t, []string{uri}, rec.launched) | |||
| 89 | + | } | |||
| 90 | + | ||||
| 91 | + | func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T) { | |||
| 92 | + | t.Parallel() | |||
| 93 | + | ||||
| 94 | + | ctx := context.Background() | |||
| 95 | + | wv := externalWebView(t, 1) | |||
| 96 | + | // GoBack must not be called: going back to an OAuth endpoint loops. | |||
| 97 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 98 | + | ||||
| 99 | + | rec := &externalLaunchRecorder{} | |||
| 100 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 101 | + | rec.install(c) | |||
| 102 | + | ||||
| 103 | + | uriChanged(ctx, c, wv, "spotify://track/123") | |||
| 104 | + | rec.respond(false) | |||
| 105 | + | ||||
| 106 | + | assert.Empty(t, rec.launched) | |||
| 107 | + | } | |||
| 108 | + | ||||
| 109 | + | func TestHandleURIChanged_OneOutstandingPromptPerPane(t *testing.T) { | |||
| 110 | + | t.Parallel() | |||
| 111 | + | ||||
| 112 | + | ctx := context.Background() | |||
| 113 | + | wv := externalWebView(t, 1) | |||
| 114 | + | wv.EXPECT().Stop(ctx).Return(nil).Times(3) | |||
| 115 | + | ||||
| 116 | + | rec := &externalLaunchRecorder{} | |||
| 117 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 118 | + | rec.install(c) | |||
| 119 | + | ||||
| 120 | + | // Alternating URIs must not queue more prompts while one is pending. | |||
| 121 | + | uriChanged(ctx, c, wv, "claude://a") | |||
| 122 | + | uriChanged(ctx, c, wv, "claude://b") | |||
| 123 | + | uriChanged(ctx, c, wv, "claude://a") | |||
| 124 | + | assert.Equal(t, []string{"claude://a"}, rec.prompts) | |||
| 125 | + | ||||
| 126 | + | // Once answered, the pane may prompt again. | |||
| 127 | + | rec.respond(false) | |||
| 128 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 129 | + | uriChanged(ctx, c, wv, "claude://b") | |||
| 130 | + | assert.Len(t, rec.prompts, 2) | |||
| 131 | + | } | |||
| 132 | + | ||||
| 133 | + | func TestHandleURIChanged_StalePromptAfterReleaseDoesNotLaunch(t *testing.T) { | |||
| 134 | + | t.Parallel() | |||
| 135 | + | ||||
| 136 | + | ctx := context.Background() | |||
| 137 | + | wv := externalWebView(t, 1) | |||
| 138 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 139 | + | ||||
| 140 | + | rec := &externalLaunchRecorder{} | |||
| 141 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 142 | + | rec.install(c) | |||
| 143 | + | ||||
| 144 | + | uriChanged(ctx, c, wv, "claude://login?code=x") | |||
| 145 | + | // Pane closed while the prompt was queued. | |||
| 146 | + | c.clearExternalPrompt("pane-1") | |||
| 147 | + | delete(c.webViews, "pane-1") | |||
| 148 | + | ||||
| 149 | + | rec.respond(true) | |||
| 150 | + | assert.Empty(t, rec.launched) | |||
| 55 | } | 151 | } | ||
| 56 | 152 | ||||
| 57 | func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | 153 | func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | ||
| 58 | t.Parallel() | 154 | t.Parallel() | ||
| 59 | 155 | ||||
| 60 | ctx := context.Background() | 156 | ctx := context.Background() | ||
| 61 | - | paneID := entity.PaneID("pane-1") | 157 | + | paneID := testPaneID |
| 62 | - | wv := mocks.NewMockWebView(t) | 158 | + | wv := externalWebView(t, 1) |
| 63 | wv.EXPECT().IsLoading().Return(false).Once() | 159 | wv.EXPECT().IsLoading().Return(false).Once() | ||
| 64 | 160 | ||||
| 65 | var recordedPane entity.PaneID | 161 | var recordedPane entity.PaneID | ||
| 66 | var recordedURI string | 162 | var recordedURI string | ||
| 67 | c := &Coordinator{ | 163 | c := &Coordinator{ | ||
| 164 | + | webViews: map[entity.PaneID]port.WebView{paneID: wv}, | |||
| 68 | onPaneURIUpdated: func(paneID entity.PaneID, uri string) { | 165 | onPaneURIUpdated: func(paneID entity.PaneID, uri string) { | ||
| 69 | recordedPane = paneID | 166 | recordedPane = paneID | ||
| 70 | recordedURI = uri | 167 | recordedURI = uri | ||
| 71 | }, | 168 | }, | ||
| 72 | } | 169 | } | ||
| 73 | 170 | ||||
| 74 | - | c.handleURIChanged(ctx, paneID, wv, "https://example.com/app#state") | 171 | + | uriChanged(ctx, c, wv, "https://example.com/app#state") |
| 75 | 172 | ||||
| 76 | assert.Equal(t, paneID, recordedPane) | 173 | assert.Equal(t, paneID, recordedPane) | ||
| 77 | assert.Equal(t, "https://example.com/app#state", recordedURI) | 174 | assert.Equal(t, "https://example.com/app#state", recordedURI) | ||
| @@ -81,41 +178,145 @@ func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) { | |||||
| 81 | t.Parallel() | 178 | t.Parallel() | ||
| 82 | 179 | ||||
| 83 | ctx := context.Background() | 180 | ctx := context.Background() | ||
| 84 | - | paneID := entity.PaneID("pane-1") | 181 | + | wv := externalWebView(t, 1) |
| 85 | - | wv := mocks.NewMockWebView(t) | |||
| 86 | wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once() | 182 | wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once() | ||
| 87 | - | wv.EXPECT().CanGoBack().Return(false).Once() | |||
| 88 | 183 | ||||
| 89 | - | launchCount := 0 | 184 | + | rec := &externalLaunchRecorder{} |
| 90 | - | c := &Coordinator{ | 185 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 91 | - | onLaunchExternalURL: func(uri string) { | 186 | + | rec.install(c) |
| 92 | - | launchCount++ | |||
| 93 | - | }, | |||
| 94 | - | } | |||
| 95 | 187 | ||||
| 96 | - | c.handleURIChanged(ctx, paneID, wv, "vscode://file/etc/passwd") | 188 | + | uriChanged(ctx, c, wv, "vscode://file/etc/passwd") |
| 97 | 189 | ||||
| 98 | - | assert.Equal(t, 0, launchCount) | 190 | + | assert.Len(t, rec.prompts, 1) |
| 191 | + | assert.Empty(t, rec.launched) | |||
| 99 | } | 192 | } | ||
| 100 | 193 | ||||
| 101 | - | func TestHandleURIChanged_GoBackErrorDoesNotPanic(t *testing.T) { | 194 | + | func TestHandleURIChanged_OldAnswerCannotLaunchReplacementPrompt(t *testing.T) { |
| 102 | t.Parallel() | 195 | t.Parallel() | ||
| 103 | 196 | ||||
| 104 | ctx := context.Background() | 197 | ctx := context.Background() | ||
| 105 | - | paneID := entity.PaneID("pane-1") | 198 | + | wv := externalWebView(t, 1) |
| 106 | - | wv := mocks.NewMockWebView(t) | 199 | + | wv.EXPECT().Stop(ctx).Return(nil).Twice() |
| 200 | + | ||||
| 201 | + | rec := &externalLaunchRecorder{} | |||
| 202 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 203 | + | rec.install(c) | |||
| 204 | + | ||||
| 205 | + | uriChanged(ctx, c, wv, "claude://old") | |||
| 206 | + | oldRespond := rec.respond | |||
| 207 | + | // The pane is invalidated, then the same WebView asks again. | |||
| 208 | + | c.clearExternalPrompt("pane-1") | |||
| 209 | + | uriChanged(ctx, c, wv, "claude://new") | |||
| 210 | + | newRespond := rec.respond | |||
| 211 | + | ||||
| 212 | + | oldRespond(true) | |||
| 213 | + | assert.Empty(t, rec.launched, "old answer must not launch") | |||
| 214 | + | ||||
| 215 | + | newRespond(true) | |||
| 216 | + | assert.Equal(t, []string{"claude://new"}, rec.launched) | |||
| 217 | + | } | |||
| 218 | + | ||||
| 219 | + | func TestHandleURIChanged_RebindToNewGenerationMakesPromptStale(t *testing.T) { | |||
| 220 | + | t.Parallel() | |||
| 221 | + | ||||
| 222 | + | ctx := context.Background() | |||
| 223 | + | wv := externalWebView(t, 1) | |||
| 107 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 224 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 108 | - | wv.EXPECT().CanGoBack().Return(true).Once() | 225 | + | rebound := externalWebView(t, 2) |
| 109 | - | wv.EXPECT().GoBack(ctx).Return(errors.New("goback failed")).Once() | |||
| 110 | 226 | ||||
| 111 | - | launchCount := 0 | 227 | + | rec := &externalLaunchRecorder{} |
| 112 | - | c := &Coordinator{ | 228 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 113 | - | onLaunchExternalURL: func(uri string) { | 229 | + | rec.install(c) |
| 114 | - | launchCount++ | 230 | + | |
| 115 | - | }, | 231 | + | uriChanged(ctx, c, wv, "claude://login?code=x") |
| 232 | + | c.webViews["pane-1"] = rebound | |||
| 233 | + | ||||
| 234 | + | rec.respond(true) | |||
| 235 | + | assert.Empty(t, rec.launched) | |||
| 236 | + | } | |||
| 237 | + | ||||
| 238 | + | func TestHandleURIChanged_PromptNamesLastWebOrigin(t *testing.T) { | |||
| 239 | + | t.Parallel() | |||
| 240 | + | ||||
| 241 | + | ctx := context.Background() | |||
| 242 | + | wv := externalWebView(t, 1) | |||
| 243 | + | wv.EXPECT().IsLoading().Return(true).Once() | |||
| 244 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 245 | + | ||||
| 246 | + | rec := &externalLaunchRecorder{} | |||
| 247 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 248 | + | rec.install(c) | |||
| 249 | + | ||||
| 250 | + | uriChanged(ctx, c, wv, "https://claude.ai/login/popup-google-auth?code=x") | |||
| 251 | + | uriChanged(ctx, c, wv, "claude://login") | |||
| 252 | + | ||||
| 253 | + | assert.Equal(t, []string{"claude.ai"}, rec.origins) | |||
| 254 | + | } | |||
| 255 | + | ||||
| 256 | + | func TestHandleURIChanged_IgnoresCallbackFromReleasedWebView(t *testing.T) { | |||
| 257 | + | t.Parallel() | |||
| 258 | + | ||||
| 259 | + | ctx := context.Background() | |||
| 260 | + | wv := externalWebView(t, 1) | |||
| 261 | + | ||||
| 262 | + | rec := &externalLaunchRecorder{} | |||
| 263 | + | c := &Coordinator{} | |||
| 264 | + | rec.install(c) | |||
| 265 | + | ||||
| 266 | + | // The callback was queued before the pane released its WebView. | |||
| 267 | + | uriChanged(ctx, c, wv, "https://site.example/") | |||
| 268 | + | uriChanged(ctx, c, wv, "claude://login") | |||
| 269 | + | ||||
| 270 | + | assert.Empty(t, rec.prompts) | |||
| 271 | + | assert.Empty(t, c.externalPromptOrigins) | |||
| 272 | + | } | |||
| 273 | + | ||||
| 274 | + | func TestHandleURIChanged_NonHTTPDocumentClearsOrigin(t *testing.T) { | |||
| 275 | + | t.Parallel() | |||
| 276 | + | ||||
| 277 | + | for _, doc := range []string{"data:text/html,hi", "file://trusted.example/page.html"} { | |||
| 278 | + | ctx := context.Background() | |||
| 279 | + | wv := externalWebView(t, 1) | |||
| 280 | + | wv.EXPECT().IsLoading().Return(true).Times(2) | |||
| 281 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 282 | + | ||||
| 283 | + | rec := &externalLaunchRecorder{} | |||
| 284 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 285 | + | rec.install(c) | |||
| 286 | + | ||||
| 287 | + | uriChanged(ctx, c, wv, "https://trusted.example/") | |||
| 288 | + | uriChanged(ctx, c, wv, doc) | |||
| 289 | + | uriChanged(ctx, c, wv, "claude://login") | |||
| 290 | + | ||||
| 291 | + | assert.Equal(t, []string{""}, rec.origins, doc) | |||
| 116 | } | 292 | } | ||
| 293 | + | } | |||
| 117 | 294 | ||||
| 118 | - | c.handleURIChanged(ctx, paneID, wv, "spotify://track/123") | 295 | + | func TestURIChangedCallback_IgnoredAfterSameWebViewIsReusedFromPool(t *testing.T) { |
| 296 | + | ctx := context.Background() | |||
| 297 | + | c := newMinimalCoordinator() | |||
| 298 | + | rec := &externalLaunchRecorder{} | |||
| 299 | + | rec.install(c) | |||
| 300 | + | spaCalls := 0 | |||
| 301 | + | c.onPaneURIUpdated = func(entity.PaneID, string) { spaCalls++ } | |||
| 119 | 302 | ||||
| 120 | - | assert.Equal(t, 0, launchCount) | 303 | + | generation := uint64(1) |
| 304 | + | wv := mocks.NewMockWebView(t) | |||
| 305 | + | wv.EXPECT().ID().Return(port.WebViewID(1)).Maybe() | |||
| 306 | + | wv.EXPECT().Generation().RunAndReturn(func() uint64 { return generation }).Maybe() | |||
| 307 | + | var callbacks *port.WebViewCallbacks | |||
| 308 | + | wv.EXPECT().SetCallbacks(mock.Anything).Run(func(cb *port.WebViewCallbacks) { callbacks = cb }).Once() | |||
| 309 | + | ||||
| 310 | + | c.setWebViewLocked(testPaneID, wv) | |||
| 311 | + | c.setupWebViewCallbacks(ctx, testPaneID, wv) | |||
| 312 | + | require.NotNil(t, callbacks) | |||
| 313 | + | ||||
| 314 | + | // The same object is reused from the pool for a new binding. | |||
| 315 | + | generation = 2 | |||
| 316 | + | callbacks.OnURIChanged("https://site.example/") | |||
| 317 | + | callbacks.OnURIChanged("claude://login") | |||
| 318 | + | ||||
| 319 | + | assert.Empty(t, rec.prompts) | |||
| 320 | + | assert.Empty(t, c.externalPromptOrigins) | |||
| 321 | + | assert.Zero(t, spaCalls) | |||
| 121 | } | 322 | } | ||
M internal/ui/coordinator/content/popup_lifecycle_test.go +100 -0
| @@ -683,3 +683,103 @@ type popupStagingHostStub struct { | |||
| 683 | 683 | ||
| 684 | 684 | func (s *popupStagingHostStub) Detach() error { s.detached = true; return s.detachErr } | |
| 685 | 685 | func (s *popupStagingHostStub) Destroy() { s.destroyCalls++ } | |
| 686 | + | ||
| 687 | + | // --------------------------------------------------------------------------- | |
| 688 | + | // Popup config concurrency / aborted popup cleanup | |
| 689 | + | // --------------------------------------------------------------------------- | |
| 690 | + | ||
| 691 | + | func TestPopupConfig_ConcurrentUpdateAndRead(t *testing.T) { | |
| 692 | + | t.Parallel() | |
| 693 | + | ||
| 694 | + | pm := newPopupManager() | |
| 695 | + | var wg sync.WaitGroup | |
| 696 | + | for i := range 20 { | |
| 697 | + | wg.Add(3) | |
| 698 | + | go func() { | |
| 699 | + | defer wg.Done() | |
| 700 | + | pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: i%2 == 0}) | |
| 701 | + | }() | |
| 702 | + | go func() { | |
| 703 | + | defer wg.Done() | |
| 704 | + | pm.setPopupConfig(&entity.BrowsingContextConfig{OAuthAutoClose: i%2 == 0}) | |
| 705 | + | }() | |
| 706 | + | go func() { | |
| 707 | + | defer wg.Done() | |
| 708 | + | if cfg := pm.currentPopupConfig(); cfg != nil { | |
| 709 | + | _ = cfg.OpenInNewPane | |
| 710 | + | } | |
| 711 | + | }() | |
| 712 | + | } | |
| 713 | + | wg.Wait() | |
| 714 | + | } | |
| 715 | + | ||
| 716 | + | func TestCurrentPopupConfig_ReturnsSnapshot(t *testing.T) { | |
| 717 | + | t.Parallel() | |
| 718 | + | ||
| 719 | + | pm := newPopupManager() | |
| 720 | + | pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: true}) | |
| 721 | + | snapshot := pm.currentPopupConfig() | |
| 722 | + | require.NotNil(t, snapshot) | |
| 723 | + | snapshot.OpenInNewPane = false | |
| 724 | + | ||
| 725 | + | require.True(t, pm.currentPopupConfig().OpenInNewPane) | |
| 726 | + | } | |
| 727 | + | ||
| 728 | + | func TestPopupClose_AbortedPendingPopupCleansPendingOAuthAndPane(t *testing.T) { | |
| 729 | + | ctx := context.Background() | |
| 730 | + | parent := mocks.NewMockWebView(t) | |
| 731 | + | parent.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 732 | + | popup := &closableLifecyclePopupStub{popupNavigationWebViewStub: &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}} | |
| 733 | + | popup.EXPECT().ID().Return(port.WebViewID(260)).Maybe() | |
| 734 | + | popup.EXPECT().URI().Return("").Maybe() | |
| 735 | + | popup.EXPECT().IsLoading().Return(false).Maybe() | |
| 736 | + | popup.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 737 | + | popup.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 738 | + | popup.EXPECT().Destroy().Maybe() | |
| 739 | + | factory := mocks.NewMockWebViewFactory(t) | |
| 740 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popup, nil).Once() | |
| 741 | + | ||
| 742 | + | c := &Coordinator{webViews: make(map[entity.PaneID]port.WebView), popups: newPopupManager()} | |
| 743 | + | c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: true, OAuthAutoClose: true}, func() string { return "popup-pane" }) | |
| 744 | + | var closedPanes []entity.PaneID | |
| 745 | + | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { return nil }) | |
| 746 | + | c.SetOnClosePane(func(_ context.Context, paneID entity.PaneID) error { | |
| 747 | + | closedPanes = append(closedPanes, paneID) | |
| 748 | + | return nil | |
| 749 | + | }) | |
| 750 | + | ||
| 751 | + | created := c.handlePopupCreate(ctx, "parent-pane", parent, port.PopupRequest{ | |
| 752 | + | TargetURI: "https://example.com/popup", FrameName: "popup", IsUserGesture: true, | |
| 753 | + | }) | |
| 754 | + | require.Same(t, popup, created) | |
| 755 | + | require.Len(t, c.popups.pendingPopups, 1) | |
| 756 | + | // OAuth tracking is normally armed for auth-intent popups; seed it so the | |
| 757 | + | // cleanup contract is covered regardless of the policy's current routing. | |
| 758 | + | c.popups.trackOAuthPopup(260, "parent-pane", "https://app.example.com/login") | |
| 759 | + | require.Len(t, c.popups.popupOAuth, 1) | |
| 760 | + | require.NotNil(t, c.getWebViewLocked("popup-pane")) | |
| 761 | + | require.NotNil(t, popup.onClose) | |
| 762 | + | ||
| 763 | + | // CEF aborts the popup before ready-to-show: the engine fires the lifecycle | |
| 764 | + | // close callback exactly as it does for a destroyed shell. | |
| 765 | + | popup.close() | |
| 766 | + | ||
| 767 | + | assert.Empty(t, c.popups.pendingPopups) | |
| 768 | + | assert.Empty(t, c.popups.popupOAuth) | |
| 769 | + | assert.Equal(t, []entity.PaneID{"popup-pane"}, closedPanes) | |
| 770 | + | assert.Nil(t, c.getWebViewLocked("popup-pane")) | |
| 771 | + | } | |
| 772 | + | ||
| 773 | + | // closableLifecyclePopupStub records the lifecycle close callback the content | |
| 774 | + | // layer installs so tests can simulate the engine tearing the shell down. | |
| 775 | + | type closableLifecyclePopupStub struct { | |
| 776 | + | *popupNavigationWebViewStub | |
| 777 | + | onClose func() | |
| 778 | + | } | |
| 779 | + | ||
| 780 | + | func (s *closableLifecyclePopupStub) SetOnClose(fn func()) { s.onClose = fn } | |
| 781 | + | func (s *closableLifecyclePopupStub) close() { | |
| 782 | + | if s.onClose != nil { | |
| 783 | + | s.onClose() | |
| 784 | + | } | |
| 785 | + | } | |
| @@ -683,3 +683,103 @@ type popupStagingHostStub struct { | |||||
| 683 | 683 | ||||
| 684 | func (s *popupStagingHostStub) Detach() error { s.detached = true; return s.detachErr } | 684 | func (s *popupStagingHostStub) Detach() error { s.detached = true; return s.detachErr } | ||
| 685 | func (s *popupStagingHostStub) Destroy() { s.destroyCalls++ } | 685 | func (s *popupStagingHostStub) Destroy() { s.destroyCalls++ } | ||
| 686 | + | ||||
| 687 | + | // --------------------------------------------------------------------------- | |||
| 688 | + | // Popup config concurrency / aborted popup cleanup | |||
| 689 | + | // --------------------------------------------------------------------------- | |||
| 690 | + | ||||
| 691 | + | func TestPopupConfig_ConcurrentUpdateAndRead(t *testing.T) { | |||
| 692 | + | t.Parallel() | |||
| 693 | + | ||||
| 694 | + | pm := newPopupManager() | |||
| 695 | + | var wg sync.WaitGroup | |||
| 696 | + | for i := range 20 { | |||
| 697 | + | wg.Add(3) | |||
| 698 | + | go func() { | |||
| 699 | + | defer wg.Done() | |||
| 700 | + | pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: i%2 == 0}) | |||
| 701 | + | }() | |||
| 702 | + | go func() { | |||
| 703 | + | defer wg.Done() | |||
| 704 | + | pm.setPopupConfig(&entity.BrowsingContextConfig{OAuthAutoClose: i%2 == 0}) | |||
| 705 | + | }() | |||
| 706 | + | go func() { | |||
| 707 | + | defer wg.Done() | |||
| 708 | + | if cfg := pm.currentPopupConfig(); cfg != nil { | |||
| 709 | + | _ = cfg.OpenInNewPane | |||
| 710 | + | } | |||
| 711 | + | }() | |||
| 712 | + | } | |||
| 713 | + | wg.Wait() | |||
| 714 | + | } | |||
| 715 | + | ||||
| 716 | + | func TestCurrentPopupConfig_ReturnsSnapshot(t *testing.T) { | |||
| 717 | + | t.Parallel() | |||
| 718 | + | ||||
| 719 | + | pm := newPopupManager() | |||
| 720 | + | pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: true}) | |||
| 721 | + | snapshot := pm.currentPopupConfig() | |||
| 722 | + | require.NotNil(t, snapshot) | |||
| 723 | + | snapshot.OpenInNewPane = false | |||
| 724 | + | ||||
| 725 | + | require.True(t, pm.currentPopupConfig().OpenInNewPane) | |||
| 726 | + | } | |||
| 727 | + | ||||
| 728 | + | func TestPopupClose_AbortedPendingPopupCleansPendingOAuthAndPane(t *testing.T) { | |||
| 729 | + | ctx := context.Background() | |||
| 730 | + | parent := mocks.NewMockWebView(t) | |||
| 731 | + | parent.EXPECT().ID().Return(port.WebViewID(101)).Once() | |||
| 732 | + | popup := &closableLifecyclePopupStub{popupNavigationWebViewStub: &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}} | |||
| 733 | + | popup.EXPECT().ID().Return(port.WebViewID(260)).Maybe() | |||
| 734 | + | popup.EXPECT().URI().Return("").Maybe() | |||
| 735 | + | popup.EXPECT().IsLoading().Return(false).Maybe() | |||
| 736 | + | popup.EXPECT().Generation().Return(uint64(1)).Maybe() | |||
| 737 | + | popup.EXPECT().SetCallbacks(mock.Anything).Once() | |||
| 738 | + | popup.EXPECT().Destroy().Maybe() | |||
| 739 | + | factory := mocks.NewMockWebViewFactory(t) | |||
| 740 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popup, nil).Once() | |||
| 741 | + | ||||
| 742 | + | c := &Coordinator{webViews: make(map[entity.PaneID]port.WebView), popups: newPopupManager()} | |||
| 743 | + | c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: true, OAuthAutoClose: true}, func() string { return "popup-pane" }) | |||
| 744 | + | var closedPanes []entity.PaneID | |||
| 745 | + | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { return nil }) | |||
| 746 | + | c.SetOnClosePane(func(_ context.Context, paneID entity.PaneID) error { | |||
| 747 | + | closedPanes = append(closedPanes, paneID) | |||
| 748 | + | return nil | |||
| 749 | + | }) | |||
| 750 | + | ||||
| 751 | + | created := c.handlePopupCreate(ctx, "parent-pane", parent, port.PopupRequest{ | |||
| 752 | + | TargetURI: "https://example.com/popup", FrameName: "popup", IsUserGesture: true, | |||
| 753 | + | }) | |||
| 754 | + | require.Same(t, popup, created) | |||
| 755 | + | require.Len(t, c.popups.pendingPopups, 1) | |||
| 756 | + | // OAuth tracking is normally armed for auth-intent popups; seed it so the | |||
| 757 | + | // cleanup contract is covered regardless of the policy's current routing. | |||
| 758 | + | c.popups.trackOAuthPopup(260, "parent-pane", "https://app.example.com/login") | |||
| 759 | + | require.Len(t, c.popups.popupOAuth, 1) | |||
| 760 | + | require.NotNil(t, c.getWebViewLocked("popup-pane")) | |||
| 761 | + | require.NotNil(t, popup.onClose) | |||
| 762 | + | ||||
| 763 | + | // CEF aborts the popup before ready-to-show: the engine fires the lifecycle | |||
| 764 | + | // close callback exactly as it does for a destroyed shell. | |||
| 765 | + | popup.close() | |||
| 766 | + | ||||
| 767 | + | assert.Empty(t, c.popups.pendingPopups) | |||
| 768 | + | assert.Empty(t, c.popups.popupOAuth) | |||
| 769 | + | assert.Equal(t, []entity.PaneID{"popup-pane"}, closedPanes) | |||
| 770 | + | assert.Nil(t, c.getWebViewLocked("popup-pane")) | |||
| 771 | + | } | |||
| 772 | + | ||||
| 773 | + | // closableLifecyclePopupStub records the lifecycle close callback the content | |||
| 774 | + | // layer installs so tests can simulate the engine tearing the shell down. | |||
| 775 | + | type closableLifecyclePopupStub struct { | |||
| 776 | + | *popupNavigationWebViewStub | |||
| 777 | + | onClose func() | |||
| 778 | + | } | |||
| 779 | + | ||||
| 780 | + | func (s *closableLifecyclePopupStub) SetOnClose(fn func()) { s.onClose = fn } | |||
| 781 | + | func (s *closableLifecyclePopupStub) close() { | |||
| 782 | + | if s.onClose != nil { | |||
| 783 | + | s.onClose() | |||
| 784 | + | } | |||
| 785 | + | } | |||
M internal/ui/coordinator/content/popup_manager.go +14 -2
| @@ -130,6 +130,8 @@ func (pm *popupManager) setPopupConfig(popupConfig *entity.BrowsingContextConfig | |||
| 130 | 130 | if pm == nil { | |
| 131 | 131 | return | |
| 132 | 132 | } | |
| 133 | + | pm.mu.Lock() | |
| 134 | + | defer pm.mu.Unlock() | |
| 133 | 135 | if popupConfig == nil { | |
| 134 | 136 | pm.popupConfig = entity.BrowsingContextConfig{} | |
| 135 | 137 | pm.hasPopupConfig = false | |
| @@ -143,15 +145,25 @@ func (pm *popupManager) updatePopupConfig(popupConfig entity.BrowsingContextConf | |||
| 143 | 145 | if pm == nil { | |
| 144 | 146 | return | |
| 145 | 147 | } | |
| 148 | + | pm.mu.Lock() | |
| 149 | + | defer pm.mu.Unlock() | |
| 146 | 150 | pm.popupConfig = popupConfig | |
| 147 | 151 | pm.hasPopupConfig = true | |
| 148 | 152 | } | |
| 149 | 153 | ||
| 154 | + | // currentPopupConfig returns a snapshot copy of the popup config (nil when | |
| 155 | + | // unset). The copy keeps callers from racing with concurrent config updates. | |
| 150 | 156 | func (pm *popupManager) currentPopupConfig() *entity.BrowsingContextConfig { | |
| 151 | - | if pm == nil || !pm.hasPopupConfig { | |
| 157 | + | if pm == nil { | |
| 158 | + | return nil | |
| 159 | + | } | |
| 160 | + | pm.mu.RLock() | |
| 161 | + | defer pm.mu.RUnlock() | |
| 162 | + | if !pm.hasPopupConfig { | |
| 152 | 163 | return nil | |
| 153 | 164 | } | |
| 154 | - | return &pm.popupConfig | |
| 165 | + | cfg := pm.popupConfig | |
| 166 | + | return &cfg | |
| 155 | 167 | } | |
| 156 | 168 | ||
| 157 | 169 | func (pm *popupManager) setWindowIDResolver(fn func(entity.PaneID) (string, bool)) { | |
| @@ -130,6 +130,8 @@ func (pm *popupManager) setPopupConfig(popupConfig *entity.BrowsingContextConfig | |||||
| 130 | if pm == nil { | 130 | if pm == nil { | ||
| 131 | return | 131 | return | ||
| 132 | } | 132 | } | ||
| 133 | + | pm.mu.Lock() | |||
| 134 | + | defer pm.mu.Unlock() | |||
| 133 | if popupConfig == nil { | 135 | if popupConfig == nil { | ||
| 134 | pm.popupConfig = entity.BrowsingContextConfig{} | 136 | pm.popupConfig = entity.BrowsingContextConfig{} | ||
| 135 | pm.hasPopupConfig = false | 137 | pm.hasPopupConfig = false | ||
| @@ -143,15 +145,25 @@ func (pm *popupManager) updatePopupConfig(popupConfig entity.BrowsingContextConf | |||||
| 143 | if pm == nil { | 145 | if pm == nil { | ||
| 144 | return | 146 | return | ||
| 145 | } | 147 | } | ||
| 148 | + | pm.mu.Lock() | |||
| 149 | + | defer pm.mu.Unlock() | |||
| 146 | pm.popupConfig = popupConfig | 150 | pm.popupConfig = popupConfig | ||
| 147 | pm.hasPopupConfig = true | 151 | pm.hasPopupConfig = true | ||
| 148 | } | 152 | } | ||
| 149 | 153 | ||||
| 154 | + | // currentPopupConfig returns a snapshot copy of the popup config (nil when | |||
| 155 | + | // unset). The copy keeps callers from racing with concurrent config updates. | |||
| 150 | func (pm *popupManager) currentPopupConfig() *entity.BrowsingContextConfig { | 156 | func (pm *popupManager) currentPopupConfig() *entity.BrowsingContextConfig { | ||
| 151 | - | if pm == nil || !pm.hasPopupConfig { | 157 | + | if pm == nil { |
| 158 | + | return nil | |||
| 159 | + | } | |||
| 160 | + | pm.mu.RLock() | |||
| 161 | + | defer pm.mu.RUnlock() | |||
| 162 | + | if !pm.hasPopupConfig { | |||
| 152 | return nil | 163 | return nil | ||
| 153 | } | 164 | } | ||
| 154 | - | return &pm.popupConfig | 165 | + | cfg := pm.popupConfig |
| 166 | + | return &cfg | |||
| 155 | } | 167 | } | ||
| 156 | 168 | ||||
| 157 | func (pm *popupManager) setWindowIDResolver(fn func(entity.PaneID) (string, bool)) { | 169 | func (pm *popupManager) setWindowIDResolver(fn func(entity.PaneID) (string, bool)) { | ||
M internal/ui/coordinator/workspace.go +14 -0
| @@ -304,6 +304,20 @@ func comparablePaneURL(raw string) string { | |||
| 304 | 304 | return "dumb:" + page | |
| 305 | 305 | } | |
| 306 | 306 | ||
| 307 | + | // FocusPaneByID makes an existing pane the active pane of ws, revealing it in | |
| 308 | + | // its stack when needed and giving it keyboard focus in wsView. | |
| 309 | + | func (c *WorkspaceCoordinator) FocusPaneByID( | |
| 310 | + | ctx context.Context, | |
| 311 | + | ws *entity.Workspace, | |
| 312 | + | wsView *component.WorkspaceView, | |
| 313 | + | paneID entity.PaneID, | |
| 314 | + | ) { | |
| 315 | + | if ws == nil || ws.FindPane(paneID) == nil { | |
| 316 | + | return | |
| 317 | + | } | |
| 318 | + | c.focusExistingPane(ctx, ws, wsView, paneID) | |
| 319 | + | } | |
| 320 | + | ||
| 307 | 321 | func (c *WorkspaceCoordinator) focusExistingPane( | |
| 308 | 322 | ctx context.Context, | |
| 309 | 323 | ws *entity.Workspace, | |
| @@ -304,6 +304,20 @@ func comparablePaneURL(raw string) string { | |||||
| 304 | return "dumb:" + page | 304 | return "dumb:" + page | ||
| 305 | } | 305 | } | ||
| 306 | 306 | ||||
| 307 | + | // FocusPaneByID makes an existing pane the active pane of ws, revealing it in | |||
| 308 | + | // its stack when needed and giving it keyboard focus in wsView. | |||
| 309 | + | func (c *WorkspaceCoordinator) FocusPaneByID( | |||
| 310 | + | ctx context.Context, | |||
| 311 | + | ws *entity.Workspace, | |||
| 312 | + | wsView *component.WorkspaceView, | |||
| 313 | + | paneID entity.PaneID, | |||
| 314 | + | ) { | |||
| 315 | + | if ws == nil || ws.FindPane(paneID) == nil { | |||
| 316 | + | return | |||
| 317 | + | } | |||
| 318 | + | c.focusExistingPane(ctx, ws, wsView, paneID) | |||
| 319 | + | } | |||
| 320 | + | ||||
| 307 | func (c *WorkspaceCoordinator) focusExistingPane( | 321 | func (c *WorkspaceCoordinator) focusExistingPane( | ||
| 308 | ctx context.Context, | 322 | ctx context.Context, | ||
| 309 | ws *entity.Workspace, | 323 | ws *entity.Workspace, | ||
M internal/ui/dialog/permission.go +43 -2
| @@ -4,6 +4,7 @@ package dialog | |||
| 4 | 4 | import ( | |
| 5 | 5 | "context" | |
| 6 | 6 | "fmt" | |
| 7 | + | neturl "net/url" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "sync" | |
| 9 | 10 | ||
| @@ -23,6 +24,11 @@ type permissionDialogRequest struct { | |||
| 23 | 24 | permTypes []entity.PermissionType | |
| 24 | 25 | metadata entity.PermissionMetadata | |
| 25 | 26 | callback func(result port.PermissionDialogResult) | |
| 27 | + | ||
| 28 | + | // heading and body, when set, replace the permission-derived text. Used | |
| 29 | + | // for one-shot confirmations that share the popup and its queue. | |
| 30 | + | heading string | |
| 31 | + | body string | |
| 26 | 32 | } | |
| 27 | 33 | ||
| 28 | 34 | // PermissionDialog implements the port.PermissionDialogPresenter interface. | |
| @@ -67,6 +73,38 @@ func (d *PermissionDialog) ShowPermissionDialog( | |||
| 67 | 73 | d.showRequest(req) | |
| 68 | 74 | } | |
| 69 | 75 | ||
| 76 | + | // ShowExternalLaunchConfirm asks whether an external application may open uri | |
| 77 | + | // on behalf of origin (the requesting site's host, may be empty). The prompt | |
| 78 | + | // shares the permission popup queue and is never persistable. | |
| 79 | + | func (d *PermissionDialog) ShowExternalLaunchConfirm(ctx context.Context, origin, uri string, callback func(allowed bool)) { | |
| 80 | + | req := permissionDialogRequest{ | |
| 81 | + | ctx: ctx, | |
| 82 | + | heading: "Open External Application?", | |
| 83 | + | body: externalLaunchBody(origin, uri), | |
| 84 | + | callback: func(result port.PermissionDialogResult) { | |
| 85 | + | callback(result.Allowed) | |
| 86 | + | }, | |
| 87 | + | } | |
| 88 | + | if !d.enqueueOrStart(req) { | |
| 89 | + | return | |
| 90 | + | } | |
| 91 | + | d.showRequest(req) | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | // externalLaunchBody names the requesting host and only the scheme and host of | |
| 95 | + | // the target, so OAuth codes in the query string are never displayed. | |
| 96 | + | func externalLaunchBody(origin, uri string) string { | |
| 97 | + | target := uri | |
| 98 | + | if parsed, err := neturl.Parse(uri); err == nil && parsed.Scheme != "" { | |
| 99 | + | target = parsed.Scheme + "://" + parsed.Host | |
| 100 | + | } | |
| 101 | + | requester := "A page" | |
| 102 | + | if origin != "" { | |
| 103 | + | requester = origin | |
| 104 | + | } | |
| 105 | + | return fmt.Sprintf("%s wants to open %s in an external application.", requester, target) | |
| 106 | + | } | |
| 107 | + | ||
| 70 | 108 | func (d *PermissionDialog) enqueueOrStart(req permissionDialogRequest) bool { | |
| 71 | 109 | d.mu.Lock() | |
| 72 | 110 | defer d.mu.Unlock() | |
| @@ -97,8 +135,11 @@ func (d *PermissionDialog) showRequest(req permissionDialogRequest) { | |||
| 97 | 135 | } | |
| 98 | 136 | ||
| 99 | 137 | // Build dialog text | |
| 100 | - | heading := d.buildHeading(permTypes) | |
| 101 | - | body := d.buildBody(origin, permTypes, metadata) | |
| 138 | + | heading, body := req.heading, req.body | |
| 139 | + | if heading == "" { | |
| 140 | + | heading = d.buildHeading(permTypes) | |
| 141 | + | body = d.buildBody(origin, permTypes, metadata) | |
| 142 | + | } | |
| 102 | 143 | ||
| 103 | 144 | isDataAccess := parsePermFlags(permTypes).dataAccess | |
| 104 | 145 | if isDataAccess { | |
| @@ -4,6 +4,7 @@ package dialog | |||||
| 4 | import ( | 4 | import ( | ||
| 5 | "context" | 5 | "context" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | neturl "net/url" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "sync" | 9 | "sync" | ||
| 9 | 10 | ||||
| @@ -23,6 +24,11 @@ type permissionDialogRequest struct { | |||||
| 23 | permTypes []entity.PermissionType | 24 | permTypes []entity.PermissionType | ||
| 24 | metadata entity.PermissionMetadata | 25 | metadata entity.PermissionMetadata | ||
| 25 | callback func(result port.PermissionDialogResult) | 26 | callback func(result port.PermissionDialogResult) | ||
| 27 | + | ||||
| 28 | + | // heading and body, when set, replace the permission-derived text. Used | |||
| 29 | + | // for one-shot confirmations that share the popup and its queue. | |||
| 30 | + | heading string | |||
| 31 | + | body string | |||
| 26 | } | 32 | } | ||
| 27 | 33 | ||||
| 28 | // PermissionDialog implements the port.PermissionDialogPresenter interface. | 34 | // PermissionDialog implements the port.PermissionDialogPresenter interface. | ||
| @@ -67,6 +73,38 @@ func (d *PermissionDialog) ShowPermissionDialog( | |||||
| 67 | d.showRequest(req) | 73 | d.showRequest(req) | ||
| 68 | } | 74 | } | ||
| 69 | 75 | ||||
| 76 | + | // ShowExternalLaunchConfirm asks whether an external application may open uri | |||
| 77 | + | // on behalf of origin (the requesting site's host, may be empty). The prompt | |||
| 78 | + | // shares the permission popup queue and is never persistable. | |||
| 79 | + | func (d *PermissionDialog) ShowExternalLaunchConfirm(ctx context.Context, origin, uri string, callback func(allowed bool)) { | |||
| 80 | + | req := permissionDialogRequest{ | |||
| 81 | + | ctx: ctx, | |||
| 82 | + | heading: "Open External Application?", | |||
| 83 | + | body: externalLaunchBody(origin, uri), | |||
| 84 | + | callback: func(result port.PermissionDialogResult) { | |||
| 85 | + | callback(result.Allowed) | |||
| 86 | + | }, | |||
| 87 | + | } | |||
| 88 | + | if !d.enqueueOrStart(req) { | |||
| 89 | + | return | |||
| 90 | + | } | |||
| 91 | + | d.showRequest(req) | |||
| 92 | + | } | |||
| 93 | + | ||||
| 94 | + | // externalLaunchBody names the requesting host and only the scheme and host of | |||
| 95 | + | // the target, so OAuth codes in the query string are never displayed. | |||
| 96 | + | func externalLaunchBody(origin, uri string) string { | |||
| 97 | + | target := uri | |||
| 98 | + | if parsed, err := neturl.Parse(uri); err == nil && parsed.Scheme != "" { | |||
| 99 | + | target = parsed.Scheme + "://" + parsed.Host | |||
| 100 | + | } | |||
| 101 | + | requester := "A page" | |||
| 102 | + | if origin != "" { | |||
| 103 | + | requester = origin | |||
| 104 | + | } | |||
| 105 | + | return fmt.Sprintf("%s wants to open %s in an external application.", requester, target) | |||
| 106 | + | } | |||
| 107 | + | ||||
| 70 | func (d *PermissionDialog) enqueueOrStart(req permissionDialogRequest) bool { | 108 | func (d *PermissionDialog) enqueueOrStart(req permissionDialogRequest) bool { | ||
| 71 | d.mu.Lock() | 109 | d.mu.Lock() | ||
| 72 | defer d.mu.Unlock() | 110 | defer d.mu.Unlock() | ||
| @@ -97,8 +135,11 @@ func (d *PermissionDialog) showRequest(req permissionDialogRequest) { | |||||
| 97 | } | 135 | } | ||
| 98 | 136 | ||||
| 99 | // Build dialog text | 137 | // Build dialog text | ||
| 100 | - | heading := d.buildHeading(permTypes) | 138 | + | heading, body := req.heading, req.body |
| 101 | - | body := d.buildBody(origin, permTypes, metadata) | 139 | + | if heading == "" { |
| 140 | + | heading = d.buildHeading(permTypes) | |||
| 141 | + | body = d.buildBody(origin, permTypes, metadata) | |||
| 142 | + | } | |||
| 102 | 143 | ||||
| 103 | isDataAccess := parsePermFlags(permTypes).dataAccess | 144 | isDataAccess := parsePermFlags(permTypes).dataAccess | ||
| 104 | if isDataAccess { | 145 | if isDataAccess { | ||
M internal/ui/dialog/permission_test.go +23 -0
| @@ -255,3 +255,26 @@ func TestPermissionDialog_PassesPersistableToPopup(t *testing.T) { | |||
| 255 | 255 | }) | |
| 256 | 256 | } | |
| 257 | 257 | } | |
| 258 | + | ||
| 259 | + | func TestPermissionDialog_ExternalLaunchConfirmHidesQueryAndIsNotPersistable(t *testing.T) { | |
| 260 | + | popup := dialogmocks.NewMockPermissionPopup(t) | |
| 261 | + | var body string | |
| 262 | + | var persistable bool | |
| 263 | + | var cb func(allowed, persistent bool) | |
| 264 | + | popup.EXPECT(). | |
| 265 | + | Show(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). | |
| 266 | + | Run(func(_ context.Context, _ string, b string, p bool, c func(allowed, persistent bool)) { | |
| 267 | + | body, persistable, cb = b, p, c | |
| 268 | + | }).Once() | |
| 269 | + | ||
| 270 | + | d := &PermissionDialog{popup: popup} | |
| 271 | + | var allowed bool | |
| 272 | + | d.ShowExternalLaunchConfirm(context.Background(), "claude.ai", "claude://login/google-auth?code=secret", func(a bool) { allowed = a }) | |
| 273 | + | ||
| 274 | + | assert.Contains(t, body, "claude.ai wants to open claude://login") | |
| 275 | + | assert.NotContains(t, body, "secret") | |
| 276 | + | assert.False(t, persistable) | |
| 277 | + | ||
| 278 | + | cb(true, false) | |
| 279 | + | assert.True(t, allowed) | |
| 280 | + | } | |
| @@ -255,3 +255,26 @@ func TestPermissionDialog_PassesPersistableToPopup(t *testing.T) { | |||||
| 255 | }) | 255 | }) | ||
| 256 | } | 256 | } | ||
| 257 | } | 257 | } | ||
| 258 | + | ||||
| 259 | + | func TestPermissionDialog_ExternalLaunchConfirmHidesQueryAndIsNotPersistable(t *testing.T) { | |||
| 260 | + | popup := dialogmocks.NewMockPermissionPopup(t) | |||
| 261 | + | var body string | |||
| 262 | + | var persistable bool | |||
| 263 | + | var cb func(allowed, persistent bool) | |||
| 264 | + | popup.EXPECT(). | |||
| 265 | + | Show(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). | |||
| 266 | + | Run(func(_ context.Context, _ string, b string, p bool, c func(allowed, persistent bool)) { | |||
| 267 | + | body, persistable, cb = b, p, c | |||
| 268 | + | }).Once() | |||
| 269 | + | ||||
| 270 | + | d := &PermissionDialog{popup: popup} | |||
| 271 | + | var allowed bool | |||
| 272 | + | d.ShowExternalLaunchConfirm(context.Background(), "claude.ai", "claude://login/google-auth?code=secret", func(a bool) { allowed = a }) | |||
| 273 | + | ||||
| 274 | + | assert.Contains(t, body, "claude.ai wants to open claude://login") | |||
| 275 | + | assert.NotContains(t, body, "secret") | |||
| 276 | + | assert.False(t, persistable) | |||
| 277 | + | ||||
| 278 | + | cb(true, false) | |||
| 279 | + | assert.True(t, allowed) | |||
| 280 | + | } | |||
A internal/ui/external_url_relay_reuse.go +120 -0
| @@ -0,0 +1,120 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 7 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |
| 8 | + | "github.com/bnema/dumber/internal/logging" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | // openRelayURLOnMainThread opens a relayed external URL, reusing the pane from | |
| 12 | + | // a recent identical request when it still exists. Must run on the main thread. | |
| 13 | + | func (a *App) openRelayURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | |
| 14 | + | if a.reuseRelayPane(ctx, url) { | |
| 15 | + | return nil | |
| 16 | + | } | |
| 17 | + | if err := a.openExternalURLOnMainThread(ctx, url, cfg); err != nil { | |
| 18 | + | return err | |
| 19 | + | } | |
| 20 | + | a.recordRelayPane(url) | |
| 21 | + | return nil | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | // reuseRelayPane navigates and focuses the pane previously opened for url. | |
| 25 | + | // It reports false when there is nothing reusable and a new pane should be created. | |
| 26 | + | func (a *App) reuseRelayPane(ctx context.Context, url string) bool { | |
| 27 | + | paneID, ok := a.externalRelayPanes.Lookup(url, func(id entity.PaneID) bool { | |
| 28 | + | return a.browserWindowForPane(id) != nil | |
| 29 | + | }) | |
| 30 | + | if !ok { | |
| 31 | + | return false | |
| 32 | + | } | |
| 33 | + | log := logging.FromContext(ctx) | |
| 34 | + | ||
| 35 | + | bw := a.browserWindowForPane(paneID) | |
| 36 | + | var tab *entity.Tab | |
| 37 | + | if bw != nil && bw.tabs != nil { | |
| 38 | + | for _, candidate := range bw.tabs.Tabs { | |
| 39 | + | if candidate != nil && candidate.Workspace != nil && candidate.Workspace.FindPane(paneID) != nil { | |
| 40 | + | tab = candidate | |
| 41 | + | break | |
| 42 | + | } | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | if tab == nil || a.navCoord == nil || a.contentCoord == nil { | |
| 46 | + | a.externalRelayPanes.Forget(url) | |
| 47 | + | return false | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | // Reload the URL first: when the pane cannot be navigated, fall back to | |
| 51 | + | // creating a fresh pane instead of focusing a dead one. | |
| 52 | + | if err := a.navCoord.NavigateWebView(ctx, url, paneID, a.contentCoord.GetWebView(paneID)); err != nil { | |
| 53 | + | log.Warn().Err(err). | |
| 54 | + | Str("pane_id", string(paneID)). | |
| 55 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 56 | + | Msg("ui: external URL relay pane could not be navigated; opening a new pane") | |
| 57 | + | a.externalRelayPanes.Forget(url) | |
| 58 | + | return false | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | a.focusRelayPane(ctx, bw, tab, paneID) | |
| 62 | + | log.Debug(). | |
| 63 | + | Str("pane_id", string(paneID)). | |
| 64 | + | Str("url_host", logging.SafeURLHost(url)). | |
| 65 | + | Msg("ui: external URL reused relay pane") | |
| 66 | + | return true | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | func (a *App) focusRelayPane(ctx context.Context, bw *browserWindow, tab *entity.Tab, paneID entity.PaneID) { | |
| 70 | + | if bw.tabs.ActiveTabID != tab.ID { | |
| 71 | + | if a.tabCoord != nil { | |
| 72 | + | if err := a.tabCoord.Switch(ctx, a.tabTargetForBrowserWindow(bw), tab.ID); err != nil { | |
| 73 | + | logging.FromContext(ctx).Warn().Err(err). | |
| 74 | + | Str("tab_id", string(tab.ID)). | |
| 75 | + | Msg("ui: failed to switch to relay pane tab") | |
| 76 | + | } | |
| 77 | + | } else { | |
| 78 | + | bw.tabs.SetActive(tab.ID) | |
| 79 | + | if bw.mainWindow != nil && bw.mainWindow.TabBar() != nil { | |
| 80 | + | bw.mainWindow.TabBar().SetActive(tab.ID) | |
| 81 | + | } | |
| 82 | + | a.switchWorkspaceView(ctx, tab.ID) | |
| 83 | + | } | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | wsView := a.workspaceViews[tab.ID] | |
| 87 | + | if a.wsCoord != nil { | |
| 88 | + | a.wsCoord.FocusPaneByID(ctx, tab.Workspace, wsView, paneID) | |
| 89 | + | } else { | |
| 90 | + | tab.Workspace.ActivePaneID = paneID | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | // A floating pane over the tab would hide the relay pane and keep the | |
| 94 | + | // active-pane override pointing at itself. | |
| 95 | + | a.hideVisibleFloatingSessions(ctx, tab.ID, nil) | |
| 96 | + | ||
| 97 | + | if bw.mainWindow != nil { | |
| 98 | + | bw.mainWindow.Show() | |
| 99 | + | } | |
| 100 | + | a.activateBrowserWindow(bw) | |
| 101 | + | a.syncFloatingFocus() | |
| 102 | + | if wsView != nil { | |
| 103 | + | wsView.FocusPane(paneID) | |
| 104 | + | } | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | // recordRelayPane remembers the pane that was just created for url. After a | |
| 108 | + | // successful placement the new pane is the active pane of the last-focused | |
| 109 | + | // window's active tab (tabbed, split, stacked, and fresh window alike). | |
| 110 | + | func (a *App) recordRelayPane(url string) { | |
| 111 | + | ws := a.activeWorkspaceForBrowserWindow(a.lastFocusedBrowserWindow()) | |
| 112 | + | if ws == nil { | |
| 113 | + | return | |
| 114 | + | } | |
| 115 | + | active := ws.ActivePane() | |
| 116 | + | if active == nil || active.Pane == nil || domainurl.RelayURLKey(active.Pane.URI) != domainurl.RelayURLKey(url) { | |
| 117 | + | return | |
| 118 | + | } | |
| 119 | + | a.externalRelayPanes.Record(url, active.Pane.ID) | |
| 120 | + | } | |
| @@ -0,0 +1,120 @@ | |||||
| 1 | + | package ui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 7 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |||
| 8 | + | "github.com/bnema/dumber/internal/logging" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | // openRelayURLOnMainThread opens a relayed external URL, reusing the pane from | |||
| 12 | + | // a recent identical request when it still exists. Must run on the main thread. | |||
| 13 | + | func (a *App) openRelayURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | |||
| 14 | + | if a.reuseRelayPane(ctx, url) { | |||
| 15 | + | return nil | |||
| 16 | + | } | |||
| 17 | + | if err := a.openExternalURLOnMainThread(ctx, url, cfg); err != nil { | |||
| 18 | + | return err | |||
| 19 | + | } | |||
| 20 | + | a.recordRelayPane(url) | |||
| 21 | + | return nil | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | // reuseRelayPane navigates and focuses the pane previously opened for url. | |||
| 25 | + | // It reports false when there is nothing reusable and a new pane should be created. | |||
| 26 | + | func (a *App) reuseRelayPane(ctx context.Context, url string) bool { | |||
| 27 | + | paneID, ok := a.externalRelayPanes.Lookup(url, func(id entity.PaneID) bool { | |||
| 28 | + | return a.browserWindowForPane(id) != nil | |||
| 29 | + | }) | |||
| 30 | + | if !ok { | |||
| 31 | + | return false | |||
| 32 | + | } | |||
| 33 | + | log := logging.FromContext(ctx) | |||
| 34 | + | ||||
| 35 | + | bw := a.browserWindowForPane(paneID) | |||
| 36 | + | var tab *entity.Tab | |||
| 37 | + | if bw != nil && bw.tabs != nil { | |||
| 38 | + | for _, candidate := range bw.tabs.Tabs { | |||
| 39 | + | if candidate != nil && candidate.Workspace != nil && candidate.Workspace.FindPane(paneID) != nil { | |||
| 40 | + | tab = candidate | |||
| 41 | + | break | |||
| 42 | + | } | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | if tab == nil || a.navCoord == nil || a.contentCoord == nil { | |||
| 46 | + | a.externalRelayPanes.Forget(url) | |||
| 47 | + | return false | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | // Reload the URL first: when the pane cannot be navigated, fall back to | |||
| 51 | + | // creating a fresh pane instead of focusing a dead one. | |||
| 52 | + | if err := a.navCoord.NavigateWebView(ctx, url, paneID, a.contentCoord.GetWebView(paneID)); err != nil { | |||
| 53 | + | log.Warn().Err(err). | |||
| 54 | + | Str("pane_id", string(paneID)). | |||
| 55 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 56 | + | Msg("ui: external URL relay pane could not be navigated; opening a new pane") | |||
| 57 | + | a.externalRelayPanes.Forget(url) | |||
| 58 | + | return false | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | a.focusRelayPane(ctx, bw, tab, paneID) | |||
| 62 | + | log.Debug(). | |||
| 63 | + | Str("pane_id", string(paneID)). | |||
| 64 | + | Str("url_host", logging.SafeURLHost(url)). | |||
| 65 | + | Msg("ui: external URL reused relay pane") | |||
| 66 | + | return true | |||
| 67 | + | } | |||
| 68 | + | ||||
| 69 | + | func (a *App) focusRelayPane(ctx context.Context, bw *browserWindow, tab *entity.Tab, paneID entity.PaneID) { | |||
| 70 | + | if bw.tabs.ActiveTabID != tab.ID { | |||
| 71 | + | if a.tabCoord != nil { | |||
| 72 | + | if err := a.tabCoord.Switch(ctx, a.tabTargetForBrowserWindow(bw), tab.ID); err != nil { | |||
| 73 | + | logging.FromContext(ctx).Warn().Err(err). | |||
| 74 | + | Str("tab_id", string(tab.ID)). | |||
| 75 | + | Msg("ui: failed to switch to relay pane tab") | |||
| 76 | + | } | |||
| 77 | + | } else { | |||
| 78 | + | bw.tabs.SetActive(tab.ID) | |||
| 79 | + | if bw.mainWindow != nil && bw.mainWindow.TabBar() != nil { | |||
| 80 | + | bw.mainWindow.TabBar().SetActive(tab.ID) | |||
| 81 | + | } | |||
| 82 | + | a.switchWorkspaceView(ctx, tab.ID) | |||
| 83 | + | } | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | wsView := a.workspaceViews[tab.ID] | |||
| 87 | + | if a.wsCoord != nil { | |||
| 88 | + | a.wsCoord.FocusPaneByID(ctx, tab.Workspace, wsView, paneID) | |||
| 89 | + | } else { | |||
| 90 | + | tab.Workspace.ActivePaneID = paneID | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | // A floating pane over the tab would hide the relay pane and keep the | |||
| 94 | + | // active-pane override pointing at itself. | |||
| 95 | + | a.hideVisibleFloatingSessions(ctx, tab.ID, nil) | |||
| 96 | + | ||||
| 97 | + | if bw.mainWindow != nil { | |||
| 98 | + | bw.mainWindow.Show() | |||
| 99 | + | } | |||
| 100 | + | a.activateBrowserWindow(bw) | |||
| 101 | + | a.syncFloatingFocus() | |||
| 102 | + | if wsView != nil { | |||
| 103 | + | wsView.FocusPane(paneID) | |||
| 104 | + | } | |||
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | // recordRelayPane remembers the pane that was just created for url. After a | |||
| 108 | + | // successful placement the new pane is the active pane of the last-focused | |||
| 109 | + | // window's active tab (tabbed, split, stacked, and fresh window alike). | |||
| 110 | + | func (a *App) recordRelayPane(url string) { | |||
| 111 | + | ws := a.activeWorkspaceForBrowserWindow(a.lastFocusedBrowserWindow()) | |||
| 112 | + | if ws == nil { | |||
| 113 | + | return | |||
| 114 | + | } | |||
| 115 | + | active := ws.ActivePane() | |||
| 116 | + | if active == nil || active.Pane == nil || domainurl.RelayURLKey(active.Pane.URI) != domainurl.RelayURLKey(url) { | |||
| 117 | + | return | |||
| 118 | + | } | |||
| 119 | + | a.externalRelayPanes.Record(url, active.Pane.ID) | |||
| 120 | + | } | |||
A internal/ui/external_url_relay_reuse_test.go +149 -0
| @@ -0,0 +1,149 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/application/port" | |
| 9 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 10 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 11 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |
| 12 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 13 | + | "github.com/bnema/dumber/internal/ui/component" | |
| 14 | + | "github.com/bnema/dumber/internal/ui/coordinator" | |
| 15 | + | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 16 | + | "github.com/stretchr/testify/assert" | |
| 17 | + | "github.com/stretchr/testify/require" | |
| 18 | + | ) | |
| 19 | + | ||
| 20 | + | const relayTestURL = "https://claude.com/cai/login/app-google-auth?open_in_browser=1&hop_nonce=XYZ" | |
| 21 | + | ||
| 22 | + | type relayReuseHarness struct { | |
| 23 | + | app *App | |
| 24 | + | tabs *entity.TabList | |
| 25 | + | window *browserWindow | |
| 26 | + | webview map[entity.PaneID]*recordingWebView | |
| 27 | + | clock *time.Time | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | // newRelayReuseHarness builds an App with one window holding one tab, using the | |
| 31 | + | // tabbed external-link behavior. New tabs get a recording webview registered so | |
| 32 | + | // reuse can navigate them. | |
| 33 | + | func newRelayReuseHarness(t *testing.T) *relayReuseHarness { | |
| 34 | + | t.Helper() | |
| 35 | + | ctx := context.Background() | |
| 36 | + | ||
| 37 | + | original := entity.NewTab("tab-1", "ws-1", entity.NewPane("pane-1")) | |
| 38 | + | tabs := entity.NewTabList() | |
| 39 | + | tabs.Add(original) | |
| 40 | + | window := &browserWindow{id: "focused", tabs: tabs} | |
| 41 | + | ||
| 42 | + | ids := []string{"tab-2", "ws-2", "pane-2", "tab-3", "ws-3", "pane-3"} | |
| 43 | + | index := 0 | |
| 44 | + | now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) | |
| 45 | + | h := &relayReuseHarness{tabs: tabs, window: window, webview: map[entity.PaneID]*recordingWebView{}, clock: &now} | |
| 46 | + | ||
| 47 | + | contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil) | |
| 48 | + | app := &App{ | |
| 49 | + | runtimeConfig: runtimeConfigStateForTest(&config.Config{Workspace: entity.WorkspaceConfig{ExternalLinks: entity.ExternalLinksConfig{Behavior: entity.ExternalLinkBehaviorTabbed}}}), | |
| 50 | + | tabsUC: usecase.NewManageTabsUseCase(func() string { id := ids[index]; index++; return id }, nil), | |
| 51 | + | browserWindows: map[string]*browserWindow{ | |
| 52 | + | window.id: window, | |
| 53 | + | }, | |
| 54 | + | browserWindowOrder: []string{window.id}, | |
| 55 | + | lastFocusedWindowID: window.id, | |
| 56 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{}, | |
| 57 | + | windowForTab: map[entity.TabID]*browserWindow{}, | |
| 58 | + | contentCoord: contentCoord, | |
| 59 | + | navCoord: coordinator.NewNavigationCoordinator(ctx, nil, contentCoord), | |
| 60 | + | } | |
| 61 | + | app.externalRelayPanes.Now = func() time.Time { return *h.clock } | |
| 62 | + | app.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{TabsUC: app.tabsUC}) | |
| 63 | + | app.tabCoord.SetOnTabCreated(func(_ context.Context, _ coordinator.TabTarget, tab *entity.Tab) { | |
| 64 | + | app.workspaceViews[tab.ID] = &component.WorkspaceView{} | |
| 65 | + | app.setBrowserWindowForTab(tab.ID, window) | |
| 66 | + | paneID := tab.Workspace.ActivePane().Pane.ID | |
| 67 | + | wv := &recordingWebView{id: port.WebViewID(100 + len(h.webview))} | |
| 68 | + | h.webview[paneID] = wv | |
| 69 | + | contentCoord.RegisterPopupWebView(paneID, wv) | |
| 70 | + | }) | |
| 71 | + | h.app = app | |
| 72 | + | return h | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func TestApp_OpenExternalURLReusesRecentRelayPane(t *testing.T) { | |
| 76 | + | ctx := context.Background() | |
| 77 | + | h := newRelayReuseHarness(t) | |
| 78 | + | ||
| 79 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 80 | + | require.Len(t, h.tabs.Tabs, 2) | |
| 81 | + | relayTab := h.tabs.ActiveTab() | |
| 82 | + | relayPaneID := relayTab.Workspace.ActivePane().Pane.ID | |
| 83 | + | assert.False(t, h.webview[relayPaneID].loadURICalled) | |
| 84 | + | ||
| 85 | + | // The user moves on to another tab, then the app retries the same URL. | |
| 86 | + | h.tabs.SetActive("tab-1") | |
| 87 | + | *h.clock = h.clock.Add(30 * time.Second) | |
| 88 | + | ||
| 89 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 90 | + | ||
| 91 | + | assert.Len(t, h.tabs.Tabs, 2, "no new tab must be created") | |
| 92 | + | assert.Equal(t, relayTab.ID, h.tabs.ActiveTabID, "relay tab must be focused again") | |
| 93 | + | assert.True(t, h.webview[relayPaneID].loadURICalled) | |
| 94 | + | assert.Equal(t, relayTestURL, h.webview[relayPaneID].loadURILastURI) | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | func TestApp_OpenExternalURLCreatesNewPaneAfterReuseWindowExpires(t *testing.T) { | |
| 98 | + | ctx := context.Background() | |
| 99 | + | h := newRelayReuseHarness(t) | |
| 100 | + | ||
| 101 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 102 | + | require.Len(t, h.tabs.Tabs, 2) | |
| 103 | + | ||
| 104 | + | *h.clock = h.clock.Add(domainurl.RelayReuseWindow + time.Second) | |
| 105 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 106 | + | ||
| 107 | + | assert.Len(t, h.tabs.Tabs, 3) | |
| 108 | + | } | |
| 109 | + | ||
| 110 | + | func TestApp_OpenExternalURLCreatesNewPaneForDifferentURL(t *testing.T) { | |
| 111 | + | ctx := context.Background() | |
| 112 | + | h := newRelayReuseHarness(t) | |
| 113 | + | ||
| 114 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 115 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL+"2")) | |
| 116 | + | ||
| 117 | + | assert.Len(t, h.tabs.Tabs, 3) | |
| 118 | + | } | |
| 119 | + | ||
| 120 | + | func TestApp_OpenExternalURLCreatesNewPaneWhenRelayPaneWasClosed(t *testing.T) { | |
| 121 | + | ctx := context.Background() | |
| 122 | + | h := newRelayReuseHarness(t) | |
| 123 | + | ||
| 124 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 125 | + | relayTab := h.tabs.ActiveTab() | |
| 126 | + | h.tabs.Remove(relayTab.ID) | |
| 127 | + | require.Len(t, h.tabs.Tabs, 1) | |
| 128 | + | ||
| 129 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 130 | + | ||
| 131 | + | assert.Len(t, h.tabs.Tabs, 2) | |
| 132 | + | assert.NotEqual(t, relayTab.ID, h.tabs.ActiveTabID) | |
| 133 | + | assert.Equal(t, 1, h.app.externalRelayPanes.Len(), "stale entry replaced by the new pane") | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func TestApp_OpenExternalURLCreatesNewPaneWhenRelayPaneCannotNavigate(t *testing.T) { | |
| 137 | + | ctx := context.Background() | |
| 138 | + | h := newRelayReuseHarness(t) | |
| 139 | + | ||
| 140 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 141 | + | relayPaneID := h.tabs.ActiveTab().Workspace.ActivePane().Pane.ID | |
| 142 | + | // The pane still exists but its WebView is gone, so navigation fails. | |
| 143 | + | h.app.contentCoord.ReleaseWebView(ctx, relayPaneID) | |
| 144 | + | ||
| 145 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |
| 146 | + | ||
| 147 | + | assert.Len(t, h.tabs.Tabs, 3) | |
| 148 | + | assert.Equal(t, 1, h.app.externalRelayPanes.Len(), "failed entry replaced by the new pane") | |
| 149 | + | } | |
| @@ -0,0 +1,149 @@ | |||||
| 1 | + | package ui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 9 | + | "github.com/bnema/dumber/internal/application/usecase" | |||
| 10 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 11 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |||
| 12 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |||
| 13 | + | "github.com/bnema/dumber/internal/ui/component" | |||
| 14 | + | "github.com/bnema/dumber/internal/ui/coordinator" | |||
| 15 | + | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | |||
| 16 | + | "github.com/stretchr/testify/assert" | |||
| 17 | + | "github.com/stretchr/testify/require" | |||
| 18 | + | ) | |||
| 19 | + | ||||
| 20 | + | const relayTestURL = "https://claude.com/cai/login/app-google-auth?open_in_browser=1&hop_nonce=XYZ" | |||
| 21 | + | ||||
| 22 | + | type relayReuseHarness struct { | |||
| 23 | + | app *App | |||
| 24 | + | tabs *entity.TabList | |||
| 25 | + | window *browserWindow | |||
| 26 | + | webview map[entity.PaneID]*recordingWebView | |||
| 27 | + | clock *time.Time | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | // newRelayReuseHarness builds an App with one window holding one tab, using the | |||
| 31 | + | // tabbed external-link behavior. New tabs get a recording webview registered so | |||
| 32 | + | // reuse can navigate them. | |||
| 33 | + | func newRelayReuseHarness(t *testing.T) *relayReuseHarness { | |||
| 34 | + | t.Helper() | |||
| 35 | + | ctx := context.Background() | |||
| 36 | + | ||||
| 37 | + | original := entity.NewTab("tab-1", "ws-1", entity.NewPane("pane-1")) | |||
| 38 | + | tabs := entity.NewTabList() | |||
| 39 | + | tabs.Add(original) | |||
| 40 | + | window := &browserWindow{id: "focused", tabs: tabs} | |||
| 41 | + | ||||
| 42 | + | ids := []string{"tab-2", "ws-2", "pane-2", "tab-3", "ws-3", "pane-3"} | |||
| 43 | + | index := 0 | |||
| 44 | + | now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) | |||
| 45 | + | h := &relayReuseHarness{tabs: tabs, window: window, webview: map[entity.PaneID]*recordingWebView{}, clock: &now} | |||
| 46 | + | ||||
| 47 | + | contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil) | |||
| 48 | + | app := &App{ | |||
| 49 | + | runtimeConfig: runtimeConfigStateForTest(&config.Config{Workspace: entity.WorkspaceConfig{ExternalLinks: entity.ExternalLinksConfig{Behavior: entity.ExternalLinkBehaviorTabbed}}}), | |||
| 50 | + | tabsUC: usecase.NewManageTabsUseCase(func() string { id := ids[index]; index++; return id }, nil), | |||
| 51 | + | browserWindows: map[string]*browserWindow{ | |||
| 52 | + | window.id: window, | |||
| 53 | + | }, | |||
| 54 | + | browserWindowOrder: []string{window.id}, | |||
| 55 | + | lastFocusedWindowID: window.id, | |||
| 56 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{}, | |||
| 57 | + | windowForTab: map[entity.TabID]*browserWindow{}, | |||
| 58 | + | contentCoord: contentCoord, | |||
| 59 | + | navCoord: coordinator.NewNavigationCoordinator(ctx, nil, contentCoord), | |||
| 60 | + | } | |||
| 61 | + | app.externalRelayPanes.Now = func() time.Time { return *h.clock } | |||
| 62 | + | app.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{TabsUC: app.tabsUC}) | |||
| 63 | + | app.tabCoord.SetOnTabCreated(func(_ context.Context, _ coordinator.TabTarget, tab *entity.Tab) { | |||
| 64 | + | app.workspaceViews[tab.ID] = &component.WorkspaceView{} | |||
| 65 | + | app.setBrowserWindowForTab(tab.ID, window) | |||
| 66 | + | paneID := tab.Workspace.ActivePane().Pane.ID | |||
| 67 | + | wv := &recordingWebView{id: port.WebViewID(100 + len(h.webview))} | |||
| 68 | + | h.webview[paneID] = wv | |||
| 69 | + | contentCoord.RegisterPopupWebView(paneID, wv) | |||
| 70 | + | }) | |||
| 71 | + | h.app = app | |||
| 72 | + | return h | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func TestApp_OpenExternalURLReusesRecentRelayPane(t *testing.T) { | |||
| 76 | + | ctx := context.Background() | |||
| 77 | + | h := newRelayReuseHarness(t) | |||
| 78 | + | ||||
| 79 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 80 | + | require.Len(t, h.tabs.Tabs, 2) | |||
| 81 | + | relayTab := h.tabs.ActiveTab() | |||
| 82 | + | relayPaneID := relayTab.Workspace.ActivePane().Pane.ID | |||
| 83 | + | assert.False(t, h.webview[relayPaneID].loadURICalled) | |||
| 84 | + | ||||
| 85 | + | // The user moves on to another tab, then the app retries the same URL. | |||
| 86 | + | h.tabs.SetActive("tab-1") | |||
| 87 | + | *h.clock = h.clock.Add(30 * time.Second) | |||
| 88 | + | ||||
| 89 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 90 | + | ||||
| 91 | + | assert.Len(t, h.tabs.Tabs, 2, "no new tab must be created") | |||
| 92 | + | assert.Equal(t, relayTab.ID, h.tabs.ActiveTabID, "relay tab must be focused again") | |||
| 93 | + | assert.True(t, h.webview[relayPaneID].loadURICalled) | |||
| 94 | + | assert.Equal(t, relayTestURL, h.webview[relayPaneID].loadURILastURI) | |||
| 95 | + | } | |||
| 96 | + | ||||
| 97 | + | func TestApp_OpenExternalURLCreatesNewPaneAfterReuseWindowExpires(t *testing.T) { | |||
| 98 | + | ctx := context.Background() | |||
| 99 | + | h := newRelayReuseHarness(t) | |||
| 100 | + | ||||
| 101 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 102 | + | require.Len(t, h.tabs.Tabs, 2) | |||
| 103 | + | ||||
| 104 | + | *h.clock = h.clock.Add(domainurl.RelayReuseWindow + time.Second) | |||
| 105 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 106 | + | ||||
| 107 | + | assert.Len(t, h.tabs.Tabs, 3) | |||
| 108 | + | } | |||
| 109 | + | ||||
| 110 | + | func TestApp_OpenExternalURLCreatesNewPaneForDifferentURL(t *testing.T) { | |||
| 111 | + | ctx := context.Background() | |||
| 112 | + | h := newRelayReuseHarness(t) | |||
| 113 | + | ||||
| 114 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 115 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL+"2")) | |||
| 116 | + | ||||
| 117 | + | assert.Len(t, h.tabs.Tabs, 3) | |||
| 118 | + | } | |||
| 119 | + | ||||
| 120 | + | func TestApp_OpenExternalURLCreatesNewPaneWhenRelayPaneWasClosed(t *testing.T) { | |||
| 121 | + | ctx := context.Background() | |||
| 122 | + | h := newRelayReuseHarness(t) | |||
| 123 | + | ||||
| 124 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 125 | + | relayTab := h.tabs.ActiveTab() | |||
| 126 | + | h.tabs.Remove(relayTab.ID) | |||
| 127 | + | require.Len(t, h.tabs.Tabs, 1) | |||
| 128 | + | ||||
| 129 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 130 | + | ||||
| 131 | + | assert.Len(t, h.tabs.Tabs, 2) | |||
| 132 | + | assert.NotEqual(t, relayTab.ID, h.tabs.ActiveTabID) | |||
| 133 | + | assert.Equal(t, 1, h.app.externalRelayPanes.Len(), "stale entry replaced by the new pane") | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func TestApp_OpenExternalURLCreatesNewPaneWhenRelayPaneCannotNavigate(t *testing.T) { | |||
| 137 | + | ctx := context.Background() | |||
| 138 | + | h := newRelayReuseHarness(t) | |||
| 139 | + | ||||
| 140 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 141 | + | relayPaneID := h.tabs.ActiveTab().Workspace.ActivePane().Pane.ID | |||
| 142 | + | // The pane still exists but its WebView is gone, so navigation fails. | |||
| 143 | + | h.app.contentCoord.ReleaseWebView(ctx, relayPaneID) | |||
| 144 | + | ||||
| 145 | + | require.NoError(t, h.app.OpenExternalURL(ctx, relayTestURL)) | |||
| 146 | + | ||||
| 147 | + | assert.Len(t, h.tabs.Tabs, 3) | |||
| 148 | + | assert.Equal(t, 1, h.app.externalRelayPanes.Len(), "failed entry replaced by the new pane") | |||
| 149 | + | } | |||