fix: popups not working in split panes, consolidate WebView callback setup
- commit
- 75cd7779406ec7be768f645250386ce6b9aac0a2
- author
- brice · · 2026-01-25 04:46 UTC
- parent
- ad477c0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- tag: v0.26.1
3 files · +15 -61
M CHANGELOG.md +8 -0
| @@ -2,6 +2,14 @@ | |||
| 2 | 2 | ||
| 3 | 3 | All notable changes to this project will be documented in this file. | |
| 4 | 4 | ||
| 5 | + | ## [0.26.1] - 2026-01-25 | |
| 6 | + | ||
| 7 | + | ### Fixed | |
| 8 | + | - **Popups not working in split panes**: Fixed `window.open()` and `target="_blank"` links not functioning in split panes. Popup handling was not being set up for WebViews created via `EnsureWebView`. | |
| 9 | + | ||
| 10 | + | ### Changed | |
| 11 | + | - **WebView callback setup refactoring**: Consolidated duplicate callback setup code into single `setupWebViewCallbacks` function. `EnsureWebView` now delegates to this function instead of inline setup. Made `setupPopupHandling` private since no external callers remain. | |
| 12 | + | ||
| 5 | 13 | ## [0.26.0] - 2026-01-24 | |
| 6 | 14 | ||
| 7 | 15 | ### Added | |
| @@ -2,6 +2,14 @@ | |||||
| 2 | 2 | ||||
| 3 | All notable changes to this project will be documented in this file. | 3 | All notable changes to this project will be documented in this file. | ||
| 4 | 4 | ||||
| 5 | + | ## [0.26.1] - 2026-01-25 | |||
| 6 | + | ||||
| 7 | + | ### Fixed | |||
| 8 | + | - **Popups not working in split panes**: Fixed `window.open()` and `target="_blank"` links not functioning in split panes. Popup handling was not being set up for WebViews created via `EnsureWebView`. | |||
| 9 | + | ||||
| 10 | + | ### Changed | |||
| 11 | + | - **WebView callback setup refactoring**: Consolidated duplicate callback setup code into single `setupWebViewCallbacks` function. `EnsureWebView` now delegates to this function instead of inline setup. Made `setupPopupHandling` private since no external callers remain. | |||
| 12 | + | ||||
| 5 | ## [0.26.0] - 2026-01-24 | 13 | ## [0.26.0] - 2026-01-24 | ||
| 6 | 14 | ||||
| 7 | ### Added | 15 | ### Added | ||
M internal/ui/app.go +0 -5
| @@ -129,7 +129,6 @@ func New(deps *Dependencies) (*App, error) { | |||
| 129 | 129 | if err := deps.Validate(); err != nil { | |
| 130 | 130 | return nil, err | |
| 131 | 131 | } | |
| 132 | - | ||
| 133 | 132 | ctx, cancel := context.WithCancelCause(deps.Ctx) | |
| 134 | 133 | ||
| 135 | 134 | app := &App{ | |
| @@ -1798,11 +1797,7 @@ func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *en | |||
| 1798 | 1797 | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | |
| 1799 | 1798 | } | |
| 1800 | 1799 | } | |
| 1801 | - | ||
| 1802 | - | // Setup popup handling for nested popups | |
| 1803 | - | a.contentCoord.SetupPopupHandling(ctx, pane.ID, wv) | |
| 1804 | 1800 | } | |
| 1805 | - | ||
| 1806 | 1801 | log.Debug(). | |
| 1807 | 1802 | Str("tab_id", string(tabID)). | |
| 1808 | 1803 | Str("pane_id", string(pane.ID)). | |
| @@ -129,7 +129,6 @@ func New(deps *Dependencies) (*App, error) { | |||||
| 129 | if err := deps.Validate(); err != nil { | 129 | if err := deps.Validate(); err != nil { | ||
| 130 | return nil, err | 130 | return nil, err | ||
| 131 | } | 131 | } | ||
| 132 | - | ||||
| 133 | ctx, cancel := context.WithCancelCause(deps.Ctx) | 132 | ctx, cancel := context.WithCancelCause(deps.Ctx) | ||
| 134 | 133 | ||||
| 135 | app := &App{ | 134 | app := &App{ | ||
| @@ -1798,11 +1797,7 @@ func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *en | |||||
| 1798 | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | 1797 | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | ||
| 1799 | } | 1798 | } | ||
| 1800 | } | 1799 | } | ||
| 1801 | - | ||||
| 1802 | - | // Setup popup handling for nested popups | |||
| 1803 | - | a.contentCoord.SetupPopupHandling(ctx, pane.ID, wv) | |||
| 1804 | } | 1800 | } | ||
| 1805 | - | ||||
| 1806 | log.Debug(). | 1801 | log.Debug(). | ||
| 1807 | Str("tab_id", string(tabID)). | 1802 | Str("tab_id", string(tabID)). | ||
| 1808 | Str("pane_id", string(pane.ID)). | 1803 | Str("pane_id", string(pane.ID)). | ||
M internal/ui/coordinator/content.go +7 -56
| @@ -217,59 +217,7 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||
| 217 | 217 | logging.Trace().Mark("webview_acquired") | |
| 218 | 218 | ||
| 219 | 219 | c.webViews[paneID] = wv | |
| 220 | - | ||
| 221 | - | // Set up title change callback | |
| 222 | - | wv.OnTitleChanged = func(title string) { | |
| 223 | - | c.onTitleChanged(ctx, paneID, title) | |
| 224 | - | } | |
| 225 | - | ||
| 226 | - | // Set up favicon change callback | |
| 227 | - | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | |
| 228 | - | c.onFaviconChanged(ctx, paneID, favicon) | |
| 229 | - | } | |
| 230 | - | ||
| 231 | - | // Set up load change callback to re-apply zoom on navigation and handle progress bar | |
| 232 | - | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |
| 233 | - | switch event { | |
| 234 | - | case webkit.LoadStarted: | |
| 235 | - | c.onLoadStarted(paneID) | |
| 236 | - | case webkit.LoadCommitted: | |
| 237 | - | c.onLoadCommitted(ctx, paneID, wv) | |
| 238 | - | case webkit.LoadFinished: | |
| 239 | - | c.onLoadFinished(ctx, paneID, wv) | |
| 240 | - | } | |
| 241 | - | } | |
| 242 | - | ||
| 243 | - | // Set up progress callback for loading indicator | |
| 244 | - | wv.OnProgressChanged = func(progress float64) { | |
| 245 | - | c.onProgressChanged(paneID, progress) | |
| 246 | - | } | |
| 247 | - | ||
| 248 | - | // Set up URI change callback for SPA navigation (History API) | |
| 249 | - | // This fires when URL changes via JavaScript without a full page load | |
| 250 | - | wv.OnURIChanged = func(uri string) { | |
| 251 | - | // Only record if not loading (SPA navigation via History API) | |
| 252 | - | // Full page loads are handled by OnLoadCommitted | |
| 253 | - | if !wv.IsLoading() && uri != "" { | |
| 254 | - | c.onSPANavigation(ctx, paneID, uri) | |
| 255 | - | } | |
| 256 | - | } | |
| 257 | - | ||
| 258 | - | // Set up middle-click / Ctrl+click handler for opening links in new pane | |
| 259 | - | wv.OnLinkMiddleClick = func(uri string) bool { | |
| 260 | - | return c.handleLinkMiddleClick(ctx, paneID, uri) | |
| 261 | - | } | |
| 262 | - | ||
| 263 | - | // Set up link hover callback for status overlay | |
| 264 | - | wv.OnLinkHover = func(uri string) { | |
| 265 | - | c.onLinkHover(paneID, uri) | |
| 266 | - | } | |
| 267 | - | ||
| 268 | - | // Set up fullscreen handlers for idle inhibition | |
| 269 | - | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | |
| 270 | - | ||
| 271 | - | // Set up popup handling for this WebView | |
| 272 | - | c.SetupPopupHandling(ctx, paneID, wv) | |
| 220 | + | c.setupWebViewCallbacks(ctx, paneID, wv) | |
| 273 | 221 | ||
| 274 | 222 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | |
| 275 | 223 | return wv, nil | |
| @@ -1213,9 +1161,9 @@ func (c *ContentCoordinator) SetOnClosePane(fn func(ctx context.Context, paneID | |||
| 1213 | 1161 | c.onClosePane = fn | |
| 1214 | 1162 | } | |
| 1215 | 1163 | ||
| 1216 | - | // SetupPopupHandling wires the popup create signal for a WebView. | |
| 1164 | + | // setupPopupHandling wires the popup create signal for a WebView. | |
| 1217 | 1165 | // This should be called after acquiring a WebView for a pane. | |
| 1218 | - | func (c *ContentCoordinator) SetupPopupHandling(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1166 | + | func (c *ContentCoordinator) setupPopupHandling(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1219 | 1167 | log := logging.FromContext(ctx) | |
| 1220 | 1168 | ||
| 1221 | 1169 | if wv == nil { | |
| @@ -1467,7 +1415,7 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||
| 1467 | 1415 | log.Info().Str("pane_id", string(paneID)).Msg("popup closed") | |
| 1468 | 1416 | } | |
| 1469 | 1417 | ||
| 1470 | - | // setupWebViewCallbacks configures standard callbacks for a WebView. | |
| 1418 | + | // setupWebViewCallbacks configures standard callbacks and popup handling. | |
| 1471 | 1419 | func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1472 | 1420 | // Title changes | |
| 1473 | 1421 | wv.OnTitleChanged = func(title string) { | |
| @@ -1515,6 +1463,9 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1515 | 1463 | ||
| 1516 | 1464 | // Fullscreen handlers for idle inhibition | |
| 1517 | 1465 | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | |
| 1466 | + | ||
| 1467 | + | // Popup handling for nested popups | |
| 1468 | + | c.setupPopupHandling(ctx, paneID, wv) | |
| 1518 | 1469 | } | |
| 1519 | 1470 | ||
| 1520 | 1471 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | |
| @@ -217,59 +217,7 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||||
| 217 | logging.Trace().Mark("webview_acquired") | 217 | logging.Trace().Mark("webview_acquired") | ||
| 218 | 218 | ||||
| 219 | c.webViews[paneID] = wv | 219 | c.webViews[paneID] = wv | ||
| 220 | - | 220 | + | c.setupWebViewCallbacks(ctx, paneID, wv) | |
| 221 | - | // Set up title change callback | |||
| 222 | - | wv.OnTitleChanged = func(title string) { | |||
| 223 | - | c.onTitleChanged(ctx, paneID, title) | |||
| 224 | - | } | |||
| 225 | - | ||||
| 226 | - | // Set up favicon change callback | |||
| 227 | - | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | |||
| 228 | - | c.onFaviconChanged(ctx, paneID, favicon) | |||
| 229 | - | } | |||
| 230 | - | ||||
| 231 | - | // Set up load change callback to re-apply zoom on navigation and handle progress bar | |||
| 232 | - | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |||
| 233 | - | switch event { | |||
| 234 | - | case webkit.LoadStarted: | |||
| 235 | - | c.onLoadStarted(paneID) | |||
| 236 | - | case webkit.LoadCommitted: | |||
| 237 | - | c.onLoadCommitted(ctx, paneID, wv) | |||
| 238 | - | case webkit.LoadFinished: | |||
| 239 | - | c.onLoadFinished(ctx, paneID, wv) | |||
| 240 | - | } | |||
| 241 | - | } | |||
| 242 | - | ||||
| 243 | - | // Set up progress callback for loading indicator | |||
| 244 | - | wv.OnProgressChanged = func(progress float64) { | |||
| 245 | - | c.onProgressChanged(paneID, progress) | |||
| 246 | - | } | |||
| 247 | - | ||||
| 248 | - | // Set up URI change callback for SPA navigation (History API) | |||
| 249 | - | // This fires when URL changes via JavaScript without a full page load | |||
| 250 | - | wv.OnURIChanged = func(uri string) { | |||
| 251 | - | // Only record if not loading (SPA navigation via History API) | |||
| 252 | - | // Full page loads are handled by OnLoadCommitted | |||
| 253 | - | if !wv.IsLoading() && uri != "" { | |||
| 254 | - | c.onSPANavigation(ctx, paneID, uri) | |||
| 255 | - | } | |||
| 256 | - | } | |||
| 257 | - | ||||
| 258 | - | // Set up middle-click / Ctrl+click handler for opening links in new pane | |||
| 259 | - | wv.OnLinkMiddleClick = func(uri string) bool { | |||
| 260 | - | return c.handleLinkMiddleClick(ctx, paneID, uri) | |||
| 261 | - | } | |||
| 262 | - | ||||
| 263 | - | // Set up link hover callback for status overlay | |||
| 264 | - | wv.OnLinkHover = func(uri string) { | |||
| 265 | - | c.onLinkHover(paneID, uri) | |||
| 266 | - | } | |||
| 267 | - | ||||
| 268 | - | // Set up fullscreen handlers for idle inhibition | |||
| 269 | - | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | |||
| 270 | - | ||||
| 271 | - | // Set up popup handling for this WebView | |||
| 272 | - | c.SetupPopupHandling(ctx, paneID, wv) | |||
| 273 | 221 | ||||
| 274 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | 222 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | ||
| 275 | return wv, nil | 223 | return wv, nil | ||
| @@ -1213,9 +1161,9 @@ func (c *ContentCoordinator) SetOnClosePane(fn func(ctx context.Context, paneID | |||||
| 1213 | c.onClosePane = fn | 1161 | c.onClosePane = fn | ||
| 1214 | } | 1162 | } | ||
| 1215 | 1163 | ||||
| 1216 | - | // SetupPopupHandling wires the popup create signal for a WebView. | 1164 | + | // setupPopupHandling wires the popup create signal for a WebView. |
| 1217 | // This should be called after acquiring a WebView for a pane. | 1165 | // This should be called after acquiring a WebView for a pane. | ||
| 1218 | - | func (c *ContentCoordinator) SetupPopupHandling(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 1166 | + | func (c *ContentCoordinator) setupPopupHandling(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { |
| 1219 | log := logging.FromContext(ctx) | 1167 | log := logging.FromContext(ctx) | ||
| 1220 | 1168 | ||||
| 1221 | if wv == nil { | 1169 | if wv == nil { | ||
| @@ -1467,7 +1415,7 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||||
| 1467 | log.Info().Str("pane_id", string(paneID)).Msg("popup closed") | 1415 | log.Info().Str("pane_id", string(paneID)).Msg("popup closed") | ||
| 1468 | } | 1416 | } | ||
| 1469 | 1417 | ||||
| 1470 | - | // setupWebViewCallbacks configures standard callbacks for a WebView. | 1418 | + | // setupWebViewCallbacks configures standard callbacks and popup handling. |
| 1471 | func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 1419 | func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | ||
| 1472 | // Title changes | 1420 | // Title changes | ||
| 1473 | wv.OnTitleChanged = func(title string) { | 1421 | wv.OnTitleChanged = func(title string) { | ||
| @@ -1515,6 +1463,9 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1515 | 1463 | ||||
| 1516 | // Fullscreen handlers for idle inhibition | 1464 | // Fullscreen handlers for idle inhibition | ||
| 1517 | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | 1465 | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | ||
| 1466 | + | ||||
| 1467 | + | // Popup handling for nested popups | |||
| 1468 | + | c.setupPopupHandling(ctx, paneID, wv) | |||
| 1518 | } | 1469 | } | ||
| 1519 | 1470 | ||||
| 1520 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | 1471 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | ||