fix(popup): insert WebView into hierarchy before create signal returns
- commit
- 7d32950b29dcc83abc6c6ceeb7edacd7b63b130c
- author
- brice · · 2026-01-23 04:38 UTC
- parent
- 8e0f4ec
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +112 -80
M internal/infrastructure/webkit/webview.go +15 -5
| @@ -450,16 +450,17 @@ func (wv *WebView) connectCreateSignal() { | |||
| 450 | 450 | ParentID: wv.id, | |
| 451 | 451 | } | |
| 452 | 452 | ||
| 453 | + | wv.logger.Info().Msg("CREATE SIGNAL: before OnCreate") | |
| 453 | 454 | newWV := wv.OnCreate(popupReq) | |
| 455 | + | wv.logger.Info().Bool("nil", newWV == nil).Msg("CREATE SIGNAL: after OnCreate") | |
| 454 | 456 | if newWV == nil { | |
| 455 | 457 | return gtk.Widget{} // Block popup | |
| 456 | 458 | } | |
| 457 | 459 | ||
| 458 | - | // DEBUG: Log the pointer being returned | |
| 459 | - | wv.logger.Debug(). | |
| 460 | - | Uint64("returning_ptr", uint64(newWV.inner.GoPointer())). | |
| 461 | - | Uint64("returning_widget_ptr", uint64(newWV.inner.Widget.GoPointer())). | |
| 462 | - | Msg("create signal returning webview widget") | |
| 460 | + | wv.logger.Info(). | |
| 461 | + | Uint64("parent_id", uint64(wv.id)). | |
| 462 | + | Uint64("popup_id", uint64(newWV.id)). | |
| 463 | + | Msg("CREATE SIGNAL: returning webview widget to WebKit") | |
| 463 | 464 | ||
| 464 | 465 | return newWV.inner.Widget | |
| 465 | 466 | } | |
| @@ -904,6 +905,15 @@ func (wv *WebView) SetBackgroundColor(r, g, b, a float32) { | |||
| 904 | 905 | wv.inner.SetBackgroundColor(rgba) | |
| 905 | 906 | } | |
| 906 | 907 | ||
| 908 | + | // Show makes the WebView widget visible. | |
| 909 | + | // This should be called after the WebView is ready to be displayed. | |
| 910 | + | func (wv *WebView) Show() { | |
| 911 | + | if wv.destroyed.Load() { | |
| 912 | + | return | |
| 913 | + | } | |
| 914 | + | wv.inner.SetVisible(true) | |
| 915 | + | } | |
| 916 | + | ||
| 907 | 917 | // State returns the current WebView state as a snapshot. | |
| 908 | 918 | func (wv *WebView) State() port.WebViewState { | |
| 909 | 919 | return port.WebViewState{ | |
| @@ -450,16 +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 | newWV := wv.OnCreate(popupReq) | 454 | newWV := wv.OnCreate(popupReq) | ||
| 455 | + | wv.logger.Info().Bool("nil", newWV == nil).Msg("CREATE SIGNAL: after OnCreate") | |||
| 454 | if newWV == nil { | 456 | if newWV == nil { | ||
| 455 | return gtk.Widget{} // Block popup | 457 | return gtk.Widget{} // Block popup | ||
| 456 | } | 458 | } | ||
| 457 | 459 | ||||
| 458 | - | // DEBUG: Log the pointer being returned | 460 | + | wv.logger.Info(). |
| 459 | - | wv.logger.Debug(). | 461 | + | Uint64("parent_id", uint64(wv.id)). |
| 460 | - | Uint64("returning_ptr", uint64(newWV.inner.GoPointer())). | 462 | + | Uint64("popup_id", uint64(newWV.id)). |
| 461 | - | Uint64("returning_widget_ptr", uint64(newWV.inner.Widget.GoPointer())). | 463 | + | Msg("CREATE SIGNAL: returning webview widget to WebKit") |
| 462 | - | Msg("create signal returning webview widget") | |||
| 463 | 464 | ||||
| 464 | return newWV.inner.Widget | 465 | return newWV.inner.Widget | ||
| 465 | } | 466 | } | ||
| @@ -904,6 +905,15 @@ func (wv *WebView) SetBackgroundColor(r, g, b, a float32) { | |||||
| 904 | wv.inner.SetBackgroundColor(rgba) | 905 | wv.inner.SetBackgroundColor(rgba) | ||
| 905 | } | 906 | } | ||
| 906 | 907 | ||||
| 908 | + | // Show makes the WebView widget visible. | |||
| 909 | + | // This should be called after the WebView is ready to be displayed. | |||
| 910 | + | func (wv *WebView) Show() { | |||
| 911 | + | if wv.destroyed.Load() { | |||
| 912 | + | return | |||
| 913 | + | } | |||
| 914 | + | wv.inner.SetVisible(true) | |||
| 915 | + | } | |||
| 916 | + | ||||
| 907 | // State returns the current WebView state as a snapshot. | 917 | // State returns the current WebView state as a snapshot. | ||
| 908 | func (wv *WebView) State() port.WebViewState { | 918 | func (wv *WebView) State() port.WebViewState { | ||
| 909 | return port.WebViewState{ | 919 | return port.WebViewState{ | ||
M internal/ui/coordinator/content.go +97 -75
| @@ -1173,8 +1173,34 @@ func (c *ContentCoordinator) SetupPopupHandling(ctx context.Context, paneID enti | |||
| 1173 | 1173 | log.Debug().Str("pane_id", string(paneID)).Msg("popup handling configured for webview") | |
| 1174 | 1174 | } | |
| 1175 | 1175 | ||
| 1176 | + | // createPopupPane creates a new pane entity for a popup window. | |
| 1177 | + | func (c *ContentCoordinator) createPopupPane( | |
| 1178 | + | popupID port.WebViewID, | |
| 1179 | + | parentPaneID entity.PaneID, | |
| 1180 | + | targetURI string, | |
| 1181 | + | ) (entity.PaneID, *entity.Pane) { | |
| 1182 | + | var paneID entity.PaneID | |
| 1183 | + | if c.generateID != nil { | |
| 1184 | + | paneID = entity.PaneID(c.generateID()) | |
| 1185 | + | } else { | |
| 1186 | + | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |
| 1187 | + | } | |
| 1188 | + | ||
| 1189 | + | popupPane := entity.NewPane(paneID) | |
| 1190 | + | popupPane.WindowType = entity.WindowPopup | |
| 1191 | + | popupPane.IsRelated = true | |
| 1192 | + | popupPane.ParentPaneID = &parentPaneID | |
| 1193 | + | popupPane.URI = targetURI | |
| 1194 | + | ||
| 1195 | + | return paneID, popupPane | |
| 1196 | + | } | |
| 1197 | + | ||
| 1176 | 1198 | // handlePopupCreate handles the WebKit "create" signal for popup windows. | |
| 1177 | 1199 | // Returns a new related WebView if popup is allowed, nil to block. | |
| 1200 | + | // | |
| 1201 | + | // IMPORTANT: The WebView MUST be added to a GtkWindow hierarchy BEFORE this | |
| 1202 | + | // signal handler returns. Otherwise WebKit won't establish window.opener. | |
| 1203 | + | // The WebView stays hidden until ready-to-show signal is received. | |
| 1178 | 1204 | func (c *ContentCoordinator) handlePopupCreate( | |
| 1179 | 1205 | ctx context.Context, | |
| 1180 | 1206 | parentPaneID entity.PaneID, | |
| @@ -1202,7 +1228,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1202 | 1228 | return nil | |
| 1203 | 1229 | } | |
| 1204 | 1230 | ||
| 1205 | - | // Create related WebView for session sharing | |
| 1231 | + | // Create related WebView for session sharing (created hidden) | |
| 1206 | 1232 | parentID := parentWV.ID() | |
| 1207 | 1233 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | |
| 1208 | 1234 | if err != nil { | |
| @@ -1212,9 +1238,65 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1212 | 1238 | ||
| 1213 | 1239 | // Detect popup type | |
| 1214 | 1240 | popupType := DetectPopupType(req.FrameName) | |
| 1215 | - | ||
| 1216 | - | // Store pending popup for ready-to-show handling | |
| 1217 | 1241 | popupID := popupWV.ID() | |
| 1242 | + | ||
| 1243 | + | // Determine behavior from config | |
| 1244 | + | behavior := GetBehavior(popupType, c.popupConfig) | |
| 1245 | + | placement := "right" | |
| 1246 | + | if c.popupConfig != nil { | |
| 1247 | + | placement = c.popupConfig.Placement | |
| 1248 | + | } | |
| 1249 | + | ||
| 1250 | + | // Create popup pane entity | |
| 1251 | + | paneID, popupPane := c.createPopupPane(popupID, parentPaneID, req.TargetURI) | |
| 1252 | + | ||
| 1253 | + | // Check OAuth configuration | |
| 1254 | + | hasConfig := c.popupConfig != nil | |
| 1255 | + | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |
| 1256 | + | isOAuth := IsOAuthURL(req.TargetURI) | |
| 1257 | + | log.Debug(). | |
| 1258 | + | Bool("has_config", hasConfig). | |
| 1259 | + | Bool("oauth_enabled", oauthEnabled). | |
| 1260 | + | Bool("is_oauth", isOAuth). | |
| 1261 | + | Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |
| 1262 | + | Msg("popup OAuth check") | |
| 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) | |
| 1278 | + | // This is required for WebKit to establish window.opener relationship | |
| 1279 | + | if c.onInsertPopup != nil { | |
| 1280 | + | popupInput := InsertPopupInput{ | |
| 1281 | + | ParentPaneID: parentPaneID, | |
| 1282 | + | PopupPane: popupPane, | |
| 1283 | + | WebView: popupWV, | |
| 1284 | + | Behavior: behavior, | |
| 1285 | + | Placement: placement, | |
| 1286 | + | PopupType: popupType, | |
| 1287 | + | TargetURI: req.TargetURI, | |
| 1288 | + | } | |
| 1289 | + | ||
| 1290 | + | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |
| 1291 | + | log.Error().Err(err).Msg("failed to insert popup into workspace") | |
| 1292 | + | // Clean up on failure | |
| 1293 | + | delete(c.webViews, paneID) | |
| 1294 | + | popupWV.Destroy() | |
| 1295 | + | return nil | |
| 1296 | + | } | |
| 1297 | + | } | |
| 1298 | + | ||
| 1299 | + | // Store pending popup for ready-to-show handling (just visibility now) | |
| 1218 | 1300 | pending := &PendingPopup{ | |
| 1219 | 1301 | WebView: popupWV, | |
| 1220 | 1302 | ParentPaneID: parentPaneID, | |
| @@ -1230,7 +1312,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1230 | 1312 | c.pendingPopups[popupID] = pending | |
| 1231 | 1313 | c.popupMu.Unlock() | |
| 1232 | 1314 | ||
| 1233 | - | // Wire ready-to-show and close signals | |
| 1315 | + | // Wire ready-to-show (now just for visibility) and close signals | |
| 1234 | 1316 | popupWV.OnReadyToShow = func() { | |
| 1235 | 1317 | c.handlePopupReadyToShow(ctx, popupID) | |
| 1236 | 1318 | } | |
| @@ -1240,15 +1322,17 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1240 | 1322 | ||
| 1241 | 1323 | log.Info(). | |
| 1242 | 1324 | Uint64("popup_id", uint64(popupID)). | |
| 1325 | + | Str("pane_id", string(paneID)). | |
| 1243 | 1326 | Str("popup_type", popupType.String()). | |
| 1244 | 1327 | Str("target_uri", req.TargetURI). | |
| 1245 | - | Msg("popup created, awaiting ready-to-show") | |
| 1328 | + | Msg("popup inserted (hidden), awaiting ready-to-show for visibility") | |
| 1246 | 1329 | ||
| 1247 | 1330 | return popupWV | |
| 1248 | 1331 | } | |
| 1249 | 1332 | ||
| 1250 | 1333 | // handlePopupReadyToShow handles the WebKit "ready-to-show" signal. | |
| 1251 | - | // This is called when the popup is configured and ready to be displayed. | |
| 1334 | + | // The popup WebView was already inserted into the workspace (hidden) during | |
| 1335 | + | // the create signal. This handler just makes it visible. | |
| 1252 | 1336 | func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID port.WebViewID) { | |
| 1253 | 1337 | log := logging.FromContext(ctx) | |
| 1254 | 1338 | ||
| @@ -1268,79 +1352,17 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID | |||
| 1268 | 1352 | log.Debug(). | |
| 1269 | 1353 | Uint64("popup_id", uint64(popupID)). | |
| 1270 | 1354 | Str("popup_type", pending.PopupType.String()). | |
| 1271 | - | Msg("popup ready to show") | |
| 1272 | - | ||
| 1273 | - | // Determine behavior from config | |
| 1274 | - | behavior := GetBehavior(pending.PopupType, c.popupConfig) | |
| 1275 | - | placement := "right" | |
| 1276 | - | if c.popupConfig != nil { | |
| 1277 | - | placement = c.popupConfig.Placement | |
| 1278 | - | } | |
| 1279 | - | ||
| 1280 | - | // Create popup pane entity | |
| 1281 | - | var paneID entity.PaneID | |
| 1282 | - | if c.generateID != nil { | |
| 1283 | - | paneID = entity.PaneID(c.generateID()) | |
| 1284 | - | } else { | |
| 1285 | - | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |
| 1286 | - | } | |
| 1287 | - | ||
| 1288 | - | popupPane := entity.NewPane(paneID) | |
| 1289 | - | popupPane.WindowType = entity.WindowPopup | |
| 1290 | - | popupPane.IsRelated = true | |
| 1291 | - | popupPane.ParentPaneID = &pending.ParentPaneID | |
| 1292 | - | popupPane.URI = pending.TargetURI | |
| 1293 | - | ||
| 1294 | - | // Enable OAuth auto-close if configured | |
| 1295 | - | hasConfig := c.popupConfig != nil | |
| 1296 | - | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |
| 1297 | - | isOAuth := IsOAuthURL(pending.TargetURI) | |
| 1298 | - | log.Debug(). | |
| 1299 | - | Bool("has_config", hasConfig). | |
| 1300 | - | Bool("oauth_enabled", oauthEnabled). | |
| 1301 | - | Bool("is_oauth", isOAuth). | |
| 1302 | - | Str("uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)). | |
| 1303 | - | Msg("popup OAuth check") | |
| 1304 | - | ||
| 1305 | - | // Register WebView in our map | |
| 1306 | - | c.webViews[paneID] = pending.WebView | |
| 1355 | + | Msg("popup ready to show - making visible") | |
| 1307 | 1356 | ||
| 1308 | - | // Setup standard callbacks first | |
| 1309 | - | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | |
| 1310 | - | ||
| 1311 | - | // Setup OAuth auto-close AFTER standard callbacks so we can wrap them | |
| 1312 | - | if hasConfig && oauthEnabled && isOAuth { | |
| 1313 | - | popupPane.AutoClose = true | |
| 1314 | - | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |
| 1315 | - | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |
| 1316 | - | } | |
| 1317 | - | ||
| 1318 | - | // Request insertion into workspace | |
| 1319 | - | if c.onInsertPopup != nil { | |
| 1320 | - | popupInput := InsertPopupInput{ | |
| 1321 | - | ParentPaneID: pending.ParentPaneID, | |
| 1322 | - | PopupPane: popupPane, | |
| 1323 | - | WebView: pending.WebView, | |
| 1324 | - | Behavior: behavior, | |
| 1325 | - | Placement: placement, | |
| 1326 | - | PopupType: pending.PopupType, | |
| 1327 | - | TargetURI: pending.TargetURI, | |
| 1328 | - | } | |
| 1329 | - | ||
| 1330 | - | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |
| 1331 | - | log.Error().Err(err).Msg("failed to insert popup into workspace") | |
| 1332 | - | // Clean up on failure | |
| 1333 | - | delete(c.webViews, paneID) | |
| 1334 | - | pending.WebView.Destroy() | |
| 1335 | - | return | |
| 1336 | - | } | |
| 1357 | + | // Make the WebView visible now that it's ready | |
| 1358 | + | if pending.WebView != nil { | |
| 1359 | + | pending.WebView.Show() | |
| 1337 | 1360 | } | |
| 1338 | 1361 | ||
| 1339 | 1362 | log.Info(). | |
| 1340 | - | Str("pane_id", string(paneID)). | |
| 1341 | - | Str("behavior", string(behavior)). | |
| 1342 | - | Str("placement", placement). | |
| 1343 | - | Msg("popup inserted into workspace") | |
| 1363 | + | Uint64("popup_id", uint64(popupID)). | |
| 1364 | + | Str("target_uri", pending.TargetURI). | |
| 1365 | + | Msg("popup now visible") | |
| 1344 | 1366 | } | |
| 1345 | 1367 | ||
| 1346 | 1368 | // handlePopupClose handles the WebKit "close" signal for popup windows. | |
| @@ -1173,8 +1173,34 @@ func (c *ContentCoordinator) SetupPopupHandling(ctx context.Context, paneID enti | |||||
| 1173 | log.Debug().Str("pane_id", string(paneID)).Msg("popup handling configured for webview") | 1173 | log.Debug().Str("pane_id", string(paneID)).Msg("popup handling configured for webview") | ||
| 1174 | } | 1174 | } | ||
| 1175 | 1175 | ||||
| 1176 | + | // createPopupPane creates a new pane entity for a popup window. | |||
| 1177 | + | func (c *ContentCoordinator) createPopupPane( | |||
| 1178 | + | popupID port.WebViewID, | |||
| 1179 | + | parentPaneID entity.PaneID, | |||
| 1180 | + | targetURI string, | |||
| 1181 | + | ) (entity.PaneID, *entity.Pane) { | |||
| 1182 | + | var paneID entity.PaneID | |||
| 1183 | + | if c.generateID != nil { | |||
| 1184 | + | paneID = entity.PaneID(c.generateID()) | |||
| 1185 | + | } else { | |||
| 1186 | + | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |||
| 1187 | + | } | |||
| 1188 | + | ||||
| 1189 | + | popupPane := entity.NewPane(paneID) | |||
| 1190 | + | popupPane.WindowType = entity.WindowPopup | |||
| 1191 | + | popupPane.IsRelated = true | |||
| 1192 | + | popupPane.ParentPaneID = &parentPaneID | |||
| 1193 | + | popupPane.URI = targetURI | |||
| 1194 | + | ||||
| 1195 | + | return paneID, popupPane | |||
| 1196 | + | } | |||
| 1197 | + | ||||
| 1176 | // handlePopupCreate handles the WebKit "create" signal for popup windows. | 1198 | // handlePopupCreate handles the WebKit "create" signal for popup windows. | ||
| 1177 | // Returns a new related WebView if popup is allowed, nil to block. | 1199 | // Returns a new related WebView if popup is allowed, nil to block. | ||
| 1200 | + | // | |||
| 1201 | + | // IMPORTANT: The WebView MUST be added to a GtkWindow hierarchy BEFORE this | |||
| 1202 | + | // signal handler returns. Otherwise WebKit won't establish window.opener. | |||
| 1203 | + | // The WebView stays hidden until ready-to-show signal is received. | |||
| 1178 | func (c *ContentCoordinator) handlePopupCreate( | 1204 | func (c *ContentCoordinator) handlePopupCreate( | ||
| 1179 | ctx context.Context, | 1205 | ctx context.Context, | ||
| 1180 | parentPaneID entity.PaneID, | 1206 | parentPaneID entity.PaneID, | ||
| @@ -1202,7 +1228,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1202 | return nil | 1228 | return nil | ||
| 1203 | } | 1229 | } | ||
| 1204 | 1230 | ||||
| 1205 | - | // Create related WebView for session sharing | 1231 | + | // Create related WebView for session sharing (created hidden) |
| 1206 | parentID := parentWV.ID() | 1232 | parentID := parentWV.ID() | ||
| 1207 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | 1233 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | ||
| 1208 | if err != nil { | 1234 | if err != nil { | ||
| @@ -1212,9 +1238,65 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1212 | 1238 | ||||
| 1213 | // Detect popup type | 1239 | // Detect popup type | ||
| 1214 | popupType := DetectPopupType(req.FrameName) | 1240 | popupType := DetectPopupType(req.FrameName) | ||
| 1215 | - | ||||
| 1216 | - | // Store pending popup for ready-to-show handling | |||
| 1217 | popupID := popupWV.ID() | 1241 | popupID := popupWV.ID() | ||
| 1242 | + | ||||
| 1243 | + | // Determine behavior from config | |||
| 1244 | + | behavior := GetBehavior(popupType, c.popupConfig) | |||
| 1245 | + | placement := "right" | |||
| 1246 | + | if c.popupConfig != nil { | |||
| 1247 | + | placement = c.popupConfig.Placement | |||
| 1248 | + | } | |||
| 1249 | + | ||||
| 1250 | + | // Create popup pane entity | |||
| 1251 | + | paneID, popupPane := c.createPopupPane(popupID, parentPaneID, req.TargetURI) | |||
| 1252 | + | ||||
| 1253 | + | // Check OAuth configuration | |||
| 1254 | + | hasConfig := c.popupConfig != nil | |||
| 1255 | + | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |||
| 1256 | + | isOAuth := IsOAuthURL(req.TargetURI) | |||
| 1257 | + | log.Debug(). | |||
| 1258 | + | Bool("has_config", hasConfig). | |||
| 1259 | + | Bool("oauth_enabled", oauthEnabled). | |||
| 1260 | + | Bool("is_oauth", isOAuth). | |||
| 1261 | + | Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |||
| 1262 | + | Msg("popup OAuth check") | |||
| 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) | |||
| 1278 | + | // This is required for WebKit to establish window.opener relationship | |||
| 1279 | + | if c.onInsertPopup != nil { | |||
| 1280 | + | popupInput := InsertPopupInput{ | |||
| 1281 | + | ParentPaneID: parentPaneID, | |||
| 1282 | + | PopupPane: popupPane, | |||
| 1283 | + | WebView: popupWV, | |||
| 1284 | + | Behavior: behavior, | |||
| 1285 | + | Placement: placement, | |||
| 1286 | + | PopupType: popupType, | |||
| 1287 | + | TargetURI: req.TargetURI, | |||
| 1288 | + | } | |||
| 1289 | + | ||||
| 1290 | + | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |||
| 1291 | + | log.Error().Err(err).Msg("failed to insert popup into workspace") | |||
| 1292 | + | // Clean up on failure | |||
| 1293 | + | delete(c.webViews, paneID) | |||
| 1294 | + | popupWV.Destroy() | |||
| 1295 | + | return nil | |||
| 1296 | + | } | |||
| 1297 | + | } | |||
| 1298 | + | ||||
| 1299 | + | // Store pending popup for ready-to-show handling (just visibility now) | |||
| 1218 | pending := &PendingPopup{ | 1300 | pending := &PendingPopup{ | ||
| 1219 | WebView: popupWV, | 1301 | WebView: popupWV, | ||
| 1220 | ParentPaneID: parentPaneID, | 1302 | ParentPaneID: parentPaneID, | ||
| @@ -1230,7 +1312,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1230 | c.pendingPopups[popupID] = pending | 1312 | c.pendingPopups[popupID] = pending | ||
| 1231 | c.popupMu.Unlock() | 1313 | c.popupMu.Unlock() | ||
| 1232 | 1314 | ||||
| 1233 | - | // Wire ready-to-show and close signals | 1315 | + | // Wire ready-to-show (now just for visibility) and close signals |
| 1234 | popupWV.OnReadyToShow = func() { | 1316 | popupWV.OnReadyToShow = func() { | ||
| 1235 | c.handlePopupReadyToShow(ctx, popupID) | 1317 | c.handlePopupReadyToShow(ctx, popupID) | ||
| 1236 | } | 1318 | } | ||
| @@ -1240,15 +1322,17 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1240 | 1322 | ||||
| 1241 | log.Info(). | 1323 | log.Info(). | ||
| 1242 | Uint64("popup_id", uint64(popupID)). | 1324 | Uint64("popup_id", uint64(popupID)). | ||
| 1325 | + | Str("pane_id", string(paneID)). | |||
| 1243 | Str("popup_type", popupType.String()). | 1326 | Str("popup_type", popupType.String()). | ||
| 1244 | Str("target_uri", req.TargetURI). | 1327 | Str("target_uri", req.TargetURI). | ||
| 1245 | - | Msg("popup created, awaiting ready-to-show") | 1328 | + | Msg("popup inserted (hidden), awaiting ready-to-show for visibility") |
| 1246 | 1329 | ||||
| 1247 | return popupWV | 1330 | return popupWV | ||
| 1248 | } | 1331 | } | ||
| 1249 | 1332 | ||||
| 1250 | // handlePopupReadyToShow handles the WebKit "ready-to-show" signal. | 1333 | // handlePopupReadyToShow handles the WebKit "ready-to-show" signal. | ||
| 1251 | - | // This is called when the popup is configured and ready to be displayed. | 1334 | + | // The popup WebView was already inserted into the workspace (hidden) during |
| 1335 | + | // the create signal. This handler just makes it visible. | |||
| 1252 | func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID port.WebViewID) { | 1336 | func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID port.WebViewID) { | ||
| 1253 | log := logging.FromContext(ctx) | 1337 | log := logging.FromContext(ctx) | ||
| 1254 | 1338 | ||||
| @@ -1268,79 +1352,17 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID | |||||
| 1268 | log.Debug(). | 1352 | log.Debug(). | ||
| 1269 | Uint64("popup_id", uint64(popupID)). | 1353 | Uint64("popup_id", uint64(popupID)). | ||
| 1270 | Str("popup_type", pending.PopupType.String()). | 1354 | Str("popup_type", pending.PopupType.String()). | ||
| 1271 | - | Msg("popup ready to show") | 1355 | + | Msg("popup ready to show - making visible") |
| 1272 | - | ||||
| 1273 | - | // Determine behavior from config | |||
| 1274 | - | behavior := GetBehavior(pending.PopupType, c.popupConfig) | |||
| 1275 | - | placement := "right" | |||
| 1276 | - | if c.popupConfig != nil { | |||
| 1277 | - | placement = c.popupConfig.Placement | |||
| 1278 | - | } | |||
| 1279 | - | ||||
| 1280 | - | // Create popup pane entity | |||
| 1281 | - | var paneID entity.PaneID | |||
| 1282 | - | if c.generateID != nil { | |||
| 1283 | - | paneID = entity.PaneID(c.generateID()) | |||
| 1284 | - | } else { | |||
| 1285 | - | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |||
| 1286 | - | } | |||
| 1287 | - | ||||
| 1288 | - | popupPane := entity.NewPane(paneID) | |||
| 1289 | - | popupPane.WindowType = entity.WindowPopup | |||
| 1290 | - | popupPane.IsRelated = true | |||
| 1291 | - | popupPane.ParentPaneID = &pending.ParentPaneID | |||
| 1292 | - | popupPane.URI = pending.TargetURI | |||
| 1293 | - | ||||
| 1294 | - | // Enable OAuth auto-close if configured | |||
| 1295 | - | hasConfig := c.popupConfig != nil | |||
| 1296 | - | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |||
| 1297 | - | isOAuth := IsOAuthURL(pending.TargetURI) | |||
| 1298 | - | log.Debug(). | |||
| 1299 | - | Bool("has_config", hasConfig). | |||
| 1300 | - | Bool("oauth_enabled", oauthEnabled). | |||
| 1301 | - | Bool("is_oauth", isOAuth). | |||
| 1302 | - | Str("uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)). | |||
| 1303 | - | Msg("popup OAuth check") | |||
| 1304 | - | ||||
| 1305 | - | // Register WebView in our map | |||
| 1306 | - | c.webViews[paneID] = pending.WebView | |||
| 1307 | 1356 | ||||
| 1308 | - | // Setup standard callbacks first | 1357 | + | // Make the WebView visible now that it's ready |
| 1309 | - | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | 1358 | + | if pending.WebView != nil { |
| 1310 | - | 1359 | + | pending.WebView.Show() | |
| 1311 | - | // Setup OAuth auto-close AFTER standard callbacks so we can wrap them | |||
| 1312 | - | if hasConfig && oauthEnabled && isOAuth { | |||
| 1313 | - | popupPane.AutoClose = true | |||
| 1314 | - | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |||
| 1315 | - | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |||
| 1316 | - | } | |||
| 1317 | - | ||||
| 1318 | - | // Request insertion into workspace | |||
| 1319 | - | if c.onInsertPopup != nil { | |||
| 1320 | - | popupInput := InsertPopupInput{ | |||
| 1321 | - | ParentPaneID: pending.ParentPaneID, | |||
| 1322 | - | PopupPane: popupPane, | |||
| 1323 | - | WebView: pending.WebView, | |||
| 1324 | - | Behavior: behavior, | |||
| 1325 | - | Placement: placement, | |||
| 1326 | - | PopupType: pending.PopupType, | |||
| 1327 | - | TargetURI: pending.TargetURI, | |||
| 1328 | - | } | |||
| 1329 | - | ||||
| 1330 | - | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |||
| 1331 | - | log.Error().Err(err).Msg("failed to insert popup into workspace") | |||
| 1332 | - | // Clean up on failure | |||
| 1333 | - | delete(c.webViews, paneID) | |||
| 1334 | - | pending.WebView.Destroy() | |||
| 1335 | - | return | |||
| 1336 | - | } | |||
| 1337 | } | 1360 | } | ||
| 1338 | 1361 | ||||
| 1339 | log.Info(). | 1362 | log.Info(). | ||
| 1340 | - | Str("pane_id", string(paneID)). | 1363 | + | Uint64("popup_id", uint64(popupID)). |
| 1341 | - | Str("behavior", string(behavior)). | 1364 | + | Str("target_uri", pending.TargetURI). |
| 1342 | - | Str("placement", placement). | 1365 | + | Msg("popup now visible") |
| 1343 | - | Msg("popup inserted into workspace") | |||
| 1344 | } | 1366 | } | ||
| 1345 | 1367 | ||||
| 1346 | // handlePopupClose handles the WebKit "close" signal for popup windows. | 1368 | // handlePopupClose handles the WebKit "close" signal for popup windows. | ||