detached 192c70d

test(ui): verify floating popup window ownership

commit
192c70d947b429c178ca91cf4de164e957a24e0a
author
brice · · 2026-07-25 06:32 UTC
committer
brice · · 2026-07-26 16:42 UTC
parent
ec8878f
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +239 -58
M internal/ui/browser_popup_window.go +9 -3
@@ -3,12 +3,19 @@ package ui
33 import (
44 "context"
55 "fmt"
6+ "sync"
67
78 "github.com/bnema/dumber/internal/application/port"
89 "github.com/bnema/dumber/internal/domain/entity"
910 "github.com/bnema/dumber/internal/ui/coordinator/content"
1011 )
1112
13+var showPopupBrowserWindow = func(bw *browserWindow) {
14+ if bw != nil && bw.mainWindow != nil {
15+ bw.mainWindow.Show()
16+ }
17+}
18+
1219 // openPopupBrowserWindow adopts the exact related WebView created for a
1320 // browsing-context request into a complete Dumber browser window.
1421 func (a *App) openPopupBrowserWindow(ctx context.Context, input content.BrowserWindowInput) (content.BrowserWindowResult, error) {
@@ -33,10 +40,9 @@ func (a *App) openPopupBrowserWindow(ctx context.Context, input content.BrowserW
3340 a.setBrowserWindowForTab(tab.ID, bw)
3441 a.activateBrowserWindow(bw)
3542
43+ var showOnce sync.Once
3644 show := func() {
37- if bw.mainWindow != nil {
38- bw.mainWindow.Show()
39- }
45+ showOnce.Do(func() { showPopupBrowserWindow(bw) })
4046 }
4147 if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok {
4248 lifecycle.SetOnClose(func() { a.closeAdoptedPopupTab(context.Background(), bw, tab.ID) })
@@ -3,12 +3,19 @@ package ui
3 import (3 import (
4 "context"4 "context"
5 "fmt"5 "fmt"
6+ "sync"
6 7
7 "github.com/bnema/dumber/internal/application/port"8 "github.com/bnema/dumber/internal/application/port"
8 "github.com/bnema/dumber/internal/domain/entity"9 "github.com/bnema/dumber/internal/domain/entity"
9 "github.com/bnema/dumber/internal/ui/coordinator/content"10 "github.com/bnema/dumber/internal/ui/coordinator/content"
10 )11 )
11 12
13+var showPopupBrowserWindow = func(bw *browserWindow) {
14+ if bw != nil && bw.mainWindow != nil {
15+ bw.mainWindow.Show()
16+ }
17+}
18+
12 // openPopupBrowserWindow adopts the exact related WebView created for a19 // openPopupBrowserWindow adopts the exact related WebView created for a
13 // browsing-context request into a complete Dumber browser window.20 // browsing-context request into a complete Dumber browser window.
14 func (a *App) openPopupBrowserWindow(ctx context.Context, input content.BrowserWindowInput) (content.BrowserWindowResult, error) {21 func (a *App) openPopupBrowserWindow(ctx context.Context, input content.BrowserWindowInput) (content.BrowserWindowResult, error) {
@@ -33,10 +40,9 @@ func (a *App) openPopupBrowserWindow(ctx context.Context, input content.BrowserW
33 a.setBrowserWindowForTab(tab.ID, bw)40 a.setBrowserWindowForTab(tab.ID, bw)
34 a.activateBrowserWindow(bw)41 a.activateBrowserWindow(bw)
35 42
43+ var showOnce sync.Once
36 show := func() {44 show := func() {
37- if bw.mainWindow != nil {45+ showOnce.Do(func() { showPopupBrowserWindow(bw) })
38- bw.mainWindow.Show()
39- }
40 }46 }
41 if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok {47 if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok {
42 lifecycle.SetOnClose(func() { a.closeAdoptedPopupTab(context.Background(), bw, tab.ID) })48 lifecycle.SetOnClose(func() { a.closeAdoptedPopupTab(context.Background(), bw, tab.ID) })
M internal/ui/browser_popup_window_test.go +49 -2
@@ -33,6 +33,9 @@ func newPopupAdoptionApp(t *testing.T, attach func(context.Context, entity.TabID
3333 delete(app.workspaceViews, tab.ID)
3434 app.tabs.Remove(tab.ID)
3535 })
36+ app.tabCoord.SetOnCurrentWindowEmpty(func(context.Context, coordinator.TabTarget) {
37+ app.cleanupEjectTargetWindow(bw)
38+ })
3639 return app, bw
3740 }
3841
@@ -66,7 +69,12 @@ func TestOpenPopupBrowserWindowAdoptsProvidedWebView(t *testing.T) {
6669 assert.Same(t, bw, app.windowForTab["adopted-tab"])
6770 }
6871
69-func TestOpenPopupBrowserWindowWaitsForReadySignalWhenNotReady(t *testing.T) {
72+func TestOpenPopupBrowserWindowWaitsForReadySignalAndShowsExactlyOnce(t *testing.T) {
73+ previousShow := showPopupBrowserWindow
74+ showCalls := 0
75+ showPopupBrowserWindow = func(*browserWindow) { showCalls++ }
76+ t.Cleanup(func() { showPopupBrowserWindow = previousShow })
77+
7078 wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}
7179 app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
7280
@@ -77,9 +85,18 @@ func TestOpenPopupBrowserWindowWaitsForReadySignalWhenNotReady(t *testing.T) {
7785 require.NoError(t, err)
7886 require.NotNil(t, wv.onReady)
7987 assert.NotNil(t, wv.onClose)
88+ assert.Zero(t, showCalls)
89+ wv.onReady()
90+ wv.onReady()
91+ assert.Equal(t, 1, showCalls)
8092 }
8193
82-func TestOpenPopupBrowserWindowDoesNotWaitForReadyWhenAlreadyReady(t *testing.T) {
94+func TestOpenPopupBrowserWindowShowsImmediatelyWhenAlreadyReady(t *testing.T) {
95+ previousShow := showPopupBrowserWindow
96+ showCalls := 0
97+ showPopupBrowserWindow = func(*browserWindow) { showCalls++ }
98+ t.Cleanup(func() { showPopupBrowserWindow = previousShow })
99+
83100 wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}
84101 app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
85102
@@ -90,6 +107,36 @@ func TestOpenPopupBrowserWindowDoesNotWaitForReadyWhenAlreadyReady(t *testing.T)
90107 require.NoError(t, err)
91108 assert.Nil(t, wv.onReady)
92109 assert.NotNil(t, wv.onClose)
110+ assert.Equal(t, 1, showCalls)
111+}
112+
113+func TestOpenPopupBrowserWindowTransferredWebViewIsNotDestroyed(t *testing.T) {
114+ wv := &recordingWebView{id: 99}
115+ app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
116+
117+ _, err := app.openPopupBrowserWindow(context.Background(), content.BrowserWindowInput{
118+ PopupPane: entity.NewPane("popup-pane"), PopupWebView: wv, Ready: true,
119+ })
120+
121+ require.NoError(t, err)
122+ assert.Zero(t, wv.destroyCalls)
123+}
124+
125+func TestOpenPopupBrowserWindowCloseRequestClosesAdoptedLastTabAndWindow(t *testing.T) {
126+ wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}
127+ app, bw := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
128+
129+ _, err := app.openPopupBrowserWindow(context.Background(), content.BrowserWindowInput{
130+ PopupPane: entity.NewPane("popup-pane"), PopupWebView: wv, Ready: true,
131+ })
132+ require.NoError(t, err)
133+ require.NotNil(t, wv.onClose)
134+
135+ wv.onClose()
136+
137+ assert.Zero(t, bw.tabs.Count())
138+ assert.False(t, app.hasBrowserWindow(bw))
139+ assert.Empty(t, app.windowForTab)
93140 }
94141
95142 func TestOpenPopupBrowserWindowRollsBackShellWhenCreateWithPaneFails(t *testing.T) {
@@ -33,6 +33,9 @@ func newPopupAdoptionApp(t *testing.T, attach func(context.Context, entity.TabID
33 delete(app.workspaceViews, tab.ID)33 delete(app.workspaceViews, tab.ID)
34 app.tabs.Remove(tab.ID)34 app.tabs.Remove(tab.ID)
35 })35 })
36+ app.tabCoord.SetOnCurrentWindowEmpty(func(context.Context, coordinator.TabTarget) {
37+ app.cleanupEjectTargetWindow(bw)
38+ })
36 return app, bw39 return app, bw
37 }40 }
38 41
@@ -66,7 +69,12 @@ func TestOpenPopupBrowserWindowAdoptsProvidedWebView(t *testing.T) {
66 assert.Same(t, bw, app.windowForTab["adopted-tab"])69 assert.Same(t, bw, app.windowForTab["adopted-tab"])
67 }70 }
68 71
69-func TestOpenPopupBrowserWindowWaitsForReadySignalWhenNotReady(t *testing.T) {72+func TestOpenPopupBrowserWindowWaitsForReadySignalAndShowsExactlyOnce(t *testing.T) {
73+ previousShow := showPopupBrowserWindow
74+ showCalls := 0
75+ showPopupBrowserWindow = func(*browserWindow) { showCalls++ }
76+ t.Cleanup(func() { showPopupBrowserWindow = previousShow })
77+
70 wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}78 wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}
71 app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })79 app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
72 80
@@ -77,9 +85,18 @@ func TestOpenPopupBrowserWindowWaitsForReadySignalWhenNotReady(t *testing.T) {
77 require.NoError(t, err)85 require.NoError(t, err)
78 require.NotNil(t, wv.onReady)86 require.NotNil(t, wv.onReady)
79 assert.NotNil(t, wv.onClose)87 assert.NotNil(t, wv.onClose)
88+ assert.Zero(t, showCalls)
89+ wv.onReady()
90+ wv.onReady()
91+ assert.Equal(t, 1, showCalls)
80 }92 }
81 93
82-func TestOpenPopupBrowserWindowDoesNotWaitForReadyWhenAlreadyReady(t *testing.T) {94+func TestOpenPopupBrowserWindowShowsImmediatelyWhenAlreadyReady(t *testing.T) {
95+ previousShow := showPopupBrowserWindow
96+ showCalls := 0
97+ showPopupBrowserWindow = func(*browserWindow) { showCalls++ }
98+ t.Cleanup(func() { showPopupBrowserWindow = previousShow })
99+
83 wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}100 wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}
84 app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })101 app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
85 102
@@ -90,6 +107,36 @@ func TestOpenPopupBrowserWindowDoesNotWaitForReadyWhenAlreadyReady(t *testing.T)
90 require.NoError(t, err)107 require.NoError(t, err)
91 assert.Nil(t, wv.onReady)108 assert.Nil(t, wv.onReady)
92 assert.NotNil(t, wv.onClose)109 assert.NotNil(t, wv.onClose)
110+ assert.Equal(t, 1, showCalls)
111+}
112+
113+func TestOpenPopupBrowserWindowTransferredWebViewIsNotDestroyed(t *testing.T) {
114+ wv := &recordingWebView{id: 99}
115+ app, _ := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
116+
117+ _, err := app.openPopupBrowserWindow(context.Background(), content.BrowserWindowInput{
118+ PopupPane: entity.NewPane("popup-pane"), PopupWebView: wv, Ready: true,
119+ })
120+
121+ require.NoError(t, err)
122+ assert.Zero(t, wv.destroyCalls)
123+}
124+
125+func TestOpenPopupBrowserWindowCloseRequestClosesAdoptedLastTabAndWindow(t *testing.T) {
126+ wv := &popupLifecycleWebView{MockWebView: portmocks.NewMockWebView(t)}
127+ app, bw := newPopupAdoptionApp(t, func(context.Context, entity.TabID, *entity.Pane, port.WebView) error { return nil })
128+
129+ _, err := app.openPopupBrowserWindow(context.Background(), content.BrowserWindowInput{
130+ PopupPane: entity.NewPane("popup-pane"), PopupWebView: wv, Ready: true,
131+ })
132+ require.NoError(t, err)
133+ require.NotNil(t, wv.onClose)
134+
135+ wv.onClose()
136+
137+ assert.Zero(t, bw.tabs.Count())
138+ assert.False(t, app.hasBrowserWindow(bw))
139+ assert.Empty(t, app.windowForTab)
93 }140 }
94 141
95 func TestOpenPopupBrowserWindowRollsBackShellWhenCreateWithPaneFails(t *testing.T) {142 func TestOpenPopupBrowserWindowRollsBackShellWhenCreateWithPaneFails(t *testing.T) {
A internal/ui/coordinator/content/popup_hosting.go +66 -0
@@ -0,0 +1,66 @@
1+package content
2+
3+import (
4+ "context"
5+ "fmt"
6+
7+ "github.com/bnema/dumber/internal/application/dto"
8+ "github.com/bnema/dumber/internal/application/port"
9+ "github.com/bnema/dumber/internal/domain/entity"
10+ "github.com/bnema/dumber/internal/logging"
11+)
12+
13+// openBrowserWindow transfers a related WebView to a complete browser window.
14+// The popup manager retains ownership until the host callback succeeds.
15+func (pm *popupManager) openBrowserWindow(
16+ ctx context.Context,
17+ hooks popupCoordinatorHooks,
18+ parentPaneID entity.PaneID,
19+ parentWebViewID port.WebViewID,
20+ req port.PopupRequest,
21+ decision dto.HostDecision,
22+ ready bool,
23+) port.WebView {
24+ log := logging.FromContext(ctx)
25+ normalized := buildPopupBrowsingContextRequest(req)
26+ normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
27+ if pm.onOpenBrowserWindow == nil {
28+ logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostUnavailable, nil)
29+ return nil
30+ }
31+ popupWV, err := pm.createPopupWebView(ctx, parentWebViewID, req.TargetURI, true)
32+ if err != nil {
33+ logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
34+ return nil
35+ }
36+ pm.setBrowsingContextDecision(popupWV, decision)
37+ paneID, popupPane := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
38+ if hooks.setupWebViewCallbacks != nil {
39+ hooks.setupWebViewCallbacks(ctx, paneID, popupWV)
40+ }
41+ result, err := pm.onOpenBrowserWindow(ctx, BrowserWindowInput{
42+ ParentPaneID: parentPaneID, ParentWebViewID: parentWebViewID, PopupPane: popupPane,
43+ PopupWebView: popupWV, TargetURI: req.TargetURI, Request: req, Ready: ready,
44+ })
45+ if err != nil || result.WindowID == "" {
46+ if err == nil {
47+ err = fmt.Errorf("browser window host returned an empty window ID")
48+ }
49+ logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
50+ popupWV.Destroy()
51+ return nil
52+ }
53+ if !req.NoJavaScriptAccess {
54+ pm.storeReusableNamedPopupWithHost(parentPaneID, req.FrameName, paneID, popupWV, result.WindowID)
55+ }
56+ if lifecycle, ok := popupWV.(port.PopupLifecycleCapable); ok {
57+ lifecycle.PrimePopupNavigation(req.TargetURI)
58+ }
59+ return popupWV
60+}
61+
62+func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {
63+ // Deferred WebKit requests are staged and routed by a later phase. Every
64+ // immediate route must be visible without depending on another signal.
65+ return true
66+}
@@ -0,0 +1,66 @@
1+package content
2+
3+import (
4+ "context"
5+ "fmt"
6+
7+ "github.com/bnema/dumber/internal/application/dto"
8+ "github.com/bnema/dumber/internal/application/port"
9+ "github.com/bnema/dumber/internal/domain/entity"
10+ "github.com/bnema/dumber/internal/logging"
11+)
12+
13+// openBrowserWindow transfers a related WebView to a complete browser window.
14+// The popup manager retains ownership until the host callback succeeds.
15+func (pm *popupManager) openBrowserWindow(
16+ ctx context.Context,
17+ hooks popupCoordinatorHooks,
18+ parentPaneID entity.PaneID,
19+ parentWebViewID port.WebViewID,
20+ req port.PopupRequest,
21+ decision dto.HostDecision,
22+ ready bool,
23+) port.WebView {
24+ log := logging.FromContext(ctx)
25+ normalized := buildPopupBrowsingContextRequest(req)
26+ normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
27+ if pm.onOpenBrowserWindow == nil {
28+ logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostUnavailable, nil)
29+ return nil
30+ }
31+ popupWV, err := pm.createPopupWebView(ctx, parentWebViewID, req.TargetURI, true)
32+ if err != nil {
33+ logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
34+ return nil
35+ }
36+ pm.setBrowsingContextDecision(popupWV, decision)
37+ paneID, popupPane := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
38+ if hooks.setupWebViewCallbacks != nil {
39+ hooks.setupWebViewCallbacks(ctx, paneID, popupWV)
40+ }
41+ result, err := pm.onOpenBrowserWindow(ctx, BrowserWindowInput{
42+ ParentPaneID: parentPaneID, ParentWebViewID: parentWebViewID, PopupPane: popupPane,
43+ PopupWebView: popupWV, TargetURI: req.TargetURI, Request: req, Ready: ready,
44+ })
45+ if err != nil || result.WindowID == "" {
46+ if err == nil {
47+ err = fmt.Errorf("browser window host returned an empty window ID")
48+ }
49+ logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
50+ popupWV.Destroy()
51+ return nil
52+ }
53+ if !req.NoJavaScriptAccess {
54+ pm.storeReusableNamedPopupWithHost(parentPaneID, req.FrameName, paneID, popupWV, result.WindowID)
55+ }
56+ if lifecycle, ok := popupWV.(port.PopupLifecycleCapable); ok {
57+ lifecycle.PrimePopupNavigation(req.TargetURI)
58+ }
59+ return popupWV
60+}
61+
62+func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {
63+ // Deferred WebKit requests are staged and routed by a later phase. Every
64+ // immediate route must be visible without depending on another signal.
65+ return true
66+}
M internal/ui/coordinator/content/popup_manager.go +0 -53
@@ -512,59 +512,6 @@ func (pm *popupManager) popupParentURIAtOpen(
512512 return parentURIAtOpen
513513 }
514514
515-func (pm *popupManager) openBrowserWindow(
516- ctx context.Context,
517- hooks popupCoordinatorHooks,
518- parentPaneID entity.PaneID,
519- parentWebViewID port.WebViewID,
520- req port.PopupRequest,
521- decision dto.HostDecision,
522- ready bool,
523-) port.WebView {
524- log := logging.FromContext(ctx)
525- normalized := buildPopupBrowsingContextRequest(req)
526- normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
527- if pm.onOpenBrowserWindow == nil {
528- logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostUnavailable, nil)
529- return nil
530- }
531- popupWV, err := pm.createPopupWebView(ctx, parentWebViewID, req.TargetURI, true)
532- if err != nil {
533- logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
534- return nil
535- }
536- pm.setBrowsingContextDecision(popupWV, decision)
537- paneID, popupPane := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
538- if hooks.setupWebViewCallbacks != nil {
539- hooks.setupWebViewCallbacks(ctx, paneID, popupWV)
540- }
541- result, err := pm.onOpenBrowserWindow(ctx, BrowserWindowInput{
542- ParentPaneID: parentPaneID, ParentWebViewID: parentWebViewID, PopupPane: popupPane,
543- PopupWebView: popupWV, TargetURI: req.TargetURI, Request: req, Ready: ready,
544- })
545- if err != nil || result.WindowID == "" {
546- if err == nil {
547- err = fmt.Errorf("browser window host returned an empty window ID")
548- }
549- logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
550- popupWV.Destroy()
551- return nil
552- }
553- if !req.NoJavaScriptAccess {
554- pm.storeReusableNamedPopupWithHost(parentPaneID, req.FrameName, paneID, popupWV, result.WindowID)
555- }
556- if lifecycle, ok := popupWV.(port.PopupLifecycleCapable); ok {
557- lifecycle.PrimePopupNavigation(req.TargetURI)
558- }
559- return popupWV
560-}
561-
562-func (pm *popupManager) readyOnCreate(engine dto.BrowserEngineKind) bool {
563- // Deferred WebKit requests are staged and routed by a later phase. Every
564- // immediate route must be visible without depending on another signal.
565- return true
566-}
567-
568515 func (pm *popupManager) openNativePopup(
569516 ctx context.Context,
570517 parentPaneID entity.PaneID,
@@ -512,59 +512,6 @@ func (pm *popupManager) popupParentURIAtOpen(
512 return parentURIAtOpen512 return parentURIAtOpen
513 }513 }
514 514
515-func (pm *popupManager) openBrowserWindow(
516- ctx context.Context,
517- hooks popupCoordinatorHooks,
518- parentPaneID entity.PaneID,
519- parentWebViewID port.WebViewID,
520- req port.PopupRequest,
521- decision dto.HostDecision,
522- ready bool,
523-) port.WebView {
524- log := logging.FromContext(ctx)
525- normalized := buildPopupBrowsingContextRequest(req)
526- normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
527- if pm.onOpenBrowserWindow == nil {
528- logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostUnavailable, nil)
529- return nil
530- }
531- popupWV, err := pm.createPopupWebView(ctx, parentWebViewID, req.TargetURI, true)
532- if err != nil {
533- logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
534- return nil
535- }
536- pm.setBrowsingContextDecision(popupWV, decision)
537- paneID, popupPane := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
538- if hooks.setupWebViewCallbacks != nil {
539- hooks.setupWebViewCallbacks(ctx, paneID, popupWV)
540- }
541- result, err := pm.onOpenBrowserWindow(ctx, BrowserWindowInput{
542- ParentPaneID: parentPaneID, ParentWebViewID: parentWebViewID, PopupPane: popupPane,
543- PopupWebView: popupWV, TargetURI: req.TargetURI, Request: req, Ready: ready,
544- })
545- if err != nil || result.WindowID == "" {
546- if err == nil {
547- err = fmt.Errorf("browser window host returned an empty window ID")
548- }
549- logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
550- popupWV.Destroy()
551- return nil
552- }
553- if !req.NoJavaScriptAccess {
554- pm.storeReusableNamedPopupWithHost(parentPaneID, req.FrameName, paneID, popupWV, result.WindowID)
555- }
556- if lifecycle, ok := popupWV.(port.PopupLifecycleCapable); ok {
557- lifecycle.PrimePopupNavigation(req.TargetURI)
558- }
559- return popupWV
560-}
561-
562-func (pm *popupManager) readyOnCreate(engine dto.BrowserEngineKind) bool {
563- // Deferred WebKit requests are staged and routed by a later phase. Every
564- // immediate route must be visible without depending on another signal.
565- return true
566-}
567-
568 func (pm *popupManager) openNativePopup(515 func (pm *popupManager) openNativePopup(
569 ctx context.Context,516 ctx context.Context,
570 parentPaneID entity.PaneID,517 parentPaneID entity.PaneID,
M internal/ui/coordinator/content/popup_test.go +115 -0
@@ -230,6 +230,121 @@ func TestPopupWorkspaceBlankRemainsBlockedWhenWorkspacePopupsDisabled(t *testing
230230 assert.Nil(t, got)
231231 }
232232
233+func TestPopupWorkspaceMiddleClickRemainsBlockedWhenWorkspacePopupsDisabled(t *testing.T) {
234+ c := &Coordinator{popups: newPopupManager()}
235+ c.SetPopupConfig(mocks.NewMockWebViewFactory(t), &entity.BrowsingContextConfig{OpenInNewPane: false}, nil)
236+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostWorkspace })
237+
238+ assert.False(t, c.handleLinkMiddleClick(context.Background(), "workspace", "https://example.com/middle"))
239+}
240+
241+func TestPopupFloatingNamedContextReusesDetachedWindow(t *testing.T) {
242+ ctx := context.Background()
243+ parentPaneID := entity.PaneID("floating")
244+ parentWV := mocks.NewMockWebView(t)
245+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice()
246+ popupWV := mocks.NewMockWebView(t)
247+ popupWV.EXPECT().ID().Return(port.WebViewID(201)).Maybe()
248+ popupWV.EXPECT().Generation().Return(uint64(1)).Maybe()
249+ popupWV.EXPECT().SetCallbacks(mock.Anything).Once()
250+ popupWV.EXPECT().IsDestroyed().Return(false).Twice()
251+ popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/second").Return(nil).Once()
252+
253+ factory := mocks.NewMockWebViewFactory(t)
254+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
255+ c := &Coordinator{webViews: make(map[entity.PaneID]port.WebView), popups: newPopupManager()}
256+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, func() string { return "detached-pane" })
257+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
258+ windowByPane := map[entity.PaneID]string{parentPaneID: "owner-window"}
259+ c.SetPopupWindowIDResolver(func(paneID entity.PaneID) (string, bool) {
260+ windowID, ok := windowByPane[paneID]
261+ return windowID, ok
262+ })
263+ hostCalls := 0
264+ c.SetOnOpenBrowserWindow(func(_ context.Context, input BrowserWindowInput) (BrowserWindowResult, error) {
265+ hostCalls++
266+ windowByPane[input.PopupPane.ID] = "detached-window"
267+ c.RegisterPopupWebView(input.PopupPane.ID, input.PopupWebView)
268+ return BrowserWindowResult{WindowID: "detached-window"}, nil
269+ })
270+
271+ first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{
272+ Engine: dto.BrowserEngineCEF, TargetURI: "https://example.com/first", FrameName: "shared",
273+ TargetDisposition: dto.WindowDispositionNewPopup, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
274+ })
275+ second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{
276+ Engine: dto.BrowserEngineCEF, TargetURI: "https://example.com/second", FrameName: "shared",
277+ TargetDisposition: dto.WindowDispositionNewPopup, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
278+ })
279+
280+ assert.Same(t, popupWV, first)
281+ assert.Same(t, popupWV, second)
282+ assert.Equal(t, 1, hostCalls)
283+}
284+
285+func TestPopupFloatingNoopenerDoesNotReuseDetachedNamedContext(t *testing.T) {
286+ ctx := context.Background()
287+ parentPaneID := entity.PaneID("floating")
288+ parentWV := mocks.NewMockWebView(t)
289+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice()
290+ firstWV := mocks.NewMockWebView(t)
291+ firstWV.EXPECT().ID().Return(port.WebViewID(201)).Maybe()
292+ firstWV.EXPECT().Generation().Return(uint64(1)).Maybe()
293+ firstWV.EXPECT().SetCallbacks(mock.Anything).Once()
294+ secondWV := mocks.NewMockWebView(t)
295+ secondWV.EXPECT().ID().Return(port.WebViewID(202)).Maybe()
296+ secondWV.EXPECT().Generation().Return(uint64(1)).Maybe()
297+ secondWV.EXPECT().SetCallbacks(mock.Anything).Once()
298+ factory := mocks.NewMockWebViewFactory(t)
299+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(firstWV, nil).Once()
300+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(secondWV, nil).Once()
301+ c := &Coordinator{popups: newPopupManager()}
302+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, func() string { return "detached-pane" })
303+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
304+ hostCalls := 0
305+ c.SetOnOpenBrowserWindow(func(_ context.Context, _ BrowserWindowInput) (BrowserWindowResult, error) {
306+ hostCalls++
307+ return BrowserWindowResult{WindowID: "detached-window"}, nil
308+ })
309+ req := port.PopupRequest{
310+ Engine: dto.BrowserEngineCEF, FrameName: "shared", NoJavaScriptAccess: true,
311+ TargetDisposition: dto.WindowDispositionNewPopup, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
312+ }
313+ req.TargetURI = "https://example.com/first"
314+ first := c.handlePopupCreate(ctx, parentPaneID, parentWV, req)
315+ req.TargetURI = "https://example.com/second"
316+ second := c.handlePopupCreate(ctx, parentPaneID, parentWV, req)
317+
318+ assert.Same(t, firstWV, first)
319+ assert.Same(t, secondWV, second)
320+ assert.Equal(t, 2, hostCalls)
321+}
322+
323+func TestPopupFloatingBrowserHostFailureDestroysWebViewExactlyOnce(t *testing.T) {
324+ parentWV := mocks.NewMockWebView(t)
325+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once()
326+ popupWV := mocks.NewMockWebView(t)
327+ popupWV.EXPECT().ID().Return(port.WebViewID(201)).Maybe()
328+ popupWV.EXPECT().Generation().Return(uint64(1)).Maybe()
329+ popupWV.EXPECT().SetCallbacks(mock.Anything).Once()
330+ popupWV.EXPECT().Destroy().Once()
331+ factory := mocks.NewMockWebViewFactory(t)
332+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
333+ c := &Coordinator{popups: newPopupManager()}
334+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, func() string { return "detached-pane" })
335+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
336+ c.SetOnOpenBrowserWindow(func(context.Context, BrowserWindowInput) (BrowserWindowResult, error) {
337+ return BrowserWindowResult{}, errors.New("host failed")
338+ })
339+
340+ got := c.handlePopupCreate(context.Background(), "floating", parentWV, port.PopupRequest{
341+ Engine: dto.BrowserEngineCEF, TargetURI: "https://example.com/new", FrameName: "_blank",
342+ TargetDisposition: dto.WindowDispositionNewTab, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
343+ })
344+
345+ assert.Nil(t, got)
346+}
347+
233348 func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) {
234349 ctx := context.Background()
235350 parentPaneID := entity.PaneID("parent-pane")
@@ -230,6 +230,121 @@ func TestPopupWorkspaceBlankRemainsBlockedWhenWorkspacePopupsDisabled(t *testing
230 assert.Nil(t, got)230 assert.Nil(t, got)
231 }231 }
232 232
233+func TestPopupWorkspaceMiddleClickRemainsBlockedWhenWorkspacePopupsDisabled(t *testing.T) {
234+ c := &Coordinator{popups: newPopupManager()}
235+ c.SetPopupConfig(mocks.NewMockWebViewFactory(t), &entity.BrowsingContextConfig{OpenInNewPane: false}, nil)
236+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostWorkspace })
237+
238+ assert.False(t, c.handleLinkMiddleClick(context.Background(), "workspace", "https://example.com/middle"))
239+}
240+
241+func TestPopupFloatingNamedContextReusesDetachedWindow(t *testing.T) {
242+ ctx := context.Background()
243+ parentPaneID := entity.PaneID("floating")
244+ parentWV := mocks.NewMockWebView(t)
245+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice()
246+ popupWV := mocks.NewMockWebView(t)
247+ popupWV.EXPECT().ID().Return(port.WebViewID(201)).Maybe()
248+ popupWV.EXPECT().Generation().Return(uint64(1)).Maybe()
249+ popupWV.EXPECT().SetCallbacks(mock.Anything).Once()
250+ popupWV.EXPECT().IsDestroyed().Return(false).Twice()
251+ popupWV.EXPECT().LoadURI(mock.Anything, "https://example.com/second").Return(nil).Once()
252+
253+ factory := mocks.NewMockWebViewFactory(t)
254+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
255+ c := &Coordinator{webViews: make(map[entity.PaneID]port.WebView), popups: newPopupManager()}
256+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, func() string { return "detached-pane" })
257+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
258+ windowByPane := map[entity.PaneID]string{parentPaneID: "owner-window"}
259+ c.SetPopupWindowIDResolver(func(paneID entity.PaneID) (string, bool) {
260+ windowID, ok := windowByPane[paneID]
261+ return windowID, ok
262+ })
263+ hostCalls := 0
264+ c.SetOnOpenBrowserWindow(func(_ context.Context, input BrowserWindowInput) (BrowserWindowResult, error) {
265+ hostCalls++
266+ windowByPane[input.PopupPane.ID] = "detached-window"
267+ c.RegisterPopupWebView(input.PopupPane.ID, input.PopupWebView)
268+ return BrowserWindowResult{WindowID: "detached-window"}, nil
269+ })
270+
271+ first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{
272+ Engine: dto.BrowserEngineCEF, TargetURI: "https://example.com/first", FrameName: "shared",
273+ TargetDisposition: dto.WindowDispositionNewPopup, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
274+ })
275+ second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{
276+ Engine: dto.BrowserEngineCEF, TargetURI: "https://example.com/second", FrameName: "shared",
277+ TargetDisposition: dto.WindowDispositionNewPopup, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
278+ })
279+
280+ assert.Same(t, popupWV, first)
281+ assert.Same(t, popupWV, second)
282+ assert.Equal(t, 1, hostCalls)
283+}
284+
285+func TestPopupFloatingNoopenerDoesNotReuseDetachedNamedContext(t *testing.T) {
286+ ctx := context.Background()
287+ parentPaneID := entity.PaneID("floating")
288+ parentWV := mocks.NewMockWebView(t)
289+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice()
290+ firstWV := mocks.NewMockWebView(t)
291+ firstWV.EXPECT().ID().Return(port.WebViewID(201)).Maybe()
292+ firstWV.EXPECT().Generation().Return(uint64(1)).Maybe()
293+ firstWV.EXPECT().SetCallbacks(mock.Anything).Once()
294+ secondWV := mocks.NewMockWebView(t)
295+ secondWV.EXPECT().ID().Return(port.WebViewID(202)).Maybe()
296+ secondWV.EXPECT().Generation().Return(uint64(1)).Maybe()
297+ secondWV.EXPECT().SetCallbacks(mock.Anything).Once()
298+ factory := mocks.NewMockWebViewFactory(t)
299+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(firstWV, nil).Once()
300+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(secondWV, nil).Once()
301+ c := &Coordinator{popups: newPopupManager()}
302+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, func() string { return "detached-pane" })
303+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
304+ hostCalls := 0
305+ c.SetOnOpenBrowserWindow(func(_ context.Context, _ BrowserWindowInput) (BrowserWindowResult, error) {
306+ hostCalls++
307+ return BrowserWindowResult{WindowID: "detached-window"}, nil
308+ })
309+ req := port.PopupRequest{
310+ Engine: dto.BrowserEngineCEF, FrameName: "shared", NoJavaScriptAccess: true,
311+ TargetDisposition: dto.WindowDispositionNewPopup, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
312+ }
313+ req.TargetURI = "https://example.com/first"
314+ first := c.handlePopupCreate(ctx, parentPaneID, parentWV, req)
315+ req.TargetURI = "https://example.com/second"
316+ second := c.handlePopupCreate(ctx, parentPaneID, parentWV, req)
317+
318+ assert.Same(t, firstWV, first)
319+ assert.Same(t, secondWV, second)
320+ assert.Equal(t, 2, hostCalls)
321+}
322+
323+func TestPopupFloatingBrowserHostFailureDestroysWebViewExactlyOnce(t *testing.T) {
324+ parentWV := mocks.NewMockWebView(t)
325+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once()
326+ popupWV := mocks.NewMockWebView(t)
327+ popupWV.EXPECT().ID().Return(port.WebViewID(201)).Maybe()
328+ popupWV.EXPECT().Generation().Return(uint64(1)).Maybe()
329+ popupWV.EXPECT().SetCallbacks(mock.Anything).Once()
330+ popupWV.EXPECT().Destroy().Once()
331+ factory := mocks.NewMockWebViewFactory(t)
332+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
333+ c := &Coordinator{popups: newPopupManager()}
334+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, func() string { return "detached-pane" })
335+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
336+ c.SetOnOpenBrowserWindow(func(context.Context, BrowserWindowInput) (BrowserWindowResult, error) {
337+ return BrowserWindowResult{}, errors.New("host failed")
338+ })
339+
340+ got := c.handlePopupCreate(context.Background(), "floating", parentWV, port.PopupRequest{
341+ Engine: dto.BrowserEngineCEF, TargetURI: "https://example.com/new", FrameName: "_blank",
342+ TargetDisposition: dto.WindowDispositionNewTab, PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesNone},
343+ })
344+
345+ assert.Nil(t, got)
346+}
347+
233 func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) {348 func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) {
234 ctx := context.Background()349 ctx := context.Background()
235 parentPaneID := entity.PaneID("parent-pane")350 parentPaneID := entity.PaneID("parent-pane")

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close