feat(workspace): enhance popup handling with proper widget lifecycle
- commit
- 8bcc8854370f872b63842337af9efd3535fa574a
- author
- bamen · · 2025-09-20 05:21 UTC
- parent
- d9972d9
- signature
- unsigned
3 files · +249 -59
M internal/app/browser/pane.go +4 -2
| @@ -87,11 +87,13 @@ func (p *BrowserPane) Cleanup() { | |||
| 87 | 87 | ||
| 88 | 88 | // Notify JS to cleanup GUI | |
| 89 | 89 | if p.webView != nil && p.hasGUI { | |
| 90 | - | p.webView.InjectScript(` | |
| 90 | + | if err := p.webView.InjectScript(` | |
| 91 | 91 | if (window.__dumber_gui_manager) { | |
| 92 | 92 | window.__dumber_gui_manager.destroy(); | |
| 93 | 93 | } | |
| 94 | - | `) | |
| 94 | + | `); err != nil { | |
| 95 | + | log.Printf("[pane-%s] failed to inject cleanup script: %v", p.id, err) | |
| 96 | + | } | |
| 95 | 97 | } | |
| 96 | 98 | ||
| 97 | 99 | // Clear tracking | |
| @@ -87,11 +87,13 @@ func (p *BrowserPane) Cleanup() { | |||||
| 87 | 87 | ||||
| 88 | // Notify JS to cleanup GUI | 88 | // Notify JS to cleanup GUI | ||
| 89 | if p.webView != nil && p.hasGUI { | 89 | if p.webView != nil && p.hasGUI { | ||
| 90 | - | p.webView.InjectScript(` | 90 | + | if err := p.webView.InjectScript(` |
| 91 | if (window.__dumber_gui_manager) { | 91 | if (window.__dumber_gui_manager) { | ||
| 92 | window.__dumber_gui_manager.destroy(); | 92 | window.__dumber_gui_manager.destroy(); | ||
| 93 | } | 93 | } | ||
| 94 | - | `) | 94 | + | `); err != nil { |
| 95 | + | log.Printf("[pane-%s] failed to inject cleanup script: %v", p.id, err) | |||
| 96 | + | } | |||
| 95 | } | 97 | } | ||
| 96 | 98 | ||||
| 97 | // Clear tracking | 99 | // Clear tracking | ||
M internal/app/browser/workspace_manager.go +233 -54
| @@ -113,7 +113,9 @@ func NewWorkspaceManager(app *BrowserApp, rootPane *BrowserPane) *WorkspaceManag | |||
| 113 | 113 | if err != nil { | |
| 114 | 114 | return nil, err | |
| 115 | 115 | } | |
| 116 | - | cfg.CreateWindow = false | |
| 116 | + | // Keep CreateWindow = true for popup WebViews to ensure proper window features initialization | |
| 117 | + | // This prevents WindowFeatures optional crashes when WebKit accesses popup properties | |
| 118 | + | cfg.CreateWindow = true | |
| 117 | 119 | return webkit.NewWebView(cfg) | |
| 118 | 120 | } | |
| 119 | 121 | manager.createPaneFn = func(view *webkit.WebView) (*BrowserPane, error) { | |
| @@ -808,7 +810,7 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 808 | 810 | ||
| 809 | 811 | // Detach existing container from its current GTK parent before inserting into new paned. | |
| 810 | 812 | if parent == nil { | |
| 811 | - | // Target is the root - remove it from the window | |
| 813 | + | // Target is the root - remove it from the window and unparent it | |
| 812 | 814 | log.Printf("[workspace] removing existing container=%#x from window", existingContainer) | |
| 813 | 815 | if wm.window != nil { | |
| 814 | 816 | wm.window.SetChild(0) | |
| @@ -821,51 +823,71 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 821 | 823 | } else if parent.right == target { | |
| 822 | 824 | webkit.PanedSetEndChild(parent.container, 0) | |
| 823 | 825 | } | |
| 826 | + | } | |
| 827 | + | ||
| 828 | + | // Properly unparent the widget from GTK's perspective | |
| 829 | + | webkit.WidgetUnparent(existingContainer) | |
| 830 | + | if parent != nil && parent.container != 0 { | |
| 824 | 831 | webkit.WidgetQueueAllocate(parent.container) | |
| 825 | 832 | } | |
| 826 | 833 | ||
| 827 | - | // Add both containers to the new paned | |
| 828 | - | // Note: GTK4 may emit a critical error about widget parenting here when splitting | |
| 829 | - | // the root pane, but the operation still succeeds. This is a limitation of GTK's | |
| 830 | - | // widget hierarchy tracking when widgets are moved between window and paned containers. | |
| 834 | + | // Set up the tree structure first | |
| 831 | 835 | if existingFirst { | |
| 832 | 836 | split.left = target | |
| 833 | 837 | split.right = newLeaf | |
| 834 | - | webkit.PanedSetStartChild(paned, existingContainer) | |
| 835 | - | webkit.PanedSetEndChild(paned, newContainer) | |
| 836 | - | log.Printf("[workspace] added existing=%#x as start child, new=%#x as end child", existingContainer, newContainer) | |
| 837 | 838 | } else { | |
| 838 | 839 | split.left = newLeaf | |
| 839 | 840 | split.right = target | |
| 840 | - | webkit.PanedSetStartChild(paned, newContainer) | |
| 841 | - | webkit.PanedSetEndChild(paned, existingContainer) | |
| 842 | - | log.Printf("[workspace] added new=%#x as start child, existing=%#x as end child", newContainer, existingContainer) | |
| 843 | 841 | } | |
| 844 | 842 | ||
| 845 | 843 | target.parent = split | |
| 846 | 844 | newLeaf.parent = split | |
| 847 | 845 | ||
| 846 | + | // Update tree root/parent references | |
| 848 | 847 | if parent == nil { | |
| 849 | 848 | wm.root = split | |
| 850 | - | if wm.window != nil { | |
| 851 | - | wm.window.SetChild(paned) | |
| 852 | - | webkit.WidgetQueueAllocate(paned) | |
| 853 | - | log.Printf("[workspace] paned set as window child: paned=%#x", paned) | |
| 854 | - | } | |
| 855 | 849 | } else { | |
| 856 | 850 | if parent.left == target { | |
| 857 | 851 | parent.left = split | |
| 858 | - | webkit.PanedSetStartChild(parent.container, paned) | |
| 859 | 852 | } else if parent.right == target { | |
| 860 | 853 | parent.right = split | |
| 861 | - | webkit.PanedSetEndChild(parent.container, paned) | |
| 862 | 854 | } | |
| 863 | - | webkit.WidgetQueueAllocate(parent.container) | |
| 864 | - | webkit.WidgetQueueAllocate(paned) | |
| 865 | - | log.Printf("[workspace] paned inserted into parent=%#x", parent.container) | |
| 866 | 855 | } | |
| 867 | 856 | ||
| 868 | - | webkit.WidgetShow(paned) | |
| 857 | + | // Use idle callback to ensure widget unparenting is complete before re-parenting | |
| 858 | + | webkit.IdleAdd(func() bool { | |
| 859 | + | // Add both containers to the new paned | |
| 860 | + | if existingFirst { | |
| 861 | + | webkit.PanedSetStartChild(paned, existingContainer) | |
| 862 | + | webkit.PanedSetEndChild(paned, newContainer) | |
| 863 | + | log.Printf("[workspace] added existing=%#x as start child, new=%#x as end child", existingContainer, newContainer) | |
| 864 | + | } else { | |
| 865 | + | webkit.PanedSetStartChild(paned, newContainer) | |
| 866 | + | webkit.PanedSetEndChild(paned, existingContainer) | |
| 867 | + | log.Printf("[workspace] added new=%#x as start child, existing=%#x as end child", newContainer, existingContainer) | |
| 868 | + | } | |
| 869 | + | ||
| 870 | + | // Attach the new paned to its parent | |
| 871 | + | if parent == nil { | |
| 872 | + | if wm.window != nil { | |
| 873 | + | wm.window.SetChild(paned) | |
| 874 | + | webkit.WidgetQueueAllocate(paned) | |
| 875 | + | log.Printf("[workspace] paned set as window child: paned=%#x", paned) | |
| 876 | + | } | |
| 877 | + | } else { | |
| 878 | + | if parent.left == split { | |
| 879 | + | webkit.PanedSetStartChild(parent.container, paned) | |
| 880 | + | } else if parent.right == split { | |
| 881 | + | webkit.PanedSetEndChild(parent.container, paned) | |
| 882 | + | } | |
| 883 | + | webkit.WidgetQueueAllocate(parent.container) | |
| 884 | + | webkit.WidgetQueueAllocate(paned) | |
| 885 | + | log.Printf("[workspace] paned inserted into parent=%#x", parent.container) | |
| 886 | + | } | |
| 887 | + | ||
| 888 | + | webkit.WidgetShow(paned) | |
| 889 | + | return false // Run once | |
| 890 | + | }) | |
| 869 | 891 | ||
| 870 | 892 | wm.viewToNode[newPane.webView] = newLeaf | |
| 871 | 893 | wm.ensureHover(newLeaf) | |
| @@ -939,7 +961,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 939 | 961 | if willBeLastPane && wm.root == node { | |
| 940 | 962 | log.Printf("[workspace] closing final pane; exiting browser") | |
| 941 | 963 | wm.detachHover(node) | |
| 942 | - | node.pane.webView.Destroy() | |
| 964 | + | if err := node.pane.webView.Destroy(); err != nil { | |
| 965 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |
| 966 | + | } | |
| 943 | 967 | webkit.QuitMainLoop() | |
| 944 | 968 | return nil | |
| 945 | 969 | } | |
| @@ -957,7 +981,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 957 | 981 | // No other panes exist, this is the final pane | |
| 958 | 982 | log.Printf("[workspace] closing final pane; exiting browser") | |
| 959 | 983 | wm.detachHover(node) | |
| 960 | - | node.pane.webView.Destroy() | |
| 984 | + | if err := node.pane.webView.Destroy(); err != nil { | |
| 985 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |
| 986 | + | } | |
| 961 | 987 | webkit.QuitMainLoop() | |
| 962 | 988 | return nil | |
| 963 | 989 | } | |
| @@ -1010,7 +1036,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1010 | 1036 | wm.detachHover(node) | |
| 1011 | 1037 | if willBeLastPane { | |
| 1012 | 1038 | // This is the last pane, safe to destroy completely | |
| 1013 | - | node.pane.webView.Destroy() | |
| 1039 | + | if err := node.pane.webView.Destroy(); err != nil { | |
| 1040 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |
| 1041 | + | } | |
| 1014 | 1042 | } else { | |
| 1015 | 1043 | // Multiple panes remain, don't destroy the window - just clean up the webview | |
| 1016 | 1044 | log.Printf("[workspace] skipping webview destruction to preserve window (panes remaining: %d)", remaining-1) | |
| @@ -1141,7 +1169,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1141 | 1169 | wm.detachHover(node) | |
| 1142 | 1170 | if willBeLastPane { | |
| 1143 | 1171 | // This is the last pane, safe to destroy completely | |
| 1144 | - | node.pane.webView.Destroy() | |
| 1172 | + | if err := node.pane.webView.Destroy(); err != nil { | |
| 1173 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |
| 1174 | + | } | |
| 1145 | 1175 | } else { | |
| 1146 | 1176 | // Multiple panes remain, don't destroy the window - just clean up the webview | |
| 1147 | 1177 | log.Printf("[workspace] skipping webview destruction to preserve window (panes remaining: %d)", remaining-1) | |
| @@ -1222,70 +1252,219 @@ func (wm *WorkspaceManager) updateMainPane() { | |||
| 1222 | 1252 | } | |
| 1223 | 1253 | } | |
| 1224 | 1254 | ||
| 1225 | - | func (wm *WorkspaceManager) HandlePopup(source *webkit.WebView, url string) bool { | |
| 1255 | + | func (wm *WorkspaceManager) HandlePopup(source *webkit.WebView, url string) *webkit.WebView { | |
| 1256 | + | log.Printf("[workspace] HandlePopup called for URL: %s", url) | |
| 1257 | + | ||
| 1226 | 1258 | if wm == nil || source == nil { | |
| 1227 | - | return false | |
| 1259 | + | log.Printf("[workspace] HandlePopup: nil workspace manager or source - allowing native popup") | |
| 1260 | + | return nil | |
| 1228 | 1261 | } | |
| 1229 | 1262 | ||
| 1230 | 1263 | node := wm.viewToNode[source] | |
| 1231 | 1264 | if node == nil { | |
| 1232 | - | log.Printf("[workspace] popup from unknown webview") | |
| 1233 | - | return false | |
| 1265 | + | log.Printf("[workspace] popup from unknown webview - allowing native popup") | |
| 1266 | + | return nil | |
| 1234 | 1267 | } | |
| 1235 | 1268 | ||
| 1236 | 1269 | cfg := wm.app.config | |
| 1237 | 1270 | if cfg == nil { | |
| 1238 | - | return false | |
| 1271 | + | log.Printf("[workspace] HandlePopup: nil config - allowing native popup") | |
| 1272 | + | return nil | |
| 1239 | 1273 | } | |
| 1240 | 1274 | ||
| 1275 | + | ||
| 1241 | 1276 | popCfg := cfg.Workspace.Popups | |
| 1277 | + | log.Printf("[workspace] Popup config - OpenInNewPane: %v, Placement: %s", popCfg.OpenInNewPane, popCfg.Placement) | |
| 1278 | + | ||
| 1242 | 1279 | if !popCfg.OpenInNewPane { | |
| 1243 | - | return false | |
| 1280 | + | log.Printf("[workspace] Popup creation disabled in config - allowing native popup") | |
| 1281 | + | return nil | |
| 1282 | + | } | |
| 1283 | + | ||
| 1284 | + | // Create a new WebView related to the source for process sharing and proper WindowFeatures | |
| 1285 | + | webkitCfg, err := wm.app.buildWebkitConfig() | |
| 1286 | + | if err != nil { | |
| 1287 | + | log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err) | |
| 1288 | + | return nil | |
| 1289 | + | } | |
| 1290 | + | // Don't create window initially - we'll embed directly in workspace | |
| 1291 | + | webkitCfg.CreateWindow = false | |
| 1292 | + | ||
| 1293 | + | newView, err := webkit.NewWebViewWithRelated(webkitCfg, source) | |
| 1294 | + | if err != nil { | |
| 1295 | + | log.Printf("[workspace] failed to create related popup WebView: %v - allowing native popup", err) | |
| 1296 | + | return nil | |
| 1297 | + | } | |
| 1298 | + | ||
| 1299 | + | // Create a pane for the new WebView | |
| 1300 | + | newPane, err := wm.createPane(newView) | |
| 1301 | + | if err != nil { | |
| 1302 | + | log.Printf("[workspace] failed to create popup pane: %v - allowing native popup", err) | |
| 1303 | + | return nil | |
| 1244 | 1304 | } | |
| 1245 | 1305 | ||
| 1306 | + | // Determine placement direction | |
| 1246 | 1307 | direction := strings.ToLower(popCfg.Placement) | |
| 1247 | 1308 | if direction == "" { | |
| 1248 | 1309 | direction = "right" | |
| 1249 | 1310 | } | |
| 1250 | 1311 | ||
| 1312 | + | // Determine target node for splitting | |
| 1251 | 1313 | target := node | |
| 1252 | 1314 | if !popCfg.FollowPaneContext && wm.active != nil { | |
| 1253 | 1315 | target = wm.active | |
| 1254 | 1316 | } | |
| 1255 | 1317 | ||
| 1256 | - | newNode, err := wm.splitNode(target, direction) | |
| 1257 | - | if err != nil { | |
| 1258 | - | log.Printf("[workspace] popup split failed: %v", err) | |
| 1259 | - | return false | |
| 1318 | + | // Add the popup pane to the workspace using manual pane insertion | |
| 1319 | + | if err := wm.insertPopupPane(target, newPane, direction); err != nil { | |
| 1320 | + | log.Printf("[workspace] popup pane insertion failed: %v - allowing native popup", err) | |
| 1321 | + | return nil | |
| 1260 | 1322 | } | |
| 1261 | 1323 | ||
| 1262 | - | if url == "" { | |
| 1263 | - | return true | |
| 1324 | + | // Load the URL if provided | |
| 1325 | + | if url != "" { | |
| 1326 | + | if err := newView.LoadURL(url); err != nil { | |
| 1327 | + | log.Printf("[workspace] failed to load popup URL: %v", err) | |
| 1328 | + | } | |
| 1264 | 1329 | } | |
| 1265 | 1330 | ||
| 1266 | - | if newNode == nil || newNode.pane == nil { | |
| 1267 | - | return true | |
| 1331 | + | log.Printf("[workspace] Created popup pane for URL: %s", url) | |
| 1332 | + | return newView | |
| 1333 | + | } | |
| 1334 | + | ||
| 1335 | + | ||
| 1336 | + | // insertPopupPane inserts a pre-created popup pane into the workspace | |
| 1337 | + | func (wm *WorkspaceManager) insertPopupPane(target *paneNode, newPane *BrowserPane, direction string) error { | |
| 1338 | + | if target == nil || !target.isLeaf || target.pane == nil { | |
| 1339 | + | return errors.New("insert target must be a leaf pane") | |
| 1268 | 1340 | } | |
| 1269 | 1341 | ||
| 1270 | - | if newNode.pane.navigationController != nil { | |
| 1271 | - | if err := newNode.pane.navigationController.NavigateToURL(url); err != nil { | |
| 1272 | - | log.Printf("[workspace] popup navigation failed: %v", err) | |
| 1273 | - | if newNode.pane.webView != nil { | |
| 1274 | - | if loadErr := newNode.pane.webView.LoadURL(url); loadErr != nil { | |
| 1275 | - | log.Printf("[workspace] popup load fallback failed: %v", loadErr) | |
| 1276 | - | } | |
| 1277 | - | } | |
| 1342 | + | if newPane == nil || newPane.webView == nil { | |
| 1343 | + | return errors.New("new pane missing webview") | |
| 1344 | + | } | |
| 1345 | + | ||
| 1346 | + | if handler := newPane.MessageHandler(); handler != nil { | |
| 1347 | + | handler.SetWorkspaceObserver(wm) | |
| 1348 | + | } | |
| 1349 | + | ||
| 1350 | + | newContainer := newPane.webView.RootWidget() | |
| 1351 | + | if newContainer == 0 { | |
| 1352 | + | return errors.New("new pane missing container") | |
| 1353 | + | } | |
| 1354 | + | ||
| 1355 | + | webkit.WidgetSetHExpand(newContainer, true) | |
| 1356 | + | webkit.WidgetSetVExpand(newContainer, true) | |
| 1357 | + | webkit.WidgetRealizeInContainer(newContainer) | |
| 1358 | + | ||
| 1359 | + | existingContainer := target.container | |
| 1360 | + | if existingContainer == 0 { | |
| 1361 | + | return errors.New("existing pane missing container") | |
| 1362 | + | } | |
| 1363 | + | ||
| 1364 | + | orientation, existingFirst := mapDirection(direction) | |
| 1365 | + | log.Printf("[workspace] inserting popup direction=%s orientation=%v existingFirst=%v target.parent=%p", direction, orientation, existingFirst, target.parent) | |
| 1366 | + | ||
| 1367 | + | paned := webkit.NewPaned(orientation) | |
| 1368 | + | if paned == 0 { | |
| 1369 | + | return errors.New("failed to create GtkPaned") | |
| 1370 | + | } | |
| 1371 | + | webkit.WidgetSetHExpand(paned, true) | |
| 1372 | + | webkit.WidgetSetVExpand(paned, true) | |
| 1373 | + | webkit.PanedSetResizeStart(paned, true) | |
| 1374 | + | webkit.PanedSetResizeEnd(paned, true) | |
| 1375 | + | ||
| 1376 | + | newLeaf := &paneNode{ | |
| 1377 | + | pane: newPane, | |
| 1378 | + | container: newContainer, | |
| 1379 | + | isLeaf: true, | |
| 1380 | + | } | |
| 1381 | + | split := &paneNode{ | |
| 1382 | + | parent: target.parent, | |
| 1383 | + | container: paned, | |
| 1384 | + | orientation: orientation, | |
| 1385 | + | isLeaf: false, | |
| 1386 | + | } | |
| 1387 | + | ||
| 1388 | + | parent := split.parent | |
| 1389 | + | ||
| 1390 | + | // Detach existing container from its current GTK parent before inserting into new paned. | |
| 1391 | + | if parent == nil { | |
| 1392 | + | // Target is the root - remove it from the window | |
| 1393 | + | log.Printf("[workspace] removing existing container=%#x from window", existingContainer) | |
| 1394 | + | if wm.window != nil { | |
| 1395 | + | wm.window.SetChild(0) | |
| 1278 | 1396 | } | |
| 1279 | - | return true | |
| 1397 | + | } else if parent.container != 0 { | |
| 1398 | + | // Target has a parent paned - unparent it from there | |
| 1399 | + | log.Printf("[workspace] unparenting existing container=%#x from parent paned=%#x", existingContainer, parent.container) | |
| 1400 | + | if parent.left == target { | |
| 1401 | + | webkit.PanedSetStartChild(parent.container, 0) | |
| 1402 | + | } else if parent.right == target { | |
| 1403 | + | webkit.PanedSetEndChild(parent.container, 0) | |
| 1404 | + | } | |
| 1405 | + | webkit.WidgetQueueAllocate(parent.container) | |
| 1406 | + | } | |
| 1407 | + | ||
| 1408 | + | // Add both containers to the new paned | |
| 1409 | + | if existingFirst { | |
| 1410 | + | split.left = target | |
| 1411 | + | split.right = newLeaf | |
| 1412 | + | webkit.PanedSetStartChild(paned, existingContainer) | |
| 1413 | + | webkit.PanedSetEndChild(paned, newContainer) | |
| 1414 | + | log.Printf("[workspace] added existing=%#x as start child, new=%#x as end child", existingContainer, newContainer) | |
| 1415 | + | } else { | |
| 1416 | + | split.left = newLeaf | |
| 1417 | + | split.right = target | |
| 1418 | + | webkit.PanedSetStartChild(paned, newContainer) | |
| 1419 | + | webkit.PanedSetEndChild(paned, existingContainer) | |
| 1420 | + | log.Printf("[workspace] added new=%#x as start child, existing=%#x as end child", newContainer, existingContainer) | |
| 1280 | 1421 | } | |
| 1281 | 1422 | ||
| 1282 | - | if newNode.pane.webView != nil { | |
| 1283 | - | if err := newNode.pane.webView.LoadURL(url); err != nil { | |
| 1284 | - | log.Printf("[workspace] popup load failed: %v", err) | |
| 1423 | + | target.parent = split | |
| 1424 | + | newLeaf.parent = split | |
| 1425 | + | ||
| 1426 | + | if parent == nil { | |
| 1427 | + | wm.root = split | |
| 1428 | + | if wm.window != nil { | |
| 1429 | + | wm.window.SetChild(paned) | |
| 1430 | + | webkit.WidgetQueueAllocate(paned) | |
| 1431 | + | log.Printf("[workspace] paned set as window child: paned=%#x", paned) | |
| 1432 | + | } | |
| 1433 | + | } else { | |
| 1434 | + | if parent.left == target { | |
| 1435 | + | parent.left = split | |
| 1436 | + | webkit.PanedSetStartChild(parent.container, paned) | |
| 1437 | + | } else if parent.right == target { | |
| 1438 | + | parent.right = split | |
| 1439 | + | webkit.PanedSetEndChild(parent.container, paned) | |
| 1285 | 1440 | } | |
| 1441 | + | webkit.WidgetQueueAllocate(parent.container) | |
| 1442 | + | webkit.WidgetQueueAllocate(paned) | |
| 1443 | + | log.Printf("[workspace] paned inserted into parent=%#x", parent.container) | |
| 1444 | + | } | |
| 1445 | + | ||
| 1446 | + | webkit.WidgetShow(paned) | |
| 1447 | + | ||
| 1448 | + | wm.viewToNode[newPane.webView] = newLeaf | |
| 1449 | + | wm.ensureHover(newLeaf) | |
| 1450 | + | wm.ensureHover(target) | |
| 1451 | + | wm.app.panes = append(wm.app.panes, newPane) | |
| 1452 | + | if newPane.zoomController != nil { | |
| 1453 | + | newPane.zoomController.ApplyInitialZoom() | |
| 1286 | 1454 | } | |
| 1287 | 1455 | ||
| 1288 | - | return true | |
| 1456 | + | // Update CSS classes for all panes now that we have multiple panes | |
| 1457 | + | wm.ensurePaneBaseClasses() | |
| 1458 | + | ||
| 1459 | + | webkit.IdleAdd(func() bool { | |
| 1460 | + | if newContainer != 0 { | |
| 1461 | + | webkit.WidgetShow(newContainer) | |
| 1462 | + | } | |
| 1463 | + | wm.focusNode(newLeaf) | |
| 1464 | + | return false | |
| 1465 | + | }) | |
| 1466 | + | ||
| 1467 | + | return nil | |
| 1289 | 1468 | } | |
| 1290 | 1469 | ||
| 1291 | 1470 | // ensureGUIInPane lazily loads GUI components into a pane when it gains focus | |
| @@ -113,7 +113,9 @@ func NewWorkspaceManager(app *BrowserApp, rootPane *BrowserPane) *WorkspaceManag | |||||
| 113 | if err != nil { | 113 | if err != nil { | ||
| 114 | return nil, err | 114 | return nil, err | ||
| 115 | } | 115 | } | ||
| 116 | - | cfg.CreateWindow = false | 116 | + | // Keep CreateWindow = true for popup WebViews to ensure proper window features initialization |
| 117 | + | // This prevents WindowFeatures optional crashes when WebKit accesses popup properties | |||
| 118 | + | cfg.CreateWindow = true | |||
| 117 | return webkit.NewWebView(cfg) | 119 | return webkit.NewWebView(cfg) | ||
| 118 | } | 120 | } | ||
| 119 | manager.createPaneFn = func(view *webkit.WebView) (*BrowserPane, error) { | 121 | manager.createPaneFn = func(view *webkit.WebView) (*BrowserPane, error) { | ||
| @@ -808,7 +810,7 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 808 | 810 | ||||
| 809 | // Detach existing container from its current GTK parent before inserting into new paned. | 811 | // Detach existing container from its current GTK parent before inserting into new paned. | ||
| 810 | if parent == nil { | 812 | if parent == nil { | ||
| 811 | - | // Target is the root - remove it from the window | 813 | + | // Target is the root - remove it from the window and unparent it |
| 812 | log.Printf("[workspace] removing existing container=%#x from window", existingContainer) | 814 | log.Printf("[workspace] removing existing container=%#x from window", existingContainer) | ||
| 813 | if wm.window != nil { | 815 | if wm.window != nil { | ||
| 814 | wm.window.SetChild(0) | 816 | wm.window.SetChild(0) | ||
| @@ -821,51 +823,71 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 821 | } else if parent.right == target { | 823 | } else if parent.right == target { | ||
| 822 | webkit.PanedSetEndChild(parent.container, 0) | 824 | webkit.PanedSetEndChild(parent.container, 0) | ||
| 823 | } | 825 | } | ||
| 826 | + | } | |||
| 827 | + | ||||
| 828 | + | // Properly unparent the widget from GTK's perspective | |||
| 829 | + | webkit.WidgetUnparent(existingContainer) | |||
| 830 | + | if parent != nil && parent.container != 0 { | |||
| 824 | webkit.WidgetQueueAllocate(parent.container) | 831 | webkit.WidgetQueueAllocate(parent.container) | ||
| 825 | } | 832 | } | ||
| 826 | 833 | ||||
| 827 | - | // Add both containers to the new paned | 834 | + | // Set up the tree structure first |
| 828 | - | // Note: GTK4 may emit a critical error about widget parenting here when splitting | |||
| 829 | - | // the root pane, but the operation still succeeds. This is a limitation of GTK's | |||
| 830 | - | // widget hierarchy tracking when widgets are moved between window and paned containers. | |||
| 831 | if existingFirst { | 835 | if existingFirst { | ||
| 832 | split.left = target | 836 | split.left = target | ||
| 833 | split.right = newLeaf | 837 | split.right = newLeaf | ||
| 834 | - | webkit.PanedSetStartChild(paned, existingContainer) | |||
| 835 | - | webkit.PanedSetEndChild(paned, newContainer) | |||
| 836 | - | log.Printf("[workspace] added existing=%#x as start child, new=%#x as end child", existingContainer, newContainer) | |||
| 837 | } else { | 838 | } else { | ||
| 838 | split.left = newLeaf | 839 | split.left = newLeaf | ||
| 839 | split.right = target | 840 | split.right = target | ||
| 840 | - | webkit.PanedSetStartChild(paned, newContainer) | |||
| 841 | - | webkit.PanedSetEndChild(paned, existingContainer) | |||
| 842 | - | log.Printf("[workspace] added new=%#x as start child, existing=%#x as end child", newContainer, existingContainer) | |||
| 843 | } | 841 | } | ||
| 844 | 842 | ||||
| 845 | target.parent = split | 843 | target.parent = split | ||
| 846 | newLeaf.parent = split | 844 | newLeaf.parent = split | ||
| 847 | 845 | ||||
| 846 | + | // Update tree root/parent references | |||
| 848 | if parent == nil { | 847 | if parent == nil { | ||
| 849 | wm.root = split | 848 | wm.root = split | ||
| 850 | - | if wm.window != nil { | |||
| 851 | - | wm.window.SetChild(paned) | |||
| 852 | - | webkit.WidgetQueueAllocate(paned) | |||
| 853 | - | log.Printf("[workspace] paned set as window child: paned=%#x", paned) | |||
| 854 | - | } | |||
| 855 | } else { | 849 | } else { | ||
| 856 | if parent.left == target { | 850 | if parent.left == target { | ||
| 857 | parent.left = split | 851 | parent.left = split | ||
| 858 | - | webkit.PanedSetStartChild(parent.container, paned) | |||
| 859 | } else if parent.right == target { | 852 | } else if parent.right == target { | ||
| 860 | parent.right = split | 853 | parent.right = split | ||
| 861 | - | webkit.PanedSetEndChild(parent.container, paned) | |||
| 862 | } | 854 | } | ||
| 863 | - | webkit.WidgetQueueAllocate(parent.container) | |||
| 864 | - | webkit.WidgetQueueAllocate(paned) | |||
| 865 | - | log.Printf("[workspace] paned inserted into parent=%#x", parent.container) | |||
| 866 | } | 855 | } | ||
| 867 | 856 | ||||
| 868 | - | webkit.WidgetShow(paned) | 857 | + | // Use idle callback to ensure widget unparenting is complete before re-parenting |
| 858 | + | webkit.IdleAdd(func() bool { | |||
| 859 | + | // Add both containers to the new paned | |||
| 860 | + | if existingFirst { | |||
| 861 | + | webkit.PanedSetStartChild(paned, existingContainer) | |||
| 862 | + | webkit.PanedSetEndChild(paned, newContainer) | |||
| 863 | + | log.Printf("[workspace] added existing=%#x as start child, new=%#x as end child", existingContainer, newContainer) | |||
| 864 | + | } else { | |||
| 865 | + | webkit.PanedSetStartChild(paned, newContainer) | |||
| 866 | + | webkit.PanedSetEndChild(paned, existingContainer) | |||
| 867 | + | log.Printf("[workspace] added new=%#x as start child, existing=%#x as end child", newContainer, existingContainer) | |||
| 868 | + | } | |||
| 869 | + | ||||
| 870 | + | // Attach the new paned to its parent | |||
| 871 | + | if parent == nil { | |||
| 872 | + | if wm.window != nil { | |||
| 873 | + | wm.window.SetChild(paned) | |||
| 874 | + | webkit.WidgetQueueAllocate(paned) | |||
| 875 | + | log.Printf("[workspace] paned set as window child: paned=%#x", paned) | |||
| 876 | + | } | |||
| 877 | + | } else { | |||
| 878 | + | if parent.left == split { | |||
| 879 | + | webkit.PanedSetStartChild(parent.container, paned) | |||
| 880 | + | } else if parent.right == split { | |||
| 881 | + | webkit.PanedSetEndChild(parent.container, paned) | |||
| 882 | + | } | |||
| 883 | + | webkit.WidgetQueueAllocate(parent.container) | |||
| 884 | + | webkit.WidgetQueueAllocate(paned) | |||
| 885 | + | log.Printf("[workspace] paned inserted into parent=%#x", parent.container) | |||
| 886 | + | } | |||
| 887 | + | ||||
| 888 | + | webkit.WidgetShow(paned) | |||
| 889 | + | return false // Run once | |||
| 890 | + | }) | |||
| 869 | 891 | ||||
| 870 | wm.viewToNode[newPane.webView] = newLeaf | 892 | wm.viewToNode[newPane.webView] = newLeaf | ||
| 871 | wm.ensureHover(newLeaf) | 893 | wm.ensureHover(newLeaf) | ||
| @@ -939,7 +961,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 939 | if willBeLastPane && wm.root == node { | 961 | if willBeLastPane && wm.root == node { | ||
| 940 | log.Printf("[workspace] closing final pane; exiting browser") | 962 | log.Printf("[workspace] closing final pane; exiting browser") | ||
| 941 | wm.detachHover(node) | 963 | wm.detachHover(node) | ||
| 942 | - | node.pane.webView.Destroy() | 964 | + | if err := node.pane.webView.Destroy(); err != nil { |
| 965 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |||
| 966 | + | } | |||
| 943 | webkit.QuitMainLoop() | 967 | webkit.QuitMainLoop() | ||
| 944 | return nil | 968 | return nil | ||
| 945 | } | 969 | } | ||
| @@ -957,7 +981,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 957 | // No other panes exist, this is the final pane | 981 | // No other panes exist, this is the final pane | ||
| 958 | log.Printf("[workspace] closing final pane; exiting browser") | 982 | log.Printf("[workspace] closing final pane; exiting browser") | ||
| 959 | wm.detachHover(node) | 983 | wm.detachHover(node) | ||
| 960 | - | node.pane.webView.Destroy() | 984 | + | if err := node.pane.webView.Destroy(); err != nil { |
| 985 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |||
| 986 | + | } | |||
| 961 | webkit.QuitMainLoop() | 987 | webkit.QuitMainLoop() | ||
| 962 | return nil | 988 | return nil | ||
| 963 | } | 989 | } | ||
| @@ -1010,7 +1036,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1010 | wm.detachHover(node) | 1036 | wm.detachHover(node) | ||
| 1011 | if willBeLastPane { | 1037 | if willBeLastPane { | ||
| 1012 | // This is the last pane, safe to destroy completely | 1038 | // This is the last pane, safe to destroy completely | ||
| 1013 | - | node.pane.webView.Destroy() | 1039 | + | if err := node.pane.webView.Destroy(); err != nil { |
| 1040 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |||
| 1041 | + | } | |||
| 1014 | } else { | 1042 | } else { | ||
| 1015 | // Multiple panes remain, don't destroy the window - just clean up the webview | 1043 | // Multiple panes remain, don't destroy the window - just clean up the webview | ||
| 1016 | log.Printf("[workspace] skipping webview destruction to preserve window (panes remaining: %d)", remaining-1) | 1044 | log.Printf("[workspace] skipping webview destruction to preserve window (panes remaining: %d)", remaining-1) | ||
| @@ -1141,7 +1169,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1141 | wm.detachHover(node) | 1169 | wm.detachHover(node) | ||
| 1142 | if willBeLastPane { | 1170 | if willBeLastPane { | ||
| 1143 | // This is the last pane, safe to destroy completely | 1171 | // This is the last pane, safe to destroy completely | ||
| 1144 | - | node.pane.webView.Destroy() | 1172 | + | if err := node.pane.webView.Destroy(); err != nil { |
| 1173 | + | log.Printf("[workspace] failed to destroy webview: %v", err) | |||
| 1174 | + | } | |||
| 1145 | } else { | 1175 | } else { | ||
| 1146 | // Multiple panes remain, don't destroy the window - just clean up the webview | 1176 | // Multiple panes remain, don't destroy the window - just clean up the webview | ||
| 1147 | log.Printf("[workspace] skipping webview destruction to preserve window (panes remaining: %d)", remaining-1) | 1177 | log.Printf("[workspace] skipping webview destruction to preserve window (panes remaining: %d)", remaining-1) | ||
| @@ -1222,70 +1252,219 @@ func (wm *WorkspaceManager) updateMainPane() { | |||||
| 1222 | } | 1252 | } | ||
| 1223 | } | 1253 | } | ||
| 1224 | 1254 | ||||
| 1225 | - | func (wm *WorkspaceManager) HandlePopup(source *webkit.WebView, url string) bool { | 1255 | + | func (wm *WorkspaceManager) HandlePopup(source *webkit.WebView, url string) *webkit.WebView { |
| 1256 | + | log.Printf("[workspace] HandlePopup called for URL: %s", url) | |||
| 1257 | + | ||||
| 1226 | if wm == nil || source == nil { | 1258 | if wm == nil || source == nil { | ||
| 1227 | - | return false | 1259 | + | log.Printf("[workspace] HandlePopup: nil workspace manager or source - allowing native popup") |
| 1260 | + | return nil | |||
| 1228 | } | 1261 | } | ||
| 1229 | 1262 | ||||
| 1230 | node := wm.viewToNode[source] | 1263 | node := wm.viewToNode[source] | ||
| 1231 | if node == nil { | 1264 | if node == nil { | ||
| 1232 | - | log.Printf("[workspace] popup from unknown webview") | 1265 | + | log.Printf("[workspace] popup from unknown webview - allowing native popup") |
| 1233 | - | return false | 1266 | + | return nil |
| 1234 | } | 1267 | } | ||
| 1235 | 1268 | ||||
| 1236 | cfg := wm.app.config | 1269 | cfg := wm.app.config | ||
| 1237 | if cfg == nil { | 1270 | if cfg == nil { | ||
| 1238 | - | return false | 1271 | + | log.Printf("[workspace] HandlePopup: nil config - allowing native popup") |
| 1272 | + | return nil | |||
| 1239 | } | 1273 | } | ||
| 1240 | 1274 | ||||
| 1275 | + | ||||
| 1241 | popCfg := cfg.Workspace.Popups | 1276 | popCfg := cfg.Workspace.Popups | ||
| 1277 | + | log.Printf("[workspace] Popup config - OpenInNewPane: %v, Placement: %s", popCfg.OpenInNewPane, popCfg.Placement) | |||
| 1278 | + | ||||
| 1242 | if !popCfg.OpenInNewPane { | 1279 | if !popCfg.OpenInNewPane { | ||
| 1243 | - | return false | 1280 | + | log.Printf("[workspace] Popup creation disabled in config - allowing native popup") |
| 1281 | + | return nil | |||
| 1282 | + | } | |||
| 1283 | + | ||||
| 1284 | + | // Create a new WebView related to the source for process sharing and proper WindowFeatures | |||
| 1285 | + | webkitCfg, err := wm.app.buildWebkitConfig() | |||
| 1286 | + | if err != nil { | |||
| 1287 | + | log.Printf("[workspace] failed to build webkit config: %v - allowing native popup", err) | |||
| 1288 | + | return nil | |||
| 1289 | + | } | |||
| 1290 | + | // Don't create window initially - we'll embed directly in workspace | |||
| 1291 | + | webkitCfg.CreateWindow = false | |||
| 1292 | + | ||||
| 1293 | + | newView, err := webkit.NewWebViewWithRelated(webkitCfg, source) | |||
| 1294 | + | if err != nil { | |||
| 1295 | + | log.Printf("[workspace] failed to create related popup WebView: %v - allowing native popup", err) | |||
| 1296 | + | return nil | |||
| 1297 | + | } | |||
| 1298 | + | ||||
| 1299 | + | // Create a pane for the new WebView | |||
| 1300 | + | newPane, err := wm.createPane(newView) | |||
| 1301 | + | if err != nil { | |||
| 1302 | + | log.Printf("[workspace] failed to create popup pane: %v - allowing native popup", err) | |||
| 1303 | + | return nil | |||
| 1244 | } | 1304 | } | ||
| 1245 | 1305 | ||||
| 1306 | + | // Determine placement direction | |||
| 1246 | direction := strings.ToLower(popCfg.Placement) | 1307 | direction := strings.ToLower(popCfg.Placement) | ||
| 1247 | if direction == "" { | 1308 | if direction == "" { | ||
| 1248 | direction = "right" | 1309 | direction = "right" | ||
| 1249 | } | 1310 | } | ||
| 1250 | 1311 | ||||
| 1312 | + | // Determine target node for splitting | |||
| 1251 | target := node | 1313 | target := node | ||
| 1252 | if !popCfg.FollowPaneContext && wm.active != nil { | 1314 | if !popCfg.FollowPaneContext && wm.active != nil { | ||
| 1253 | target = wm.active | 1315 | target = wm.active | ||
| 1254 | } | 1316 | } | ||
| 1255 | 1317 | ||||
| 1256 | - | newNode, err := wm.splitNode(target, direction) | 1318 | + | // Add the popup pane to the workspace using manual pane insertion |
| 1257 | - | if err != nil { | 1319 | + | if err := wm.insertPopupPane(target, newPane, direction); err != nil { |
| 1258 | - | log.Printf("[workspace] popup split failed: %v", err) | 1320 | + | log.Printf("[workspace] popup pane insertion failed: %v - allowing native popup", err) |
| 1259 | - | return false | 1321 | + | return nil |
| 1260 | } | 1322 | } | ||
| 1261 | 1323 | ||||
| 1262 | - | if url == "" { | 1324 | + | // Load the URL if provided |
| 1263 | - | return true | 1325 | + | if url != "" { |
| 1326 | + | if err := newView.LoadURL(url); err != nil { | |||
| 1327 | + | log.Printf("[workspace] failed to load popup URL: %v", err) | |||
| 1328 | + | } | |||
| 1264 | } | 1329 | } | ||
| 1265 | 1330 | ||||
| 1266 | - | if newNode == nil || newNode.pane == nil { | 1331 | + | log.Printf("[workspace] Created popup pane for URL: %s", url) |
| 1267 | - | return true | 1332 | + | return newView |
| 1333 | + | } | |||
| 1334 | + | ||||
| 1335 | + | ||||
| 1336 | + | // insertPopupPane inserts a pre-created popup pane into the workspace | |||
| 1337 | + | func (wm *WorkspaceManager) insertPopupPane(target *paneNode, newPane *BrowserPane, direction string) error { | |||
| 1338 | + | if target == nil || !target.isLeaf || target.pane == nil { | |||
| 1339 | + | return errors.New("insert target must be a leaf pane") | |||
| 1268 | } | 1340 | } | ||
| 1269 | 1341 | ||||
| 1270 | - | if newNode.pane.navigationController != nil { | 1342 | + | if newPane == nil || newPane.webView == nil { |
| 1271 | - | if err := newNode.pane.navigationController.NavigateToURL(url); err != nil { | 1343 | + | return errors.New("new pane missing webview") |
| 1272 | - | log.Printf("[workspace] popup navigation failed: %v", err) | 1344 | + | } |
| 1273 | - | if newNode.pane.webView != nil { | 1345 | + | |
| 1274 | - | if loadErr := newNode.pane.webView.LoadURL(url); loadErr != nil { | 1346 | + | if handler := newPane.MessageHandler(); handler != nil { |
| 1275 | - | log.Printf("[workspace] popup load fallback failed: %v", loadErr) | 1347 | + | handler.SetWorkspaceObserver(wm) |
| 1276 | - | } | 1348 | + | } |
| 1277 | - | } | 1349 | + | |
| 1350 | + | newContainer := newPane.webView.RootWidget() | |||
| 1351 | + | if newContainer == 0 { | |||
| 1352 | + | return errors.New("new pane missing container") | |||
| 1353 | + | } | |||
| 1354 | + | ||||
| 1355 | + | webkit.WidgetSetHExpand(newContainer, true) | |||
| 1356 | + | webkit.WidgetSetVExpand(newContainer, true) | |||
| 1357 | + | webkit.WidgetRealizeInContainer(newContainer) | |||
| 1358 | + | ||||
| 1359 | + | existingContainer := target.container | |||
| 1360 | + | if existingContainer == 0 { | |||
| 1361 | + | return errors.New("existing pane missing container") | |||
| 1362 | + | } | |||
| 1363 | + | ||||
| 1364 | + | orientation, existingFirst := mapDirection(direction) | |||
| 1365 | + | log.Printf("[workspace] inserting popup direction=%s orientation=%v existingFirst=%v target.parent=%p", direction, orientation, existingFirst, target.parent) | |||
| 1366 | + | ||||
| 1367 | + | paned := webkit.NewPaned(orientation) | |||
| 1368 | + | if paned == 0 { | |||
| 1369 | + | return errors.New("failed to create GtkPaned") | |||
| 1370 | + | } | |||
| 1371 | + | webkit.WidgetSetHExpand(paned, true) | |||
| 1372 | + | webkit.WidgetSetVExpand(paned, true) | |||
| 1373 | + | webkit.PanedSetResizeStart(paned, true) | |||
| 1374 | + | webkit.PanedSetResizeEnd(paned, true) | |||
| 1375 | + | ||||
| 1376 | + | newLeaf := &paneNode{ | |||
| 1377 | + | pane: newPane, | |||
| 1378 | + | container: newContainer, | |||
| 1379 | + | isLeaf: true, | |||
| 1380 | + | } | |||
| 1381 | + | split := &paneNode{ | |||
| 1382 | + | parent: target.parent, | |||
| 1383 | + | container: paned, | |||
| 1384 | + | orientation: orientation, | |||
| 1385 | + | isLeaf: false, | |||
| 1386 | + | } | |||
| 1387 | + | ||||
| 1388 | + | parent := split.parent | |||
| 1389 | + | ||||
| 1390 | + | // Detach existing container from its current GTK parent before inserting into new paned. | |||
| 1391 | + | if parent == nil { | |||
| 1392 | + | // Target is the root - remove it from the window | |||
| 1393 | + | log.Printf("[workspace] removing existing container=%#x from window", existingContainer) | |||
| 1394 | + | if wm.window != nil { | |||
| 1395 | + | wm.window.SetChild(0) | |||
| 1278 | } | 1396 | } | ||
| 1279 | - | return true | 1397 | + | } else if parent.container != 0 { |
| 1398 | + | // Target has a parent paned - unparent it from there | |||
| 1399 | + | log.Printf("[workspace] unparenting existing container=%#x from parent paned=%#x", existingContainer, parent.container) | |||
| 1400 | + | if parent.left == target { | |||
| 1401 | + | webkit.PanedSetStartChild(parent.container, 0) | |||
| 1402 | + | } else if parent.right == target { | |||
| 1403 | + | webkit.PanedSetEndChild(parent.container, 0) | |||
| 1404 | + | } | |||
| 1405 | + | webkit.WidgetQueueAllocate(parent.container) | |||
| 1406 | + | } | |||
| 1407 | + | ||||
| 1408 | + | // Add both containers to the new paned | |||
| 1409 | + | if existingFirst { | |||
| 1410 | + | split.left = target | |||
| 1411 | + | split.right = newLeaf | |||
| 1412 | + | webkit.PanedSetStartChild(paned, existingContainer) | |||
| 1413 | + | webkit.PanedSetEndChild(paned, newContainer) | |||
| 1414 | + | log.Printf("[workspace] added existing=%#x as start child, new=%#x as end child", existingContainer, newContainer) | |||
| 1415 | + | } else { | |||
| 1416 | + | split.left = newLeaf | |||
| 1417 | + | split.right = target | |||
| 1418 | + | webkit.PanedSetStartChild(paned, newContainer) | |||
| 1419 | + | webkit.PanedSetEndChild(paned, existingContainer) | |||
| 1420 | + | log.Printf("[workspace] added new=%#x as start child, existing=%#x as end child", newContainer, existingContainer) | |||
| 1280 | } | 1421 | } | ||
| 1281 | 1422 | ||||
| 1282 | - | if newNode.pane.webView != nil { | 1423 | + | target.parent = split |
| 1283 | - | if err := newNode.pane.webView.LoadURL(url); err != nil { | 1424 | + | newLeaf.parent = split |
| 1284 | - | log.Printf("[workspace] popup load failed: %v", err) | 1425 | + | |
| 1426 | + | if parent == nil { | |||
| 1427 | + | wm.root = split | |||
| 1428 | + | if wm.window != nil { | |||
| 1429 | + | wm.window.SetChild(paned) | |||
| 1430 | + | webkit.WidgetQueueAllocate(paned) | |||
| 1431 | + | log.Printf("[workspace] paned set as window child: paned=%#x", paned) | |||
| 1432 | + | } | |||
| 1433 | + | } else { | |||
| 1434 | + | if parent.left == target { | |||
| 1435 | + | parent.left = split | |||
| 1436 | + | webkit.PanedSetStartChild(parent.container, paned) | |||
| 1437 | + | } else if parent.right == target { | |||
| 1438 | + | parent.right = split | |||
| 1439 | + | webkit.PanedSetEndChild(parent.container, paned) | |||
| 1285 | } | 1440 | } | ||
| 1441 | + | webkit.WidgetQueueAllocate(parent.container) | |||
| 1442 | + | webkit.WidgetQueueAllocate(paned) | |||
| 1443 | + | log.Printf("[workspace] paned inserted into parent=%#x", parent.container) | |||
| 1444 | + | } | |||
| 1445 | + | ||||
| 1446 | + | webkit.WidgetShow(paned) | |||
| 1447 | + | ||||
| 1448 | + | wm.viewToNode[newPane.webView] = newLeaf | |||
| 1449 | + | wm.ensureHover(newLeaf) | |||
| 1450 | + | wm.ensureHover(target) | |||
| 1451 | + | wm.app.panes = append(wm.app.panes, newPane) | |||
| 1452 | + | if newPane.zoomController != nil { | |||
| 1453 | + | newPane.zoomController.ApplyInitialZoom() | |||
| 1286 | } | 1454 | } | ||
| 1287 | 1455 | ||||
| 1288 | - | return true | 1456 | + | // Update CSS classes for all panes now that we have multiple panes |
| 1457 | + | wm.ensurePaneBaseClasses() | |||
| 1458 | + | ||||
| 1459 | + | webkit.IdleAdd(func() bool { | |||
| 1460 | + | if newContainer != 0 { | |||
| 1461 | + | webkit.WidgetShow(newContainer) | |||
| 1462 | + | } | |||
| 1463 | + | wm.focusNode(newLeaf) | |||
| 1464 | + | return false | |||
| 1465 | + | }) | |||
| 1466 | + | ||||
| 1467 | + | return nil | |||
| 1289 | } | 1468 | } | ||
| 1290 | 1469 | ||||
| 1291 | // ensureGUIInPane lazily loads GUI components into a pane when it gains focus | 1470 | // ensureGUIInPane lazily loads GUI components into a pane when it gains focus | ||
M internal/app/browser/workspace_manager_tree_test.go +12 -3
| @@ -389,7 +389,10 @@ func TestErrorHandling(t *testing.T) { | |||
| 389 | 389 | wm := newTestWorkspaceManagerWithMocksForTree(t) | |
| 390 | 390 | ||
| 391 | 391 | // Create a branch node | |
| 392 | - | wm.splitNode(wm.root, "right") | |
| 392 | + | _, err := wm.splitNode(wm.root, "right") | |
| 393 | + | if err != nil { | |
| 394 | + | t.Fatalf("Failed to create branch node: %v", err) | |
| 395 | + | } | |
| 393 | 396 | ||
| 394 | 397 | // Try to split the branch (root is now a branch) | |
| 395 | 398 | _, err := wm.splitNode(wm.root, "down") | |
| @@ -402,7 +405,10 @@ func TestErrorHandling(t *testing.T) { | |||
| 402 | 405 | t.Run("Cannot close non-leaf", func(t *testing.T) { | |
| 403 | 406 | wm := newTestWorkspaceManagerWithMocksForTree(t) | |
| 404 | 407 | ||
| 405 | - | wm.splitNode(wm.root, "right") | |
| 408 | + | _, err := wm.splitNode(wm.root, "right") | |
| 409 | + | if err != nil { | |
| 410 | + | t.Fatalf("Failed to create branch node: %v", err) | |
| 411 | + | } | |
| 406 | 412 | ||
| 407 | 413 | // Try to close the branch node | |
| 408 | 414 | err := wm.closePane(wm.root) | |
| @@ -717,7 +723,10 @@ func TestViewToNodeMapping(t *testing.T) { | |||
| 717 | 723 | } | |
| 718 | 724 | ||
| 719 | 725 | // Close should remove mapping | |
| 720 | - | wm.closePane(newNode) | |
| 726 | + | err := wm.closePane(newNode) | |
| 727 | + | if err != nil { | |
| 728 | + | t.Fatalf("Failed to close pane: %v", err) | |
| 729 | + | } | |
| 721 | 730 | ||
| 722 | 731 | if _, ok := wm.viewToNode[newView]; ok { | |
| 723 | 732 | t.Error("Closed pane's webview should be removed from mapping") | |
| @@ -389,7 +389,10 @@ func TestErrorHandling(t *testing.T) { | |||||
| 389 | wm := newTestWorkspaceManagerWithMocksForTree(t) | 389 | wm := newTestWorkspaceManagerWithMocksForTree(t) | ||
| 390 | 390 | ||||
| 391 | // Create a branch node | 391 | // Create a branch node | ||
| 392 | - | wm.splitNode(wm.root, "right") | 392 | + | _, err := wm.splitNode(wm.root, "right") |
| 393 | + | if err != nil { | |||
| 394 | + | t.Fatalf("Failed to create branch node: %v", err) | |||
| 395 | + | } | |||
| 393 | 396 | ||||
| 394 | // Try to split the branch (root is now a branch) | 397 | // Try to split the branch (root is now a branch) | ||
| 395 | _, err := wm.splitNode(wm.root, "down") | 398 | _, err := wm.splitNode(wm.root, "down") | ||
| @@ -402,7 +405,10 @@ func TestErrorHandling(t *testing.T) { | |||||
| 402 | t.Run("Cannot close non-leaf", func(t *testing.T) { | 405 | t.Run("Cannot close non-leaf", func(t *testing.T) { | ||
| 403 | wm := newTestWorkspaceManagerWithMocksForTree(t) | 406 | wm := newTestWorkspaceManagerWithMocksForTree(t) | ||
| 404 | 407 | ||||
| 405 | - | wm.splitNode(wm.root, "right") | 408 | + | _, err := wm.splitNode(wm.root, "right") |
| 409 | + | if err != nil { | |||
| 410 | + | t.Fatalf("Failed to create branch node: %v", err) | |||
| 411 | + | } | |||
| 406 | 412 | ||||
| 407 | // Try to close the branch node | 413 | // Try to close the branch node | ||
| 408 | err := wm.closePane(wm.root) | 414 | err := wm.closePane(wm.root) | ||
| @@ -717,7 +723,10 @@ func TestViewToNodeMapping(t *testing.T) { | |||||
| 717 | } | 723 | } | ||
| 718 | 724 | ||||
| 719 | // Close should remove mapping | 725 | // Close should remove mapping | ||
| 720 | - | wm.closePane(newNode) | 726 | + | err := wm.closePane(newNode) |
| 727 | + | if err != nil { | |||
| 728 | + | t.Fatalf("Failed to close pane: %v", err) | |||
| 729 | + | } | |||
| 721 | 730 | ||||
| 722 | if _, ok := wm.viewToNode[newView]; ok { | 731 | if _, ok := wm.viewToNode[newView]; ok { | ||
| 723 | t.Error("Closed pane's webview should be removed from mapping") | 732 | t.Error("Closed pane's webview should be removed from mapping") | ||