fix(browser): fix pane close operation in gotk4 migration
Fixed GTK assertion error when closing split panes by properly handling widget unparenting for gotk4 API. Key changes: - Removed incorrect nil checks after WidgetUnparent() in attachRoot() In gotk4, Go object references remain valid after unparent, unlike CGO where C pointers could become invalid - Added manual unparenting of sibling from parent paned before promotion GTK4 auto-unparents for paned operations (SetStartChild/SetEndChild) but window.SetChild() requires widgets to already be unparented - Updated comments to explain gotk4/GTK4 behavior differences This fixes the GTK error: "gtk_window_set_child: assertion 'child == NULL || priv->child == child || gtk_widget_get_parent (child) == NULL' failed" Pane close operations now complete successfully without killing both panes.
- commit
- 58f535ab423075b9d05b2184504c086fdb240bda
- author
- bamen · · 2025-10-10 05:50 UTC
- parent
- a1a9d2d
- signature
- unsigned
1 file · +23 -14
M internal/app/browser/workspace_pane_ops.go +23 -14
| @@ -1051,13 +1051,12 @@ func (wm *WorkspaceManager) attachRoot(root *paneNode) { | |||
| 1051 | 1051 | // No need for manual Ref()/Unref() calls | |
| 1052 | 1052 | ||
| 1053 | 1053 | // Detach from previous container (paned, stack, etc.) before replacing the window child. | |
| 1054 | + | // While GTK4 auto-unparents for paned operations, window.SetChild requires manual unparent | |
| 1054 | 1055 | if parent := webkit.WidgetGetParent(root.container); parent != nil { | |
| 1055 | 1056 | log.Printf("[workspace] unparenting widget %p from parent %p before window attach", root.container, parent) | |
| 1056 | 1057 | webkit.WidgetUnparent(root.container) | |
| 1057 | - | if root.container == nil { | |
| 1058 | - | log.Printf("[workspace] WARNING: widget %p became invalid during unparent; aborting attachRoot", root.container) | |
| 1059 | - | return | |
| 1060 | - | } | |
| 1058 | + | // Note: In gotk4, the Go object reference remains valid after unparent | |
| 1059 | + | // Only the GTK parent relationship is cleared | |
| 1061 | 1060 | } | |
| 1062 | 1061 | ||
| 1063 | 1062 | // Clear any existing window child to avoid GTK warnings when swapping roots. | |
| @@ -1068,11 +1067,7 @@ func (wm *WorkspaceManager) attachRoot(root *paneNode) { | |||
| 1068 | 1067 | webkit.WidgetSetVExpand(root.container, true) | |
| 1069 | 1068 | } | |
| 1070 | 1069 | ||
| 1071 | - | if root.container == nil { | |
| 1072 | - | log.Printf("[workspace] WARNING: widget %p invalid prior to window attach", root.container) | |
| 1073 | - | return | |
| 1074 | - | } | |
| 1075 | - | ||
| 1070 | + | // GTK4 will automatically unparent root.container from its previous parent when we attach it here | |
| 1076 | 1071 | wm.window.SetChild(root.container) | |
| 1077 | 1072 | if root.container != nil { | |
| 1078 | 1073 | webkit.WidgetQueueAllocate(root.container) | |
| @@ -1251,11 +1246,25 @@ func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) { | |||
| 1251 | 1246 | } | |
| 1252 | 1247 | } | |
| 1253 | 1248 | ||
| 1254 | - | // STEP 5: Promote sibling in-place (tree structure) | |
| 1249 | + | // STEP 5: Unparent sibling from parent paned BEFORE promotion | |
| 1250 | + | // The sibling needs to be detached from the parent paned before we can attach it elsewhere | |
| 1251 | + | if sibling.container != nil && parent.container != nil { | |
| 1252 | + | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |
| 1253 | + | if parent.left == sibling { | |
| 1254 | + | parentPaned.SetStartChild(nil) | |
| 1255 | + | } else if parent.right == sibling { | |
| 1256 | + | parentPaned.SetEndChild(nil) | |
| 1257 | + | } | |
| 1258 | + | log.Printf("[workspace] unparented sibling container %p from parent paned %p", sibling.container, parent.container) | |
| 1259 | + | } | |
| 1260 | + | } | |
| 1261 | + | ||
| 1262 | + | // STEP 6: Promote sibling in-place (tree structure) | |
| 1255 | 1263 | wm.promoteSibling(grandparent, parent, sibling) | |
| 1256 | 1264 | ||
| 1257 | - | // STEP 6: GTK handles all reparenting automatically | |
| 1258 | - | // PanedSetStartChild/EndChild and window.SetChild auto-unparent widgets | |
| 1265 | + | // STEP 7: GTK widget reparenting | |
| 1266 | + | // For paned containers: GTK4 PanedSetStartChild/EndChild auto-unparent from current parent | |
| 1267 | + | // For window: we already manually unparented above, so now we can safely attach | |
| 1259 | 1268 | wm.swapContainers(grandparent, sibling) | |
| 1260 | 1269 | ||
| 1261 | 1270 | // Reconnect the promoted sibling's rendering pipeline after GTK reparenting. | |
| @@ -1273,14 +1282,14 @@ func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) { | |||
| 1273 | 1282 | }, sibling) | |
| 1274 | 1283 | } | |
| 1275 | 1284 | ||
| 1276 | - | // STEP 7: Check if grandparent now has only one child (cascade promotion needed) | |
| 1285 | + | // STEP 8: Check if grandparent now has only one child (cascade promotion needed) | |
| 1277 | 1286 | if grandparent != nil && ((grandparent.left == nil) != (grandparent.right == nil)) { | |
| 1278 | 1287 | // Grandparent has exactly one child - it should be promoted too | |
| 1279 | 1288 | log.Printf("[workspace] Grandparent %p now has only one child, cascading promotion", grandparent) | |
| 1280 | 1289 | wm.cascadePromotion(grandparent) | |
| 1281 | 1290 | } | |
| 1282 | 1291 | ||
| 1283 | - | // STEP 8: Cleanup & focus | |
| 1292 | + | // STEP 9: Cleanup & focus | |
| 1284 | 1293 | wm.cleanupPane(node, ctx.Generation()) | |
| 1285 | 1294 | wm.decommissionParent(parent, ctx.Generation()) | |
| 1286 | 1295 | if sibling != nil { | |
| @@ -1051,13 +1051,12 @@ func (wm *WorkspaceManager) attachRoot(root *paneNode) { | |||||
| 1051 | // No need for manual Ref()/Unref() calls | 1051 | // No need for manual Ref()/Unref() calls | ||
| 1052 | 1052 | ||||
| 1053 | // Detach from previous container (paned, stack, etc.) before replacing the window child. | 1053 | // Detach from previous container (paned, stack, etc.) before replacing the window child. | ||
| 1054 | + | // While GTK4 auto-unparents for paned operations, window.SetChild requires manual unparent | |||
| 1054 | if parent := webkit.WidgetGetParent(root.container); parent != nil { | 1055 | if parent := webkit.WidgetGetParent(root.container); parent != nil { | ||
| 1055 | log.Printf("[workspace] unparenting widget %p from parent %p before window attach", root.container, parent) | 1056 | log.Printf("[workspace] unparenting widget %p from parent %p before window attach", root.container, parent) | ||
| 1056 | webkit.WidgetUnparent(root.container) | 1057 | webkit.WidgetUnparent(root.container) | ||
| 1057 | - | if root.container == nil { | 1058 | + | // Note: In gotk4, the Go object reference remains valid after unparent |
| 1058 | - | log.Printf("[workspace] WARNING: widget %p became invalid during unparent; aborting attachRoot", root.container) | 1059 | + | // Only the GTK parent relationship is cleared |
| 1059 | - | return | |||
| 1060 | - | } | |||
| 1061 | } | 1060 | } | ||
| 1062 | 1061 | ||||
| 1063 | // Clear any existing window child to avoid GTK warnings when swapping roots. | 1062 | // Clear any existing window child to avoid GTK warnings when swapping roots. | ||
| @@ -1068,11 +1067,7 @@ func (wm *WorkspaceManager) attachRoot(root *paneNode) { | |||||
| 1068 | webkit.WidgetSetVExpand(root.container, true) | 1067 | webkit.WidgetSetVExpand(root.container, true) | ||
| 1069 | } | 1068 | } | ||
| 1070 | 1069 | ||||
| 1071 | - | if root.container == nil { | 1070 | + | // GTK4 will automatically unparent root.container from its previous parent when we attach it here |
| 1072 | - | log.Printf("[workspace] WARNING: widget %p invalid prior to window attach", root.container) | |||
| 1073 | - | return | |||
| 1074 | - | } | |||
| 1075 | - | ||||
| 1076 | wm.window.SetChild(root.container) | 1071 | wm.window.SetChild(root.container) | ||
| 1077 | if root.container != nil { | 1072 | if root.container != nil { | ||
| 1078 | webkit.WidgetQueueAllocate(root.container) | 1073 | webkit.WidgetQueueAllocate(root.container) | ||
| @@ -1251,11 +1246,25 @@ func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) { | |||||
| 1251 | } | 1246 | } | ||
| 1252 | } | 1247 | } | ||
| 1253 | 1248 | ||||
| 1254 | - | // STEP 5: Promote sibling in-place (tree structure) | 1249 | + | // STEP 5: Unparent sibling from parent paned BEFORE promotion |
| 1250 | + | // The sibling needs to be detached from the parent paned before we can attach it elsewhere | |||
| 1251 | + | if sibling.container != nil && parent.container != nil { | |||
| 1252 | + | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |||
| 1253 | + | if parent.left == sibling { | |||
| 1254 | + | parentPaned.SetStartChild(nil) | |||
| 1255 | + | } else if parent.right == sibling { | |||
| 1256 | + | parentPaned.SetEndChild(nil) | |||
| 1257 | + | } | |||
| 1258 | + | log.Printf("[workspace] unparented sibling container %p from parent paned %p", sibling.container, parent.container) | |||
| 1259 | + | } | |||
| 1260 | + | } | |||
| 1261 | + | ||||
| 1262 | + | // STEP 6: Promote sibling in-place (tree structure) | |||
| 1255 | wm.promoteSibling(grandparent, parent, sibling) | 1263 | wm.promoteSibling(grandparent, parent, sibling) | ||
| 1256 | 1264 | ||||
| 1257 | - | // STEP 6: GTK handles all reparenting automatically | 1265 | + | // STEP 7: GTK widget reparenting |
| 1258 | - | // PanedSetStartChild/EndChild and window.SetChild auto-unparent widgets | 1266 | + | // For paned containers: GTK4 PanedSetStartChild/EndChild auto-unparent from current parent |
| 1267 | + | // For window: we already manually unparented above, so now we can safely attach | |||
| 1259 | wm.swapContainers(grandparent, sibling) | 1268 | wm.swapContainers(grandparent, sibling) | ||
| 1260 | 1269 | ||||
| 1261 | // Reconnect the promoted sibling's rendering pipeline after GTK reparenting. | 1270 | // Reconnect the promoted sibling's rendering pipeline after GTK reparenting. | ||
| @@ -1273,14 +1282,14 @@ func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) { | |||||
| 1273 | }, sibling) | 1282 | }, sibling) | ||
| 1274 | } | 1283 | } | ||
| 1275 | 1284 | ||||
| 1276 | - | // STEP 7: Check if grandparent now has only one child (cascade promotion needed) | 1285 | + | // STEP 8: Check if grandparent now has only one child (cascade promotion needed) |
| 1277 | if grandparent != nil && ((grandparent.left == nil) != (grandparent.right == nil)) { | 1286 | if grandparent != nil && ((grandparent.left == nil) != (grandparent.right == nil)) { | ||
| 1278 | // Grandparent has exactly one child - it should be promoted too | 1287 | // Grandparent has exactly one child - it should be promoted too | ||
| 1279 | log.Printf("[workspace] Grandparent %p now has only one child, cascading promotion", grandparent) | 1288 | log.Printf("[workspace] Grandparent %p now has only one child, cascading promotion", grandparent) | ||
| 1280 | wm.cascadePromotion(grandparent) | 1289 | wm.cascadePromotion(grandparent) | ||
| 1281 | } | 1290 | } | ||
| 1282 | 1291 | ||||
| 1283 | - | // STEP 8: Cleanup & focus | 1292 | + | // STEP 9: Cleanup & focus |
| 1284 | wm.cleanupPane(node, ctx.Generation()) | 1293 | wm.cleanupPane(node, ctx.Generation()) | ||
| 1285 | wm.decommissionParent(parent, ctx.Generation()) | 1294 | wm.decommissionParent(parent, ctx.Generation()) | ||
| 1286 | if sibling != nil { | 1295 | if sibling != nil { | ||