detached 1dedbbf

fix: address PR review feedback

- Move GLib handler install before any GTK/GLib calls
- Remove debug script injection from popup WebViews
- Change popup create signal logs to Debug level
- Move callback setup after successful workspace insertion
- Truncate OAuth URLs in log messages for privacy
- Fix snapshot dirty flag to preserve pending snapshots
- Cancel OAuth safety timer on normal popup close
commit
1dedbbf26428918f3f9591e30525389f692d8127
author
brice · · 2026-01-23 05:08 UTC
parent
8582952
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +39 -50
Unified
UnifiedSplit
M internal/bootstrap/webkit_stack.go +6 -6
@@ -188,6 +188,12 @@ func configureRenderingEnvironment(
188188 perfSettings *config.ResolvedPerformanceSettings,
189189 logger zerolog.Logger,
190190 ) {
191+ // Install GLib log handler FIRST if configured (must be before any GTK/GLib calls)
192+ if cfg.Logging.CaptureGTKLogs {
193+ enableDebug := cfg.Logging.Level == "debug" || cfg.Logging.Level == "trace"
194+ logging.InstallGLibLogHandler(ctx, logger, enableDebug)
195+ }
196+
191197 renderEnv := env.NewManager()
192198 gpuVendor := renderEnv.DetectGPUVendor(ctx)
193199 renderSettings := port.RenderingEnvSettings{
@@ -223,10 +229,4 @@ func configureRenderingEnvironment(
223229 Str("gpu", string(gpuVendor)).
224230 Interface("vars", renderEnv.GetAppliedVars()).
225231 Msg("rendering environment configured")
226-
227- // Install GLib log handler if configured (captures GTK4/WebKitGTK6 messages)
228- if cfg.Logging.CaptureGTKLogs {
229- enableDebug := cfg.Logging.Level == "debug" || cfg.Logging.Level == "trace"
230- logging.InstallGLibLogHandler(ctx, logger, enableDebug)
231- }
232232 }
@@ -188,6 +188,12 @@ func configureRenderingEnvironment(
188 perfSettings *config.ResolvedPerformanceSettings,188 perfSettings *config.ResolvedPerformanceSettings,
189 logger zerolog.Logger,189 logger zerolog.Logger,
190 ) {190 ) {
191+ // Install GLib log handler FIRST if configured (must be before any GTK/GLib calls)
192+ if cfg.Logging.CaptureGTKLogs {
193+ enableDebug := cfg.Logging.Level == "debug" || cfg.Logging.Level == "trace"
194+ logging.InstallGLibLogHandler(ctx, logger, enableDebug)
195+ }
196+
191 renderEnv := env.NewManager()197 renderEnv := env.NewManager()
192 gpuVendor := renderEnv.DetectGPUVendor(ctx)198 gpuVendor := renderEnv.DetectGPUVendor(ctx)
193 renderSettings := port.RenderingEnvSettings{199 renderSettings := port.RenderingEnvSettings{
@@ -223,10 +229,4 @@ func configureRenderingEnvironment(
223 Str("gpu", string(gpuVendor)).229 Str("gpu", string(gpuVendor)).
224 Interface("vars", renderEnv.GetAppliedVars()).230 Interface("vars", renderEnv.GetAppliedVars()).
225 Msg("rendering environment configured")231 Msg("rendering environment configured")
226-
227- // Install GLib log handler if configured (captures GTK4/WebKitGTK6 messages)
228- if cfg.Logging.CaptureGTKLogs {
229- enableDebug := cfg.Logging.Level == "debug" || cfg.Logging.Level == "trace"
230- logging.InstallGLibLogHandler(ctx, logger, enableDebug)
231- }
232 }232 }
M internal/infrastructure/snapshot/service.go +5 -4
@@ -121,14 +121,15 @@ func (s *Service) SaveNow(ctx context.Context) error {
121121
122122 func (s *Service) saveSnapshot(ctx context.Context) error {
123123 s.mu.Lock()
124- s.dirty = false
125124 ready := s.ready
126- s.mu.Unlock()
127-
128- // Don't save until session is persisted to avoid FK constraint violations
129125 if !ready {
126+ // Don't clear dirty if not ready - keep pending snapshot for later
127+ s.mu.Unlock()
130128 return nil
131129 }
130+ // Only clear dirty when we're actually going to save
131+ s.dirty = false
132+ s.mu.Unlock()
132133
133134 tabList := s.provider.GetTabList()
134135 sessionID := s.provider.GetSessionID()
@@ -121,14 +121,15 @@ func (s *Service) SaveNow(ctx context.Context) error {
121 121
122 func (s *Service) saveSnapshot(ctx context.Context) error {122 func (s *Service) saveSnapshot(ctx context.Context) error {
123 s.mu.Lock()123 s.mu.Lock()
124- s.dirty = false
125 ready := s.ready124 ready := s.ready
126- s.mu.Unlock()
127-
128- // Don't save until session is persisted to avoid FK constraint violations
129 if !ready {125 if !ready {
126+ // Don't clear dirty if not ready - keep pending snapshot for later
127+ s.mu.Unlock()
130 return nil128 return nil
131 }129 }
130+ // Only clear dirty when we're actually going to save
131+ s.dirty = false
132+ s.mu.Unlock()
132 133
133 tabList := s.provider.GetTabList()134 tabList := s.provider.GetTabList()
134 sessionID := s.provider.GetSessionID()135 sessionID := s.provider.GetSessionID()
M internal/infrastructure/webkit/factory.go +0 -19
@@ -6,7 +6,6 @@ import (
66
77 "github.com/bnema/dumber/internal/application/port"
88 "github.com/bnema/dumber/internal/logging"
9- wk "github.com/bnema/puregotk-webkit/webkit"
109 )
1110
1211 // WebViewFactory creates WebView instances for the application.
@@ -115,24 +114,6 @@ func (f *WebViewFactory) CreateRelated(ctx context.Context, parentID port.WebVie
115114 log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to related webview")
116115 }
117116
118- // DEBUG: Inject script to check window.opener status
119- openerDebugScript := wk.NewUserScript(
120- `console.log('[DUMBER DEBUG] window.opener:', window.opener);
121- console.log('[DUMBER DEBUG] window.opener type:', typeof window.opener);
122- if (window.opener) {
123- console.log('[DUMBER DEBUG] window.opener.location:', window.opener.location?.href || 'BLOCKED');
124- console.log('[DUMBER DEBUG] can postMessage:', typeof window.opener.postMessage);
125- }`,
126- wk.UserContentInjectAllFramesValue,
127- wk.UserScriptInjectAtDocumentStartValue,
128- nil,
129- nil,
130- )
131- if openerDebugScript != nil {
132- wv.ucm.AddScript(openerDebugScript)
133- log.Debug().Msg("injected window.opener debug script into popup")
134- }
135-
136117 // Apply content filters if configured
137118 if f.filterApplier != nil {
138119 f.filterApplier.ApplyTo(ctx, wv.ucm)
@@ -6,7 +6,6 @@ import (
6 6
7 "github.com/bnema/dumber/internal/application/port"7 "github.com/bnema/dumber/internal/application/port"
8 "github.com/bnema/dumber/internal/logging"8 "github.com/bnema/dumber/internal/logging"
9- wk "github.com/bnema/puregotk-webkit/webkit"
10 )9 )
11 10
12 // WebViewFactory creates WebView instances for the application.11 // WebViewFactory creates WebView instances for the application.
@@ -115,24 +114,6 @@ func (f *WebViewFactory) CreateRelated(ctx context.Context, parentID port.WebVie
115 log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to related webview")114 log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to related webview")
116 }115 }
117 116
118- // DEBUG: Inject script to check window.opener status
119- openerDebugScript := wk.NewUserScript(
120- `console.log('[DUMBER DEBUG] window.opener:', window.opener);
121- console.log('[DUMBER DEBUG] window.opener type:', typeof window.opener);
122- if (window.opener) {
123- console.log('[DUMBER DEBUG] window.opener.location:', window.opener.location?.href || 'BLOCKED');
124- console.log('[DUMBER DEBUG] can postMessage:', typeof window.opener.postMessage);
125- }`,
126- wk.UserContentInjectAllFramesValue,
127- wk.UserScriptInjectAtDocumentStartValue,
128- nil,
129- nil,
130- )
131- if openerDebugScript != nil {
132- wv.ucm.AddScript(openerDebugScript)
133- log.Debug().Msg("injected window.opener debug script into popup")
134- }
135-
136 // Apply content filters if configured117 // Apply content filters if configured
137 if f.filterApplier != nil {118 if f.filterApplier != nil {
138 f.filterApplier.ApplyTo(ctx, wv.ucm)119 f.filterApplier.ApplyTo(ctx, wv.ucm)
M internal/infrastructure/webkit/webview.go +4 -4
@@ -450,17 +450,17 @@ func (wv *WebView) connectCreateSignal() {
450450 ParentID: wv.id,
451451 }
452452
453- wv.logger.Info().Msg("CREATE SIGNAL: before OnCreate")
453+ wv.logger.Debug().Msg("create signal: invoking OnCreate handler")
454454 newWV := wv.OnCreate(popupReq)
455- wv.logger.Info().Bool("nil", newWV == nil).Msg("CREATE SIGNAL: after OnCreate")
455+ wv.logger.Debug().Bool("nil", newWV == nil).Msg("create signal: OnCreate returned")
456456 if newWV == nil {
457457 return gtk.Widget{} // Block popup
458458 }
459459
460- wv.logger.Info().
460+ wv.logger.Debug().
461461 Uint64("parent_id", uint64(wv.id)).
462462 Uint64("popup_id", uint64(newWV.id)).
463- Msg("CREATE SIGNAL: returning webview widget to WebKit")
463+ Msg("create signal: returning webview widget to WebKit")
464464
465465 return newWV.inner.Widget
466466 }
@@ -450,17 +450,17 @@ func (wv *WebView) connectCreateSignal() {
450 ParentID: wv.id,450 ParentID: wv.id,
451 }451 }
452 452
453- wv.logger.Info().Msg("CREATE SIGNAL: before OnCreate")453+ wv.logger.Debug().Msg("create signal: invoking OnCreate handler")
454 newWV := wv.OnCreate(popupReq)454 newWV := wv.OnCreate(popupReq)
455- wv.logger.Info().Bool("nil", newWV == nil).Msg("CREATE SIGNAL: after OnCreate")455+ wv.logger.Debug().Bool("nil", newWV == nil).Msg("create signal: OnCreate returned")
456 if newWV == nil {456 if newWV == nil {
457 return gtk.Widget{} // Block popup457 return gtk.Widget{} // Block popup
458 }458 }
459 459
460- wv.logger.Info().460+ wv.logger.Debug().
461 Uint64("parent_id", uint64(wv.id)).461 Uint64("parent_id", uint64(wv.id)).
462 Uint64("popup_id", uint64(newWV.id)).462 Uint64("popup_id", uint64(newWV.id)).
463- Msg("CREATE SIGNAL: returning webview widget to WebKit")463+ Msg("create signal: returning webview widget to WebKit")
464 464
465 return newWV.inner.Widget465 return newWV.inner.Widget
466 }466 }
M internal/ui/coordinator/content.go +24 -17
@@ -1261,19 +1261,6 @@ func (c *ContentCoordinator) handlePopupCreate(
12611261 Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).
12621262 Msg("popup OAuth check")
12631263
1264- // Register WebView in our map
1265- c.webViews[paneID] = popupWV
1266-
1267- // Setup standard callbacks
1268- c.setupWebViewCallbacks(ctx, paneID, popupWV)
1269-
1270- // Setup OAuth auto-close if configured
1271- if hasConfig && oauthEnabled && isOAuth {
1272- popupPane.AutoClose = true
1273- c.setupOAuthAutoClose(ctx, paneID, popupWV)
1274- log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup")
1275- }
1276-
12771264 // Insert into workspace IMMEDIATELY (WebView stays hidden)
12781265 // This is required for WebKit to establish window.opener relationship
12791266 if c.onInsertPopup != nil {
@@ -1289,13 +1276,24 @@ func (c *ContentCoordinator) handlePopupCreate(
12891276
12901277 if err := c.onInsertPopup(ctx, popupInput); err != nil {
12911278 log.Error().Err(err).Msg("failed to insert popup into workspace")
1292- // Clean up on failure
1293- delete(c.webViews, paneID)
12941279 popupWV.Destroy()
12951280 return nil
12961281 }
12971282 }
12981283
1284+ // Register WebView in our map (after successful insertion)
1285+ c.webViews[paneID] = popupWV
1286+
1287+ // Setup standard callbacks (after successful insertion to avoid leak)
1288+ c.setupWebViewCallbacks(ctx, paneID, popupWV)
1289+
1290+ // Setup OAuth auto-close if configured
1291+ if hasConfig && oauthEnabled && isOAuth {
1292+ popupPane.AutoClose = true
1293+ c.setupOAuthAutoClose(ctx, paneID, popupWV)
1294+ log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup")
1295+ }
1296+
12991297 // Store pending popup for ready-to-show handling (just visibility now)
13001298 pending := &PendingPopup{
13011299 WebView: popupWV,
@@ -1324,7 +1322,7 @@ func (c *ContentCoordinator) handlePopupCreate(
13241322 Uint64("popup_id", uint64(popupID)).
13251323 Str("pane_id", string(paneID)).
13261324 Str("popup_type", popupType.String()).
1327- Str("target_uri", req.TargetURI).
1325+ Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).
13281326 Msg("popup inserted (hidden), awaiting ready-to-show for visibility")
13291327
13301328 return popupWV
@@ -1361,7 +1359,7 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID
13611359
13621360 log.Info().
13631361 Uint64("popup_id", uint64(popupID)).
1364- Str("target_uri", pending.TargetURI).
1362+ Str("target_uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)).
13651363 Msg("popup now visible")
13661364 }
13671365
@@ -1544,6 +1542,15 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent
15441542 }
15451543 }
15461544 }
1545+
1546+ // Wrap OnClose to cancel safety timer when popup closes normally
1547+ originalOnClose := wv.OnClose
1548+ wv.OnClose = func() {
1549+ cancelSafetyTimer()
1550+ if originalOnClose != nil {
1551+ originalOnClose()
1552+ }
1553+ }
15471554 }
15481555
15491556 // handleLinkMiddleClick handles middle-click / Ctrl+click on links.
@@ -1261,19 +1261,6 @@ func (c *ContentCoordinator) handlePopupCreate(
1261 Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).1261 Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).
1262 Msg("popup OAuth check")1262 Msg("popup OAuth check")
1263 1263
1264- // Register WebView in our map
1265- c.webViews[paneID] = popupWV
1266-
1267- // Setup standard callbacks
1268- c.setupWebViewCallbacks(ctx, paneID, popupWV)
1269-
1270- // Setup OAuth auto-close if configured
1271- if hasConfig && oauthEnabled && isOAuth {
1272- popupPane.AutoClose = true
1273- c.setupOAuthAutoClose(ctx, paneID, popupWV)
1274- log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup")
1275- }
1276-
1277 // Insert into workspace IMMEDIATELY (WebView stays hidden)1264 // Insert into workspace IMMEDIATELY (WebView stays hidden)
1278 // This is required for WebKit to establish window.opener relationship1265 // This is required for WebKit to establish window.opener relationship
1279 if c.onInsertPopup != nil {1266 if c.onInsertPopup != nil {
@@ -1289,13 +1276,24 @@ func (c *ContentCoordinator) handlePopupCreate(
1289 1276
1290 if err := c.onInsertPopup(ctx, popupInput); err != nil {1277 if err := c.onInsertPopup(ctx, popupInput); err != nil {
1291 log.Error().Err(err).Msg("failed to insert popup into workspace")1278 log.Error().Err(err).Msg("failed to insert popup into workspace")
1292- // Clean up on failure
1293- delete(c.webViews, paneID)
1294 popupWV.Destroy()1279 popupWV.Destroy()
1295 return nil1280 return nil
1296 }1281 }
1297 }1282 }
1298 1283
1284+ // Register WebView in our map (after successful insertion)
1285+ c.webViews[paneID] = popupWV
1286+
1287+ // Setup standard callbacks (after successful insertion to avoid leak)
1288+ c.setupWebViewCallbacks(ctx, paneID, popupWV)
1289+
1290+ // Setup OAuth auto-close if configured
1291+ if hasConfig && oauthEnabled && isOAuth {
1292+ popupPane.AutoClose = true
1293+ c.setupOAuthAutoClose(ctx, paneID, popupWV)
1294+ log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup")
1295+ }
1296+
1299 // Store pending popup for ready-to-show handling (just visibility now)1297 // Store pending popup for ready-to-show handling (just visibility now)
1300 pending := &PendingPopup{1298 pending := &PendingPopup{
1301 WebView: popupWV,1299 WebView: popupWV,
@@ -1324,7 +1322,7 @@ func (c *ContentCoordinator) handlePopupCreate(
1324 Uint64("popup_id", uint64(popupID)).1322 Uint64("popup_id", uint64(popupID)).
1325 Str("pane_id", string(paneID)).1323 Str("pane_id", string(paneID)).
1326 Str("popup_type", popupType.String()).1324 Str("popup_type", popupType.String()).
1327- Str("target_uri", req.TargetURI).1325+ Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).
1328 Msg("popup inserted (hidden), awaiting ready-to-show for visibility")1326 Msg("popup inserted (hidden), awaiting ready-to-show for visibility")
1329 1327
1330 return popupWV1328 return popupWV
@@ -1361,7 +1359,7 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID
1361 1359
1362 log.Info().1360 log.Info().
1363 Uint64("popup_id", uint64(popupID)).1361 Uint64("popup_id", uint64(popupID)).
1364- Str("target_uri", pending.TargetURI).1362+ Str("target_uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)).
1365 Msg("popup now visible")1363 Msg("popup now visible")
1366 }1364 }
1367 1365
@@ -1544,6 +1542,15 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent
1544 }1542 }
1545 }1543 }
1546 }1544 }
1545+
1546+ // Wrap OnClose to cancel safety timer when popup closes normally
1547+ originalOnClose := wv.OnClose
1548+ wv.OnClose = func() {
1549+ cancelSafetyTimer()
1550+ if originalOnClose != nil {
1551+ originalOnClose()
1552+ }
1553+ }
1547 }1554 }
1548 1555
1549 // handleLinkMiddleClick handles middle-click / Ctrl+click on links.1556 // handleLinkMiddleClick handles middle-click / Ctrl+click on links.

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