detached 32ee89e

Harden browsing context reuse and native popup setup

Reason:
Current-tab browsing-context intents could be misclassified, failed native popup setup could leak a webview, and stale named-context lookups could delete freshly re-registered entries.

Scope:
- classify current-tab popup requests as navigation intents instead of unknown triggers
- make named browsing-context cleanup revalidate state before deleting stale entries
- destroy popup webviews when native popup setup fails before a shell is attached
- simplify repeated-shortcut regression coverage while preserving the expanded guardrails

Description:
This follow-up hardens the post-refactor seams that CodeRabbit surfaced. It adds TOCTOU-safe deletion in the named browsing-context registry, fixes current-tab trigger inference, cleans up failed native popup setup deterministically, and extends regression tests around these paths and the shortcut repeat suppression suite.
commit
32ee89e92ad1a19fc9ba0a05614ca5a7f92ba532
author
brice · · 2026-05-13 04:59 UTC
parent
07857df
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
7 files · +204 -78
Unified
UnifiedSplit
M internal/ui/coordinator/content/browsing_context_policy.go +28 -23
@@ -56,34 +56,34 @@ func (browsingContextPolicy) Decide(req port.NewBrowsingContextRequest, namedCon
5656
5757 func buildPopupBrowsingContextRequest(req port.PopupRequest) port.NewBrowsingContextRequest {
5858 return port.NewBrowsingContextRequest{
59- ParentWebViewID: req.ParentViewID,
60- SourceBrowserID: req.SourceBrowserID,
61- SourceFrameID: req.SourceFrameID,
62- SourceFrameURL: req.SourceFrameURL,
63- TargetURI: req.TargetURI,
64- TargetFrameName: req.FrameName,
65- TargetDisposition: inferPopupWindowDisposition(req),
66- IsUserGesture: req.IsUserGesture,
67- NoJavaScriptAccess: req.NoJavaScriptAccess,
68- WindowFeatures: req.WindowFeatures,
69- TriggerKind: inferPopupTriggerKind(req),
70- AuthIntent: IsOAuthURL(req.TargetURI),
71- RequiresNativeOpener: false,
72- RequestContextDisposition: port.RequestContextInheritParent,
59+ ParentWebViewID: req.ParentViewID,
60+ SourceBrowserID: req.SourceBrowserID,
61+ SourceFrameID: req.SourceFrameID,
62+ SourceFrameURL: req.SourceFrameURL,
63+ TargetURI: req.TargetURI,
64+ TargetFrameName: req.FrameName,
65+ TargetDisposition: inferPopupWindowDisposition(req),
66+ IsUserGesture: req.IsUserGesture,
67+ NoJavaScriptAccess: req.NoJavaScriptAccess,
68+ WindowFeatures: req.WindowFeatures,
69+ TriggerKind: inferPopupTriggerKind(req),
70+ AuthIntent: IsOAuthURL(req.TargetURI),
71+ RequiresNativeOpener: false,
72+ RequestContextDisposition: port.RequestContextInheritParent,
7373 }
7474 }
7575
7676 func buildLinkBrowsingContextRequest(parentWebViewID port.WebViewID, uri string) port.NewBrowsingContextRequest {
7777 return port.NewBrowsingContextRequest{
78- ParentWebViewID: parentWebViewID,
79- TargetURI: uri,
80- TargetFrameName: "_blank",
81- TargetDisposition: port.WindowDispositionNewTab,
82- IsUserGesture: true,
83- NoJavaScriptAccess: true,
84- TriggerKind: port.TriggerLinkNewPage,
85- AuthIntent: IsOAuthURL(uri),
86- RequestContextDisposition: port.RequestContextInheritParent,
78+ ParentWebViewID: parentWebViewID,
79+ TargetURI: uri,
80+ TargetFrameName: "_blank",
81+ TargetDisposition: port.WindowDispositionNewTab,
82+ IsUserGesture: true,
83+ NoJavaScriptAccess: true,
84+ TriggerKind: port.TriggerLinkNewPage,
85+ AuthIntent: IsOAuthURL(uri),
86+ RequestContextDisposition: port.RequestContextInheritParent,
8787 }
8888 }
8989
@@ -99,6 +99,11 @@ func inferPopupWindowDisposition(req port.PopupRequest) port.WindowDisposition {
9999
100100 func inferPopupTriggerKind(req port.PopupRequest) port.TriggerKind {
101101 switch inferPopupWindowDisposition(req) {
102+ case port.WindowDispositionCurrentTab:
103+ if reusableBrowsingContextName(req.FrameName) != "" {
104+ return port.TriggerNamedTargetNavigation
105+ }
106+ return port.TriggerLinkNewPage
102107 case port.WindowDispositionNewTab:
103108 if reusableBrowsingContextName(req.FrameName) != "" {
104109 return port.TriggerNamedTargetNavigation
@@ -56,34 +56,34 @@ func (browsingContextPolicy) Decide(req port.NewBrowsingContextRequest, namedCon
56 56
57 func buildPopupBrowsingContextRequest(req port.PopupRequest) port.NewBrowsingContextRequest {57 func buildPopupBrowsingContextRequest(req port.PopupRequest) port.NewBrowsingContextRequest {
58 return port.NewBrowsingContextRequest{58 return port.NewBrowsingContextRequest{
59- ParentWebViewID: req.ParentViewID,59+ ParentWebViewID: req.ParentViewID,
60- SourceBrowserID: req.SourceBrowserID,60+ SourceBrowserID: req.SourceBrowserID,
61- SourceFrameID: req.SourceFrameID,61+ SourceFrameID: req.SourceFrameID,
62- SourceFrameURL: req.SourceFrameURL,62+ SourceFrameURL: req.SourceFrameURL,
63- TargetURI: req.TargetURI,63+ TargetURI: req.TargetURI,
64- TargetFrameName: req.FrameName,64+ TargetFrameName: req.FrameName,
65- TargetDisposition: inferPopupWindowDisposition(req),65+ TargetDisposition: inferPopupWindowDisposition(req),
66- IsUserGesture: req.IsUserGesture,66+ IsUserGesture: req.IsUserGesture,
67- NoJavaScriptAccess: req.NoJavaScriptAccess,67+ NoJavaScriptAccess: req.NoJavaScriptAccess,
68- WindowFeatures: req.WindowFeatures,68+ WindowFeatures: req.WindowFeatures,
69- TriggerKind: inferPopupTriggerKind(req),69+ TriggerKind: inferPopupTriggerKind(req),
70- AuthIntent: IsOAuthURL(req.TargetURI),70+ AuthIntent: IsOAuthURL(req.TargetURI),
71- RequiresNativeOpener: false,71+ RequiresNativeOpener: false,
72- RequestContextDisposition: port.RequestContextInheritParent,72+ RequestContextDisposition: port.RequestContextInheritParent,
73 }73 }
74 }74 }
75 75
76 func buildLinkBrowsingContextRequest(parentWebViewID port.WebViewID, uri string) port.NewBrowsingContextRequest {76 func buildLinkBrowsingContextRequest(parentWebViewID port.WebViewID, uri string) port.NewBrowsingContextRequest {
77 return port.NewBrowsingContextRequest{77 return port.NewBrowsingContextRequest{
78- ParentWebViewID: parentWebViewID,78+ ParentWebViewID: parentWebViewID,
79- TargetURI: uri,79+ TargetURI: uri,
80- TargetFrameName: "_blank",80+ TargetFrameName: "_blank",
81- TargetDisposition: port.WindowDispositionNewTab,81+ TargetDisposition: port.WindowDispositionNewTab,
82- IsUserGesture: true,82+ IsUserGesture: true,
83- NoJavaScriptAccess: true,83+ NoJavaScriptAccess: true,
84- TriggerKind: port.TriggerLinkNewPage,84+ TriggerKind: port.TriggerLinkNewPage,
85- AuthIntent: IsOAuthURL(uri),85+ AuthIntent: IsOAuthURL(uri),
86- RequestContextDisposition: port.RequestContextInheritParent,86+ RequestContextDisposition: port.RequestContextInheritParent,
87 }87 }
88 }88 }
89 89
@@ -99,6 +99,11 @@ func inferPopupWindowDisposition(req port.PopupRequest) port.WindowDisposition {
99 99
100 func inferPopupTriggerKind(req port.PopupRequest) port.TriggerKind {100 func inferPopupTriggerKind(req port.PopupRequest) port.TriggerKind {
101 switch inferPopupWindowDisposition(req) {101 switch inferPopupWindowDisposition(req) {
102+ case port.WindowDispositionCurrentTab:
103+ if reusableBrowsingContextName(req.FrameName) != "" {
104+ return port.TriggerNamedTargetNavigation
105+ }
106+ return port.TriggerLinkNewPage
102 case port.WindowDispositionNewTab:107 case port.WindowDispositionNewTab:
103 if reusableBrowsingContextName(req.FrameName) != "" {108 if reusableBrowsingContextName(req.FrameName) != "" {
104 return port.TriggerNamedTargetNavigation109 return port.TriggerNamedTargetNavigation
M internal/ui/coordinator/content/browsing_context_policy_test.go +35 -0
@@ -108,3 +108,38 @@ func TestInferPopupTriggerKind_DoesNotTreatNoJavaScriptAccessFalseAsNativeSignal
108108
109109 assert.Equal(t, port.TriggerScriptWindowOpen, inferPopupTriggerKind(req))
110110 }
111+
112+func TestInferPopupTriggerKind_ClassifiesCurrentTabAsNavigation(t *testing.T) {
113+ t.Parallel()
114+
115+ tests := []struct {
116+ name string
117+ req port.PopupRequest
118+ want port.TriggerKind
119+ }{
120+ {
121+ name: "unnamed current-tab request stays a navigation",
122+ req: port.PopupRequest{
123+ TargetURI: "https://example.com/docs",
124+ TargetDisposition: port.WindowDispositionCurrentTab,
125+ },
126+ want: port.TriggerLinkNewPage,
127+ },
128+ {
129+ name: "named current-tab request is treated as named target navigation",
130+ req: port.PopupRequest{
131+ TargetURI: "https://example.com/docs",
132+ FrameName: "shared-pane",
133+ TargetDisposition: port.WindowDispositionCurrentTab,
134+ },
135+ want: port.TriggerNamedTargetNavigation,
136+ },
137+ }
138+
139+ for _, tt := range tests {
140+ t.Run(tt.name, func(t *testing.T) {
141+ t.Parallel()
142+ assert.Equal(t, tt.want, inferPopupTriggerKind(tt.req))
143+ })
144+ }
145+}
@@ -108,3 +108,38 @@ func TestInferPopupTriggerKind_DoesNotTreatNoJavaScriptAccessFalseAsNativeSignal
108 108
109 assert.Equal(t, port.TriggerScriptWindowOpen, inferPopupTriggerKind(req))109 assert.Equal(t, port.TriggerScriptWindowOpen, inferPopupTriggerKind(req))
110 }110 }
111+
112+func TestInferPopupTriggerKind_ClassifiesCurrentTabAsNavigation(t *testing.T) {
113+ t.Parallel()
114+
115+ tests := []struct {
116+ name string
117+ req port.PopupRequest
118+ want port.TriggerKind
119+ }{
120+ {
121+ name: "unnamed current-tab request stays a navigation",
122+ req: port.PopupRequest{
123+ TargetURI: "https://example.com/docs",
124+ TargetDisposition: port.WindowDispositionCurrentTab,
125+ },
126+ want: port.TriggerLinkNewPage,
127+ },
128+ {
129+ name: "named current-tab request is treated as named target navigation",
130+ req: port.PopupRequest{
131+ TargetURI: "https://example.com/docs",
132+ FrameName: "shared-pane",
133+ TargetDisposition: port.WindowDispositionCurrentTab,
134+ },
135+ want: port.TriggerNamedTargetNavigation,
136+ },
137+ }
138+
139+ for _, tt := range tests {
140+ t.Run(tt.name, func(t *testing.T) {
141+ t.Parallel()
142+ assert.Equal(t, tt.want, inferPopupTriggerKind(tt.req))
143+ })
144+ }
145+}
M internal/ui/coordinator/content/named_context_registry.go +13 -6
@@ -59,23 +59,30 @@ func (r *namedBrowsingContextRegistry) Lookup(
5959
6060 currentWindowID, ok := resolveWindowID(state.PaneID)
6161 if !ok || currentWindowID != windowID {
62- r.mu.Lock()
63- delete(r.contexts, key)
64- r.mu.Unlock()
62+ r.deleteIfStateMatches(key, state)
6563 return namedBrowsingContextState{}, nil, false
6664 }
6765
6866 wv := lookupWebView(state.PaneID)
6967 if wv == nil || wv.IsDestroyed() || wv.ID() != state.WebViewID {
70- r.mu.Lock()
71- delete(r.contexts, key)
72- r.mu.Unlock()
68+ r.deleteIfStateMatches(key, state)
7369 return namedBrowsingContextState{}, nil, false
7470 }
7571
7672 return state, wv, true
7773 }
7874
75+func (r *namedBrowsingContextRegistry) deleteIfStateMatches(key namedBrowsingContextKey, state namedBrowsingContextState) {
76+ if r == nil {
77+ return
78+ }
79+ r.mu.Lock()
80+ if currentState, ok := r.contexts[key]; ok && currentState == state {
81+ delete(r.contexts, key)
82+ }
83+ r.mu.Unlock()
84+}
85+
7986 func (r *namedBrowsingContextRegistry) UnregisterByPaneID(paneID entity.PaneID) {
8087 if r == nil || paneID == "" {
8188 return
@@ -59,23 +59,30 @@ func (r *namedBrowsingContextRegistry) Lookup(
59 59
60 currentWindowID, ok := resolveWindowID(state.PaneID)60 currentWindowID, ok := resolveWindowID(state.PaneID)
61 if !ok || currentWindowID != windowID {61 if !ok || currentWindowID != windowID {
62- r.mu.Lock()62+ r.deleteIfStateMatches(key, state)
63- delete(r.contexts, key)
64- r.mu.Unlock()
65 return namedBrowsingContextState{}, nil, false63 return namedBrowsingContextState{}, nil, false
66 }64 }
67 65
68 wv := lookupWebView(state.PaneID)66 wv := lookupWebView(state.PaneID)
69 if wv == nil || wv.IsDestroyed() || wv.ID() != state.WebViewID {67 if wv == nil || wv.IsDestroyed() || wv.ID() != state.WebViewID {
70- r.mu.Lock()68+ r.deleteIfStateMatches(key, state)
71- delete(r.contexts, key)
72- r.mu.Unlock()
73 return namedBrowsingContextState{}, nil, false69 return namedBrowsingContextState{}, nil, false
74 }70 }
75 71
76 return state, wv, true72 return state, wv, true
77 }73 }
78 74
75+func (r *namedBrowsingContextRegistry) deleteIfStateMatches(key namedBrowsingContextKey, state namedBrowsingContextState) {
76+ if r == nil {
77+ return
78+ }
79+ r.mu.Lock()
80+ if currentState, ok := r.contexts[key]; ok && currentState == state {
81+ delete(r.contexts, key)
82+ }
83+ r.mu.Unlock()
84+}
85+
79 func (r *namedBrowsingContextRegistry) UnregisterByPaneID(paneID entity.PaneID) {86 func (r *namedBrowsingContextRegistry) UnregisterByPaneID(paneID entity.PaneID) {
80 if r == nil || paneID == "" {87 if r == nil || paneID == "" {
81 return88 return
M internal/ui/coordinator/content/named_context_registry_test.go +87 -0
@@ -78,3 +78,90 @@ func TestNamedBrowsingContextRegistry_DropsStaleWindowOwnershipOnLookup(t *testi
7878 )
7979 assert.False(t, ok)
8080 }
81+
82+func TestNamedBrowsingContextRegistry_LookupDoesNotDeleteReRegisteredEntryAfterWindowMismatch(t *testing.T) {
83+ t.Parallel()
84+
85+ reg := newNamedBrowsingContextRegistry()
86+ oldPaneID := entity.PaneID("pane-old")
87+ newPaneID := entity.PaneID("pane-new")
88+ newWebViewID := port.WebViewID(42)
89+ reg.Register("window-1", "shared-pane", oldPaneID, port.WebViewID(41))
90+
91+ _, _, ok := reg.Lookup(
92+ "window-1",
93+ "shared-pane",
94+ func(entity.PaneID) port.WebView { return nil },
95+ func(gotPaneID entity.PaneID) (string, bool) {
96+ assert.Equal(t, oldPaneID, gotPaneID)
97+ reg.Register("window-1", "shared-pane", newPaneID, newWebViewID)
98+ return "window-2", true
99+ },
100+ )
101+ assert.False(t, ok)
102+
103+ wv := mocks.NewMockWebView(t)
104+ wv.EXPECT().IsDestroyed().Return(false).Once()
105+ wv.EXPECT().ID().Return(newWebViewID).Once()
106+
107+ state, gotWV, ok := reg.Lookup(
108+ "window-1",
109+ "shared-pane",
110+ func(gotPaneID entity.PaneID) port.WebView {
111+ assert.Equal(t, newPaneID, gotPaneID)
112+ return wv
113+ },
114+ func(gotPaneID entity.PaneID) (string, bool) {
115+ assert.Equal(t, newPaneID, gotPaneID)
116+ return "window-1", true
117+ },
118+ )
119+ assert.True(t, ok)
120+ assert.Equal(t, newPaneID, state.PaneID)
121+ assert.Same(t, wv, gotWV)
122+}
123+
124+func TestNamedBrowsingContextRegistry_LookupDoesNotDeleteReRegisteredEntryAfterStaleWebView(t *testing.T) {
125+ t.Parallel()
126+
127+ reg := newNamedBrowsingContextRegistry()
128+ oldPaneID := entity.PaneID("pane-old")
129+ newPaneID := entity.PaneID("pane-new")
130+ newWebViewID := port.WebViewID(42)
131+ reg.Register("window-1", "shared-pane", oldPaneID, port.WebViewID(41))
132+
133+ _, _, ok := reg.Lookup(
134+ "window-1",
135+ "shared-pane",
136+ func(gotPaneID entity.PaneID) port.WebView {
137+ assert.Equal(t, oldPaneID, gotPaneID)
138+ reg.Register("window-1", "shared-pane", newPaneID, newWebViewID)
139+ return nil
140+ },
141+ func(gotPaneID entity.PaneID) (string, bool) {
142+ assert.Equal(t, oldPaneID, gotPaneID)
143+ return "window-1", true
144+ },
145+ )
146+ assert.False(t, ok)
147+
148+ wv := mocks.NewMockWebView(t)
149+ wv.EXPECT().IsDestroyed().Return(false).Once()
150+ wv.EXPECT().ID().Return(newWebViewID).Once()
151+
152+ state, gotWV, ok := reg.Lookup(
153+ "window-1",
154+ "shared-pane",
155+ func(gotPaneID entity.PaneID) port.WebView {
156+ assert.Equal(t, newPaneID, gotPaneID)
157+ return wv
158+ },
159+ func(gotPaneID entity.PaneID) (string, bool) {
160+ assert.Equal(t, newPaneID, gotPaneID)
161+ return "window-1", true
162+ },
163+ )
164+ assert.True(t, ok)
165+ assert.Equal(t, newPaneID, state.PaneID)
166+ assert.Same(t, wv, gotWV)
167+}
@@ -78,3 +78,90 @@ func TestNamedBrowsingContextRegistry_DropsStaleWindowOwnershipOnLookup(t *testi
78 )78 )
79 assert.False(t, ok)79 assert.False(t, ok)
80 }80 }
81+
82+func TestNamedBrowsingContextRegistry_LookupDoesNotDeleteReRegisteredEntryAfterWindowMismatch(t *testing.T) {
83+ t.Parallel()
84+
85+ reg := newNamedBrowsingContextRegistry()
86+ oldPaneID := entity.PaneID("pane-old")
87+ newPaneID := entity.PaneID("pane-new")
88+ newWebViewID := port.WebViewID(42)
89+ reg.Register("window-1", "shared-pane", oldPaneID, port.WebViewID(41))
90+
91+ _, _, ok := reg.Lookup(
92+ "window-1",
93+ "shared-pane",
94+ func(entity.PaneID) port.WebView { return nil },
95+ func(gotPaneID entity.PaneID) (string, bool) {
96+ assert.Equal(t, oldPaneID, gotPaneID)
97+ reg.Register("window-1", "shared-pane", newPaneID, newWebViewID)
98+ return "window-2", true
99+ },
100+ )
101+ assert.False(t, ok)
102+
103+ wv := mocks.NewMockWebView(t)
104+ wv.EXPECT().IsDestroyed().Return(false).Once()
105+ wv.EXPECT().ID().Return(newWebViewID).Once()
106+
107+ state, gotWV, ok := reg.Lookup(
108+ "window-1",
109+ "shared-pane",
110+ func(gotPaneID entity.PaneID) port.WebView {
111+ assert.Equal(t, newPaneID, gotPaneID)
112+ return wv
113+ },
114+ func(gotPaneID entity.PaneID) (string, bool) {
115+ assert.Equal(t, newPaneID, gotPaneID)
116+ return "window-1", true
117+ },
118+ )
119+ assert.True(t, ok)
120+ assert.Equal(t, newPaneID, state.PaneID)
121+ assert.Same(t, wv, gotWV)
122+}
123+
124+func TestNamedBrowsingContextRegistry_LookupDoesNotDeleteReRegisteredEntryAfterStaleWebView(t *testing.T) {
125+ t.Parallel()
126+
127+ reg := newNamedBrowsingContextRegistry()
128+ oldPaneID := entity.PaneID("pane-old")
129+ newPaneID := entity.PaneID("pane-new")
130+ newWebViewID := port.WebViewID(42)
131+ reg.Register("window-1", "shared-pane", oldPaneID, port.WebViewID(41))
132+
133+ _, _, ok := reg.Lookup(
134+ "window-1",
135+ "shared-pane",
136+ func(gotPaneID entity.PaneID) port.WebView {
137+ assert.Equal(t, oldPaneID, gotPaneID)
138+ reg.Register("window-1", "shared-pane", newPaneID, newWebViewID)
139+ return nil
140+ },
141+ func(gotPaneID entity.PaneID) (string, bool) {
142+ assert.Equal(t, oldPaneID, gotPaneID)
143+ return "window-1", true
144+ },
145+ )
146+ assert.False(t, ok)
147+
148+ wv := mocks.NewMockWebView(t)
149+ wv.EXPECT().IsDestroyed().Return(false).Once()
150+ wv.EXPECT().ID().Return(newWebViewID).Once()
151+
152+ state, gotWV, ok := reg.Lookup(
153+ "window-1",
154+ "shared-pane",
155+ func(gotPaneID entity.PaneID) port.WebView {
156+ assert.Equal(t, newPaneID, gotPaneID)
157+ return wv
158+ },
159+ func(gotPaneID entity.PaneID) (string, bool) {
160+ assert.Equal(t, newPaneID, gotPaneID)
161+ return "window-1", true
162+ },
163+ )
164+ assert.True(t, ok)
165+ assert.Equal(t, newPaneID, state.PaneID)
166+ assert.Same(t, wv, gotWV)
167+}
M internal/ui/input/global_shortcuts_test.go +4 -48
@@ -44,18 +44,6 @@ func TestGlobalShortcutHandlerSuppressesRepeatedBrowserNavigationActions(t *test
4444 }
4545 }
4646
47-func TestGlobalShortcutHandlerSuppressesRepeatedPrintPageAction(t *testing.T) {
48- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
49- now := time.Unix(100, 0)
50-
51- if h.suppressRepeatedShortcut(ActionPrintPage, now) {
52- t.Fatal("first print-page dispatch was suppressed")
53- }
54- if !h.suppressRepeatedShortcut(ActionPrintPage, now.Add(time.Millisecond)) {
55- t.Fatal("repeated print-page dispatch was not suppressed")
56- }
57-}
58-
5947 func TestGlobalShortcutHandlerSuppressesRepeatedTabIndexSwitchActions(t *testing.T) {
6048 actions := []Action{
6149 ActionSwitchTabIndex1,
@@ -132,45 +120,13 @@ func TestGlobalShortcutHandlerReturnsInactiveForNilOrDetached(t *testing.T) {
132120 }
133121 }
134122
135-func TestGlobalShortcutHandlerSuppressesRepeatedReloadAction(t *testing.T) {
136- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
137- now := time.Unix(100, 0)
138-
139- if h.suppressRepeatedShortcut(ActionReload, now) {
140- t.Fatal("first reload dispatch was suppressed")
141- }
142- if !h.suppressRepeatedShortcut(ActionReload, now.Add(time.Millisecond)) {
143- t.Fatal("repeated reload dispatch was not suppressed")
144- }
145-}
146-
147-func TestGlobalShortcutHandlerSuppressesRepeatedToggleFullscreenAction(t *testing.T) {
148- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
149- now := time.Unix(100, 0)
150-
151- if h.suppressRepeatedShortcut(ActionToggleFullscreen, now) {
152- t.Fatal("first toggle-fullscreen dispatch was suppressed")
153- }
154- if !h.suppressRepeatedShortcut(ActionToggleFullscreen, now.Add(time.Millisecond)) {
155- t.Fatal("repeated toggle-fullscreen dispatch was not suppressed")
156- }
157-}
158-
159-func TestGlobalShortcutHandlerSuppressesRepeatedToggleHistorySystemViewAction(t *testing.T) {
160- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
161- now := time.Unix(100, 0)
162-
163- if h.suppressRepeatedShortcut(ActionToggleHistorySystemView, now) {
164- t.Fatal("first toggle-history-systemview dispatch was suppressed")
165- }
166- if !h.suppressRepeatedShortcut(ActionToggleHistorySystemView, now.Add(time.Millisecond)) {
167- t.Fatal("repeated toggle-history-systemview dispatch was not suppressed")
168- }
169-}
170-
171123 func TestGlobalShortcutHandlerSuppressesRepeatedAdditionalOneShotUIActions(t *testing.T) {
172124 actions := []Action{
125+ ActionPrintPage,
126+ ActionReload,
173127 ActionHardReload,
128+ ActionToggleFullscreen,
129+ ActionToggleHistorySystemView,
174130 ActionOpenOmnibox,
175131 ActionOpenFind,
176132 ActionOpenDevTools,
@@ -44,18 +44,6 @@ func TestGlobalShortcutHandlerSuppressesRepeatedBrowserNavigationActions(t *test
44 }44 }
45 }45 }
46 46
47-func TestGlobalShortcutHandlerSuppressesRepeatedPrintPageAction(t *testing.T) {
48- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
49- now := time.Unix(100, 0)
50-
51- if h.suppressRepeatedShortcut(ActionPrintPage, now) {
52- t.Fatal("first print-page dispatch was suppressed")
53- }
54- if !h.suppressRepeatedShortcut(ActionPrintPage, now.Add(time.Millisecond)) {
55- t.Fatal("repeated print-page dispatch was not suppressed")
56- }
57-}
58-
59 func TestGlobalShortcutHandlerSuppressesRepeatedTabIndexSwitchActions(t *testing.T) {47 func TestGlobalShortcutHandlerSuppressesRepeatedTabIndexSwitchActions(t *testing.T) {
60 actions := []Action{48 actions := []Action{
61 ActionSwitchTabIndex1,49 ActionSwitchTabIndex1,
@@ -132,45 +120,13 @@ func TestGlobalShortcutHandlerReturnsInactiveForNilOrDetached(t *testing.T) {
132 }120 }
133 }121 }
134 122
135-func TestGlobalShortcutHandlerSuppressesRepeatedReloadAction(t *testing.T) {
136- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
137- now := time.Unix(100, 0)
138-
139- if h.suppressRepeatedShortcut(ActionReload, now) {
140- t.Fatal("first reload dispatch was suppressed")
141- }
142- if !h.suppressRepeatedShortcut(ActionReload, now.Add(time.Millisecond)) {
143- t.Fatal("repeated reload dispatch was not suppressed")
144- }
145-}
146-
147-func TestGlobalShortcutHandlerSuppressesRepeatedToggleFullscreenAction(t *testing.T) {
148- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
149- now := time.Unix(100, 0)
150-
151- if h.suppressRepeatedShortcut(ActionToggleFullscreen, now) {
152- t.Fatal("first toggle-fullscreen dispatch was suppressed")
153- }
154- if !h.suppressRepeatedShortcut(ActionToggleFullscreen, now.Add(time.Millisecond)) {
155- t.Fatal("repeated toggle-fullscreen dispatch was not suppressed")
156- }
157-}
158-
159-func TestGlobalShortcutHandlerSuppressesRepeatedToggleHistorySystemViewAction(t *testing.T) {
160- h := &GlobalShortcutHandler{lastDispatchAt: make(map[Action]time.Time)}
161- now := time.Unix(100, 0)
162-
163- if h.suppressRepeatedShortcut(ActionToggleHistorySystemView, now) {
164- t.Fatal("first toggle-history-systemview dispatch was suppressed")
165- }
166- if !h.suppressRepeatedShortcut(ActionToggleHistorySystemView, now.Add(time.Millisecond)) {
167- t.Fatal("repeated toggle-history-systemview dispatch was not suppressed")
168- }
169-}
170-
171 func TestGlobalShortcutHandlerSuppressesRepeatedAdditionalOneShotUIActions(t *testing.T) {123 func TestGlobalShortcutHandlerSuppressesRepeatedAdditionalOneShotUIActions(t *testing.T) {
172 actions := []Action{124 actions := []Action{
125+ ActionPrintPage,
126+ ActionReload,
173 ActionHardReload,127 ActionHardReload,
128+ ActionToggleFullscreen,
129+ ActionToggleHistorySystemView,
174 ActionOpenOmnibox,130 ActionOpenOmnibox,
175 ActionOpenFind,131 ActionOpenFind,
176 ActionOpenDevTools,132 ActionOpenDevTools,
M internal/ui/native_popup_window.go +18 -1
@@ -22,6 +22,19 @@ type nativePopupWindow struct {
2222 closeOnce sync.Once
2323 }
2424
25+type nativePopupDestroyer interface {
26+ Destroy()
27+}
28+
29+func destroyFailedNativePopupSetup(popupShell nativePopupDestroyer, wv port.WebView) {
30+ if wv != nil && !wv.IsDestroyed() {
31+ wv.Destroy()
32+ }
33+ if popupShell != nil {
34+ popupShell.Destroy()
35+ }
36+}
37+
2538 func (a *App) ensureNativePopupWindows() {
2639 if a.nativePopupWindows == nil {
2740 a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow)
@@ -40,9 +53,13 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
4053 if err != nil {
4154 return err
4255 }
56+ if a.contentCoord == nil {
57+ destroyFailedNativePopupSetup(popupShell, input.PopupWebView)
58+ return fmt.Errorf("content coordinator not available for native popup")
59+ }
4360 widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView)
4461 if widget == nil || widget.GtkWidget() == nil {
45- popupShell.Destroy()
62+ destroyFailedNativePopupSetup(popupShell, input.PopupWebView)
4663 return fmt.Errorf("failed to wrap native popup webview widget")
4764 }
4865 popupShell.SetContent(widget.GtkWidget())
@@ -22,6 +22,19 @@ type nativePopupWindow struct {
22 closeOnce sync.Once22 closeOnce sync.Once
23 }23 }
24 24
25+type nativePopupDestroyer interface {
26+ Destroy()
27+}
28+
29+func destroyFailedNativePopupSetup(popupShell nativePopupDestroyer, wv port.WebView) {
30+ if wv != nil && !wv.IsDestroyed() {
31+ wv.Destroy()
32+ }
33+ if popupShell != nil {
34+ popupShell.Destroy()
35+ }
36+}
37+
25 func (a *App) ensureNativePopupWindows() {38 func (a *App) ensureNativePopupWindows() {
26 if a.nativePopupWindows == nil {39 if a.nativePopupWindows == nil {
27 a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow)40 a.nativePopupWindows = make(map[port.WebViewID]*nativePopupWindow)
@@ -40,9 +53,13 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
40 if err != nil {53 if err != nil {
41 return err54 return err
42 }55 }
56+ if a.contentCoord == nil {
57+ destroyFailedNativePopupSetup(popupShell, input.PopupWebView)
58+ return fmt.Errorf("content coordinator not available for native popup")
59+ }
43 widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView)60 widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView)
44 if widget == nil || widget.GtkWidget() == nil {61 if widget == nil || widget.GtkWidget() == nil {
45- popupShell.Destroy()62+ destroyFailedNativePopupSetup(popupShell, input.PopupWebView)
46 return fmt.Errorf("failed to wrap native popup webview widget")63 return fmt.Errorf("failed to wrap native popup webview widget")
47 }64 }
48 popupShell.SetContent(widget.GtkWidget())65 popupShell.SetContent(widget.GtkWidget())
M internal/ui/native_popup_window_test.go +19 -0
@@ -8,6 +8,25 @@ import (
88 "github.com/stretchr/testify/assert"
99 )
1010
11+type popupDestroySpy struct {
12+ destroyed bool
13+}
14+
15+func (p *popupDestroySpy) Destroy() {
16+ p.destroyed = true
17+}
18+
19+func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) {
20+ wv := portmocks.NewMockWebView(t)
21+ wv.EXPECT().IsDestroyed().Return(false).Once()
22+ wv.EXPECT().Destroy().Once()
23+
24+ shell := &popupDestroySpy{}
25+ destroyFailedNativePopupSetup(shell, wv)
26+
27+ assert.True(t, shell.destroyed)
28+}
29+
1130 func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) {
1231 wv := portmocks.NewMockWebView(t)
1332 wv.EXPECT().IsDestroyed().Return(false).Once()
@@ -8,6 +8,25 @@ import (
8 "github.com/stretchr/testify/assert"8 "github.com/stretchr/testify/assert"
9 )9 )
10 10
11+type popupDestroySpy struct {
12+ destroyed bool
13+}
14+
15+func (p *popupDestroySpy) Destroy() {
16+ p.destroyed = true
17+}
18+
19+func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) {
20+ wv := portmocks.NewMockWebView(t)
21+ wv.EXPECT().IsDestroyed().Return(false).Once()
22+ wv.EXPECT().Destroy().Once()
23+
24+ shell := &popupDestroySpy{}
25+ destroyFailedNativePopupSetup(shell, wv)
26+
27+ assert.True(t, shell.destroyed)
28+}
29+
11 func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) {30 func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) {
12 wv := portmocks.NewMockWebView(t)31 wv := portmocks.NewMockWebView(t)
13 wv.EXPECT().IsDestroyed().Return(false).Once()32 wv.EXPECT().IsDestroyed().Return(false).Once()

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