fix(workspace): enhance pane promotion with better widget type detection and timing
- commit
- c5d90b9b0876bff81519f2cc5b2bc6ff8a7e6d1e
- author
- Brice · · 2025-09-28 20:10 UTC
- parent
- e55dc9a
- signature
- unsigned
5 files · +688 -18
A docs/PANE_CLOSE_REFACTORING_PLAN.md +444 -0
| @@ -0,0 +1,444 @@ | |||
| 1 | + | # Detailed Pane Close Refactoring Plan | |
| 2 | + | ||
| 3 | + | ## Problem Analysis Summary | |
| 4 | + | ||
| 5 | + | The current implementation has **THREE CRITICAL BUGS**: | |
| 6 | + | 1. **Multiple Pane Closure**: Sibling identification logic at lines 920-928 incorrectly identifies siblings | |
| 7 | + | 2. **Invalid Widget Promotion**: Lines 1042-1090 operate on destroyed widgets | |
| 8 | + | 3. **Race Conditions**: Widget cleanup happens before promotion completes | |
| 9 | + | ||
| 10 | + | ## PHASE 1: New Simplified Architecture | |
| 11 | + | ||
| 12 | + | ### 1.1 Create New File Structure | |
| 13 | + | ``` | |
| 14 | + | workspace_pane_close_v2.go # New simplified close logic | |
| 15 | + | workspace_pane_close_legacy.go # Rename current implementation | |
| 16 | + | workspace_pane_close_test.go # Comprehensive tests | |
| 17 | + | ``` | |
| 18 | + | ||
| 19 | + | ### 1.2 Core Data Structure Changes | |
| 20 | + | ```go | |
| 21 | + | // Add to paneNode struct | |
| 22 | + | type paneNode struct { | |
| 23 | + | // ... existing fields ... | |
| 24 | + | ||
| 25 | + | // New field to track widget validity | |
| 26 | + | widgetValid bool // Set to false when widget destroyed | |
| 27 | + | } | |
| 28 | + | ``` | |
| 29 | + | ||
| 30 | + | ## PHASE 2: Detailed New closePane Implementation | |
| 31 | + | ||
| 32 | + | ### 2.1 Main Close Function (50-70 lines total) | |
| 33 | + | ```go | |
| 34 | + | func (wm *WorkspaceManager) closePaneV2(node *paneNode) (*paneNode, error) { | |
| 35 | + | // STEP 1: Basic validation (5 lines) | |
| 36 | + | if node == nil || !node.isLeaf { | |
| 37 | + | return nil, errors.New("invalid close target") | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | // STEP 2: Handle special cases (15 lines) | |
| 41 | + | // 2a. Stacked pane | |
| 42 | + | if node.parent != nil && node.parent.isStacked { | |
| 43 | + | return wm.closeStackedPaneSimple(node) | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | // 2b. Last pane | |
| 47 | + | if wm.countPanes() == 1 { | |
| 48 | + | wm.cleanupAndExit(node) | |
| 49 | + | return nil, nil | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | // 2c. Root pane with others | |
| 53 | + | if node == wm.root { | |
| 54 | + | return wm.promoteNewRoot(node) | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | // STEP 3: Standard close - promote sibling (30 lines) | |
| 58 | + | parent := node.parent | |
| 59 | + | sibling := wm.getSibling(node) | |
| 60 | + | grandparent := parent.parent | |
| 61 | + | ||
| 62 | + | // Critical: Update tree structure FIRST | |
| 63 | + | sibling.parent = grandparent | |
| 64 | + | if grandparent != nil { | |
| 65 | + | if grandparent.left == parent { | |
| 66 | + | grandparent.left = sibling | |
| 67 | + | } else { | |
| 68 | + | grandparent.right = sibling | |
| 69 | + | } | |
| 70 | + | } else { | |
| 71 | + | wm.root = sibling | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | // STEP 4: Update GTK widgets (using GTK4 auto-unparenting) | |
| 75 | + | if grandparent != nil && grandparent.container != nil { | |
| 76 | + | grandparent.container.Execute(func(gpPtr uintptr) error { | |
| 77 | + | sibling.container.Execute(func(sibPtr uintptr) error { | |
| 78 | + | // GTK4 automatically unparents old child when setting new | |
| 79 | + | if grandparent.left == sibling { | |
| 80 | + | webkit.PanedSetStartChild(gpPtr, sibPtr) | |
| 81 | + | } else { | |
| 82 | + | webkit.PanedSetEndChild(gpPtr, sibPtr) | |
| 83 | + | } | |
| 84 | + | return nil | |
| 85 | + | }) | |
| 86 | + | return nil | |
| 87 | + | }) | |
| 88 | + | } else if sibling == wm.root { | |
| 89 | + | // Sibling is new root | |
| 90 | + | sibling.container.Execute(func(sibPtr uintptr) error { | |
| 91 | + | wm.window.SetChild(sibPtr) | |
| 92 | + | return nil | |
| 93 | + | }) | |
| 94 | + | } | |
| 95 | + | ||
| 96 | + | // STEP 5: Cleanup (10 lines) | |
| 97 | + | wm.cleanupPane(node) | |
| 98 | + | wm.cleanupPane(parent) // Parent paned is no longer needed | |
| 99 | + | ||
| 100 | + | // STEP 6: Update focus | |
| 101 | + | wm.setFocusToLeaf(sibling) | |
| 102 | + | ||
| 103 | + | return sibling, nil | |
| 104 | + | } | |
| 105 | + | ``` | |
| 106 | + | ||
| 107 | + | ### 2.2 Helper Functions with Exact Implementation | |
| 108 | + | ||
| 109 | + | #### getSibling - CORRECTED LOGIC | |
| 110 | + | ```go | |
| 111 | + | func (wm *WorkspaceManager) getSibling(node *paneNode) *paneNode { | |
| 112 | + | if node.parent == nil { | |
| 113 | + | return nil | |
| 114 | + | } | |
| 115 | + | // FIXED: Simple, clear sibling identification | |
| 116 | + | if node.parent.left == node { | |
| 117 | + | return node.parent.right | |
| 118 | + | } | |
| 119 | + | return node.parent.left | |
| 120 | + | } | |
| 121 | + | ``` | |
| 122 | + | ||
| 123 | + | #### promoteNewRoot - Handle Root Replacement | |
| 124 | + | ```go | |
| 125 | + | func (wm *WorkspaceManager) promoteNewRoot(oldRoot *paneNode) (*paneNode, error) { | |
| 126 | + | // Find first non-root pane by traversing tree | |
| 127 | + | var newRoot *paneNode | |
| 128 | + | ||
| 129 | + | // Try left subtree first | |
| 130 | + | if oldRoot.left != nil { | |
| 131 | + | newRoot = oldRoot.left | |
| 132 | + | } else if oldRoot.right != nil { | |
| 133 | + | newRoot = oldRoot.right | |
| 134 | + | } else { | |
| 135 | + | // No other panes - shouldn't happen | |
| 136 | + | return nil, errors.New("no replacement root found") | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | // Detach new root from its parent | |
| 140 | + | if newRoot.parent != nil { | |
| 141 | + | sibling := wm.getSibling(newRoot) | |
| 142 | + | parent := newRoot.parent | |
| 143 | + | ||
| 144 | + | // Promote sibling to parent's position | |
| 145 | + | if parent.parent != nil { | |
| 146 | + | // Has grandparent - attach sibling to it | |
| 147 | + | grand := parent.parent | |
| 148 | + | sibling.parent = grand | |
| 149 | + | if grand.left == parent { | |
| 150 | + | grand.left = sibling | |
| 151 | + | } else { | |
| 152 | + | grand.right = sibling | |
| 153 | + | } | |
| 154 | + | ||
| 155 | + | // Update GTK widget | |
| 156 | + | grand.container.Execute(func(gPtr uintptr) error { | |
| 157 | + | sibling.container.Execute(func(sPtr uintptr) error { | |
| 158 | + | if grand.left == sibling { | |
| 159 | + | webkit.PanedSetStartChild(gPtr, sPtr) | |
| 160 | + | } else { | |
| 161 | + | webkit.PanedSetEndChild(gPtr, sPtr) | |
| 162 | + | } | |
| 163 | + | return nil | |
| 164 | + | }) | |
| 165 | + | return nil | |
| 166 | + | }) | |
| 167 | + | } | |
| 168 | + | } | |
| 169 | + | ||
| 170 | + | // Make newRoot the root | |
| 171 | + | newRoot.parent = nil | |
| 172 | + | wm.root = newRoot | |
| 173 | + | ||
| 174 | + | // Attach to window | |
| 175 | + | newRoot.container.Execute(func(ptr uintptr) error { | |
| 176 | + | wm.window.SetChild(ptr) | |
| 177 | + | return nil | |
| 178 | + | }) | |
| 179 | + | ||
| 180 | + | // Cleanup old root | |
| 181 | + | wm.cleanupPane(oldRoot) | |
| 182 | + | ||
| 183 | + | return newRoot, nil | |
| 184 | + | } | |
| 185 | + | ``` | |
| 186 | + | ||
| 187 | + | #### cleanupPane - Safe Cleanup | |
| 188 | + | ```go | |
| 189 | + | func (wm *WorkspaceManager) cleanupPane(node *paneNode) { | |
| 190 | + | if node == nil || node.widgetValid == false { | |
| 191 | + | return // Already cleaned | |
| 192 | + | } | |
| 193 | + | ||
| 194 | + | // Mark as invalid immediately | |
| 195 | + | node.widgetValid = false | |
| 196 | + | ||
| 197 | + | // Clean up pane resources | |
| 198 | + | if node.pane != nil { | |
| 199 | + | node.pane.CleanupFromWorkspace(wm) | |
| 200 | + | if node.pane.webView != nil { | |
| 201 | + | node.pane.webView.Destroy() | |
| 202 | + | } | |
| 203 | + | } | |
| 204 | + | ||
| 205 | + | // Invalidate container | |
| 206 | + | if node.container != nil { | |
| 207 | + | node.container.Invalidate() | |
| 208 | + | node.container = nil | |
| 209 | + | } | |
| 210 | + | ||
| 211 | + | // Clear tree pointers | |
| 212 | + | node.parent = nil | |
| 213 | + | node.left = nil | |
| 214 | + | node.right = nil | |
| 215 | + | } | |
| 216 | + | ``` | |
| 217 | + | ||
| 218 | + | ## PHASE 3: Stack Container Simplification | |
| 219 | + | ||
| 220 | + | ### 3.1 Simplified Stack Close | |
| 221 | + | ```go | |
| 222 | + | func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, error) { | |
| 223 | + | stack := node.parent | |
| 224 | + | ||
| 225 | + | // Find index | |
| 226 | + | index := -1 | |
| 227 | + | for i, pane := range stack.stackedPanes { | |
| 228 | + | if pane == node { | |
| 229 | + | index = i | |
| 230 | + | break | |
| 231 | + | } | |
| 232 | + | } | |
| 233 | + | ||
| 234 | + | if index == -1 { | |
| 235 | + | return nil, errors.New("pane not in stack") | |
| 236 | + | } | |
| 237 | + | ||
| 238 | + | // Remove from array | |
| 239 | + | stack.stackedPanes = append( | |
| 240 | + | stack.stackedPanes[:index], | |
| 241 | + | stack.stackedPanes[index+1:]... | |
| 242 | + | ) | |
| 243 | + | ||
| 244 | + | // If only one pane left, unstack it | |
| 245 | + | if len(stack.stackedPanes) == 1 { | |
| 246 | + | remaining := stack.stackedPanes[0] | |
| 247 | + | ||
| 248 | + | // Replace stack with remaining pane in tree | |
| 249 | + | remaining.parent = stack.parent | |
| 250 | + | if stack.parent != nil { | |
| 251 | + | if stack.parent.left == stack { | |
| 252 | + | stack.parent.left = remaining | |
| 253 | + | } else { | |
| 254 | + | stack.parent.right = remaining | |
| 255 | + | } | |
| 256 | + | ||
| 257 | + | // Update GTK widgets | |
| 258 | + | stack.parent.container.Execute(func(pPtr uintptr) error { | |
| 259 | + | remaining.container.Execute(func(rPtr uintptr) error { | |
| 260 | + | if stack.parent.left == remaining { | |
| 261 | + | webkit.PanedSetStartChild(pPtr, rPtr) | |
| 262 | + | } else { | |
| 263 | + | webkit.PanedSetEndChild(pPtr, rPtr) | |
| 264 | + | } | |
| 265 | + | return nil | |
| 266 | + | }) | |
| 267 | + | return nil | |
| 268 | + | }) | |
| 269 | + | } else { | |
| 270 | + | // Stack was root | |
| 271 | + | wm.root = remaining | |
| 272 | + | remaining.container.Execute(func(ptr uintptr) error { | |
| 273 | + | wm.window.SetChild(ptr) | |
| 274 | + | return nil | |
| 275 | + | }) | |
| 276 | + | } | |
| 277 | + | ||
| 278 | + | // Cleanup stack container | |
| 279 | + | wm.cleanupPane(stack) | |
| 280 | + | ||
| 281 | + | return remaining, nil | |
| 282 | + | } | |
| 283 | + | ||
| 284 | + | // Update active index if needed | |
| 285 | + | if stack.activeStackIndex >= len(stack.stackedPanes) { | |
| 286 | + | stack.activeStackIndex = len(stack.stackedPanes) - 1 | |
| 287 | + | } | |
| 288 | + | ||
| 289 | + | // Remove widget from GTK box | |
| 290 | + | stack.container.Execute(func(boxPtr uintptr) error { | |
| 291 | + | node.container.Execute(func(nodePtr uintptr) error { | |
| 292 | + | webkit.BoxRemove(boxPtr, nodePtr) | |
| 293 | + | return nil | |
| 294 | + | }) | |
| 295 | + | return nil | |
| 296 | + | }) | |
| 297 | + | ||
| 298 | + | // Show new active pane | |
| 299 | + | if stack.activeStackIndex >= 0 { | |
| 300 | + | active := stack.stackedPanes[stack.activeStackIndex] | |
| 301 | + | active.container.Execute(func(ptr uintptr) error { | |
| 302 | + | webkit.WidgetShow(ptr) | |
| 303 | + | return nil | |
| 304 | + | }) | |
| 305 | + | } | |
| 306 | + | ||
| 307 | + | // Cleanup closed pane | |
| 308 | + | wm.cleanupPane(node) | |
| 309 | + | ||
| 310 | + | return stack, nil | |
| 311 | + | } | |
| 312 | + | ``` | |
| 313 | + | ||
| 314 | + | ## PHASE 4: GTK4 Widget Lifecycle Rules | |
| 315 | + | ||
| 316 | + | ### 4.1 Key GTK4 Rules to Follow | |
| 317 | + | 1. **Auto-unparenting**: When calling `gtk_paned_set_start_child(paned, new_child)`, GTK4 automatically: | |
| 318 | + | - Unparents the old child (if any) | |
| 319 | + | - Unparents new_child from its current parent (if any) | |
| 320 | + | - Sets new_child as the start child | |
| 321 | + | ||
| 322 | + | 2. **Widget Validity**: Always check `GTK_IS_WIDGET(widget)` before operations | |
| 323 | + | ||
| 324 | + | 3. **Reference Counting**: GTK4 uses reference counting - widgets are destroyed when ref count reaches 0 | |
| 325 | + | ||
| 326 | + | ### 4.2 Safe Widget Operations Pattern | |
| 327 | + | ```go | |
| 328 | + | // NEVER do this: | |
| 329 | + | webkit.WidgetUnparent(widget) // Manual unparent | |
| 330 | + | webkit.PanedSetStartChild(paned, widget) // Then set | |
| 331 | + | ||
| 332 | + | // ALWAYS do this: | |
| 333 | + | webkit.PanedSetStartChild(paned, widget) // GTK handles unparenting | |
| 334 | + | ||
| 335 | + | // For boxes: | |
| 336 | + | webkit.BoxRemove(box, widget) // Removes and unparents | |
| 337 | + | // OR | |
| 338 | + | webkit.BoxAppend(box, widget) // Auto-unparents from old parent | |
| 339 | + | ``` | |
| 340 | + | ||
| 341 | + | ## PHASE 5: Migration Strategy | |
| 342 | + | ||
| 343 | + | ### 5.1 Step-by-Step Migration | |
| 344 | + | 1. **Week 1**: Implement new functions alongside old ones | |
| 345 | + | 2. **Week 2**: Add comprehensive logging to both paths | |
| 346 | + | 3. **Week 3**: A/B test with feature flag | |
| 347 | + | 4. **Week 4**: Full migration after stability proven | |
| 348 | + | ||
| 349 | + | ### 5.2 Testing Scenarios | |
| 350 | + | ```go | |
| 351 | + | func TestClosePaneScenarios(t *testing.T) { | |
| 352 | + | tests := []struct{ | |
| 353 | + | name string | |
| 354 | + | setup func() *paneNode | |
| 355 | + | target string // path to target node | |
| 356 | + | expectPanes int | |
| 357 | + | expectRoot string | |
| 358 | + | }{ | |
| 359 | + | { | |
| 360 | + | name: "close_right_child_simple_split", | |
| 361 | + | // A[B,C] -> close C -> A becomes B | |
| 362 | + | }, | |
| 363 | + | { | |
| 364 | + | name: "close_middle_nested_split", | |
| 365 | + | // A[B[D,E],C] -> close B -> A[sibling(D,E),C] | |
| 366 | + | }, | |
| 367 | + | { | |
| 368 | + | name: "close_in_stack", | |
| 369 | + | // Stack[A,B,C] -> close B -> Stack[A,C] | |
| 370 | + | }, | |
| 371 | + | { | |
| 372 | + | name: "close_last_in_stack", | |
| 373 | + | // A[Stack[B,C],D] -> close C -> A[B,D] | |
| 374 | + | }, | |
| 375 | + | } | |
| 376 | + | } | |
| 377 | + | ``` | |
| 378 | + | ||
| 379 | + | ## PHASE 6: Validation & Safety | |
| 380 | + | ||
| 381 | + | ### 6.1 Add Assertions | |
| 382 | + | ```go | |
| 383 | + | // Add to every widget operation: | |
| 384 | + | if !webkit.WidgetIsValid(ptr) { | |
| 385 | + | return fmt.Errorf("invalid widget %#x", ptr) | |
| 386 | + | } | |
| 387 | + | ||
| 388 | + | // Before tree operations: | |
| 389 | + | if err := wm.validateTreeConsistency(); err != nil { | |
| 390 | + | return nil, fmt.Errorf("tree inconsistent before close: %w", err) | |
| 391 | + | } | |
| 392 | + | ``` | |
| 393 | + | ||
| 394 | + | ### 6.2 Comprehensive Logging | |
| 395 | + | ```go | |
| 396 | + | // Before operation: | |
| 397 | + | log.Printf("[CLOSE] Starting: node=%p parent=%p sibling=%p", | |
| 398 | + | node, node.parent, sibling) | |
| 399 | + | ||
| 400 | + | // After tree update: | |
| 401 | + | log.Printf("[CLOSE] Tree updated: new_root=%p promoted=%p", | |
| 402 | + | wm.root, promoted) | |
| 403 | + | ||
| 404 | + | // After GTK update: | |
| 405 | + | log.Printf("[CLOSE] GTK updated: widget=%#x parent=%#x", | |
| 406 | + | widgetPtr, parentPtr) | |
| 407 | + | ``` | |
| 408 | + | ||
| 409 | + | ## Expected Results | |
| 410 | + | ||
| 411 | + | ### Before (Current Issues): | |
| 412 | + | - 400+ lines of complex code | |
| 413 | + | - Segfaults on complex layouts | |
| 414 | + | - Multiple panes closed incorrectly | |
| 415 | + | - Widget corruption errors | |
| 416 | + | ||
| 417 | + | ### After (New Implementation): | |
| 418 | + | - ~150 lines total | |
| 419 | + | - Clean GTK4 lifecycle adherence | |
| 420 | + | - Predictable single-pane closure | |
| 421 | + | - No widget corruption | |
| 422 | + | ||
| 423 | + | ## Implementation Order | |
| 424 | + | 1. Implement `closePaneV2` function | |
| 425 | + | 2. Implement helper functions | |
| 426 | + | 3. Add comprehensive logging | |
| 427 | + | 4. Create test suite | |
| 428 | + | 5. Run side-by-side with flag | |
| 429 | + | 6. Migrate after proven stable | |
| 430 | + | ||
| 431 | + | ## Key Files to Modify | |
| 432 | + | ||
| 433 | + | 1. `workspace_types.go:8` - Add `widgetValid bool` field to `paneNode` | |
| 434 | + | 2. `workspace_pane_ops.go:707` - Current `closePane` function (rename to `closePaneLegacy`) | |
| 435 | + | 3. **NEW** `workspace_pane_close_v2.go` - Simplified implementation | |
| 436 | + | 4. **NEW** `workspace_pane_close_test.go` - Comprehensive test suite | |
| 437 | + | 5. `stacked_panes.go` - Simplify `CloseStackedPane` function | |
| 438 | + | ||
| 439 | + | ## Critical Success Metrics | |
| 440 | + | ||
| 441 | + | 1. **Stability**: No segfaults in any close scenario | |
| 442 | + | 2. **Correctness**: Only requested pane closes (never multiple) | |
| 443 | + | 3. **Performance**: Close operation completes in <50ms | |
| 444 | + | 4. **Maintainability**: Code is <200 lines and easily understood⏎̸ | |
| @@ -0,0 +1,444 @@ | |||||
| 1 | + | # Detailed Pane Close Refactoring Plan | |||
| 2 | + | ||||
| 3 | + | ## Problem Analysis Summary | |||
| 4 | + | ||||
| 5 | + | The current implementation has **THREE CRITICAL BUGS**: | |||
| 6 | + | 1. **Multiple Pane Closure**: Sibling identification logic at lines 920-928 incorrectly identifies siblings | |||
| 7 | + | 2. **Invalid Widget Promotion**: Lines 1042-1090 operate on destroyed widgets | |||
| 8 | + | 3. **Race Conditions**: Widget cleanup happens before promotion completes | |||
| 9 | + | ||||
| 10 | + | ## PHASE 1: New Simplified Architecture | |||
| 11 | + | ||||
| 12 | + | ### 1.1 Create New File Structure | |||
| 13 | + | ``` | |||
| 14 | + | workspace_pane_close_v2.go # New simplified close logic | |||
| 15 | + | workspace_pane_close_legacy.go # Rename current implementation | |||
| 16 | + | workspace_pane_close_test.go # Comprehensive tests | |||
| 17 | + | ``` | |||
| 18 | + | ||||
| 19 | + | ### 1.2 Core Data Structure Changes | |||
| 20 | + | ```go | |||
| 21 | + | // Add to paneNode struct | |||
| 22 | + | type paneNode struct { | |||
| 23 | + | // ... existing fields ... | |||
| 24 | + | ||||
| 25 | + | // New field to track widget validity | |||
| 26 | + | widgetValid bool // Set to false when widget destroyed | |||
| 27 | + | } | |||
| 28 | + | ``` | |||
| 29 | + | ||||
| 30 | + | ## PHASE 2: Detailed New closePane Implementation | |||
| 31 | + | ||||
| 32 | + | ### 2.1 Main Close Function (50-70 lines total) | |||
| 33 | + | ```go | |||
| 34 | + | func (wm *WorkspaceManager) closePaneV2(node *paneNode) (*paneNode, error) { | |||
| 35 | + | // STEP 1: Basic validation (5 lines) | |||
| 36 | + | if node == nil || !node.isLeaf { | |||
| 37 | + | return nil, errors.New("invalid close target") | |||
| 38 | + | } | |||
| 39 | + | ||||
| 40 | + | // STEP 2: Handle special cases (15 lines) | |||
| 41 | + | // 2a. Stacked pane | |||
| 42 | + | if node.parent != nil && node.parent.isStacked { | |||
| 43 | + | return wm.closeStackedPaneSimple(node) | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | // 2b. Last pane | |||
| 47 | + | if wm.countPanes() == 1 { | |||
| 48 | + | wm.cleanupAndExit(node) | |||
| 49 | + | return nil, nil | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | // 2c. Root pane with others | |||
| 53 | + | if node == wm.root { | |||
| 54 | + | return wm.promoteNewRoot(node) | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | // STEP 3: Standard close - promote sibling (30 lines) | |||
| 58 | + | parent := node.parent | |||
| 59 | + | sibling := wm.getSibling(node) | |||
| 60 | + | grandparent := parent.parent | |||
| 61 | + | ||||
| 62 | + | // Critical: Update tree structure FIRST | |||
| 63 | + | sibling.parent = grandparent | |||
| 64 | + | if grandparent != nil { | |||
| 65 | + | if grandparent.left == parent { | |||
| 66 | + | grandparent.left = sibling | |||
| 67 | + | } else { | |||
| 68 | + | grandparent.right = sibling | |||
| 69 | + | } | |||
| 70 | + | } else { | |||
| 71 | + | wm.root = sibling | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | // STEP 4: Update GTK widgets (using GTK4 auto-unparenting) | |||
| 75 | + | if grandparent != nil && grandparent.container != nil { | |||
| 76 | + | grandparent.container.Execute(func(gpPtr uintptr) error { | |||
| 77 | + | sibling.container.Execute(func(sibPtr uintptr) error { | |||
| 78 | + | // GTK4 automatically unparents old child when setting new | |||
| 79 | + | if grandparent.left == sibling { | |||
| 80 | + | webkit.PanedSetStartChild(gpPtr, sibPtr) | |||
| 81 | + | } else { | |||
| 82 | + | webkit.PanedSetEndChild(gpPtr, sibPtr) | |||
| 83 | + | } | |||
| 84 | + | return nil | |||
| 85 | + | }) | |||
| 86 | + | return nil | |||
| 87 | + | }) | |||
| 88 | + | } else if sibling == wm.root { | |||
| 89 | + | // Sibling is new root | |||
| 90 | + | sibling.container.Execute(func(sibPtr uintptr) error { | |||
| 91 | + | wm.window.SetChild(sibPtr) | |||
| 92 | + | return nil | |||
| 93 | + | }) | |||
| 94 | + | } | |||
| 95 | + | ||||
| 96 | + | // STEP 5: Cleanup (10 lines) | |||
| 97 | + | wm.cleanupPane(node) | |||
| 98 | + | wm.cleanupPane(parent) // Parent paned is no longer needed | |||
| 99 | + | ||||
| 100 | + | // STEP 6: Update focus | |||
| 101 | + | wm.setFocusToLeaf(sibling) | |||
| 102 | + | ||||
| 103 | + | return sibling, nil | |||
| 104 | + | } | |||
| 105 | + | ``` | |||
| 106 | + | ||||
| 107 | + | ### 2.2 Helper Functions with Exact Implementation | |||
| 108 | + | ||||
| 109 | + | #### getSibling - CORRECTED LOGIC | |||
| 110 | + | ```go | |||
| 111 | + | func (wm *WorkspaceManager) getSibling(node *paneNode) *paneNode { | |||
| 112 | + | if node.parent == nil { | |||
| 113 | + | return nil | |||
| 114 | + | } | |||
| 115 | + | // FIXED: Simple, clear sibling identification | |||
| 116 | + | if node.parent.left == node { | |||
| 117 | + | return node.parent.right | |||
| 118 | + | } | |||
| 119 | + | return node.parent.left | |||
| 120 | + | } | |||
| 121 | + | ``` | |||
| 122 | + | ||||
| 123 | + | #### promoteNewRoot - Handle Root Replacement | |||
| 124 | + | ```go | |||
| 125 | + | func (wm *WorkspaceManager) promoteNewRoot(oldRoot *paneNode) (*paneNode, error) { | |||
| 126 | + | // Find first non-root pane by traversing tree | |||
| 127 | + | var newRoot *paneNode | |||
| 128 | + | ||||
| 129 | + | // Try left subtree first | |||
| 130 | + | if oldRoot.left != nil { | |||
| 131 | + | newRoot = oldRoot.left | |||
| 132 | + | } else if oldRoot.right != nil { | |||
| 133 | + | newRoot = oldRoot.right | |||
| 134 | + | } else { | |||
| 135 | + | // No other panes - shouldn't happen | |||
| 136 | + | return nil, errors.New("no replacement root found") | |||
| 137 | + | } | |||
| 138 | + | ||||
| 139 | + | // Detach new root from its parent | |||
| 140 | + | if newRoot.parent != nil { | |||
| 141 | + | sibling := wm.getSibling(newRoot) | |||
| 142 | + | parent := newRoot.parent | |||
| 143 | + | ||||
| 144 | + | // Promote sibling to parent's position | |||
| 145 | + | if parent.parent != nil { | |||
| 146 | + | // Has grandparent - attach sibling to it | |||
| 147 | + | grand := parent.parent | |||
| 148 | + | sibling.parent = grand | |||
| 149 | + | if grand.left == parent { | |||
| 150 | + | grand.left = sibling | |||
| 151 | + | } else { | |||
| 152 | + | grand.right = sibling | |||
| 153 | + | } | |||
| 154 | + | ||||
| 155 | + | // Update GTK widget | |||
| 156 | + | grand.container.Execute(func(gPtr uintptr) error { | |||
| 157 | + | sibling.container.Execute(func(sPtr uintptr) error { | |||
| 158 | + | if grand.left == sibling { | |||
| 159 | + | webkit.PanedSetStartChild(gPtr, sPtr) | |||
| 160 | + | } else { | |||
| 161 | + | webkit.PanedSetEndChild(gPtr, sPtr) | |||
| 162 | + | } | |||
| 163 | + | return nil | |||
| 164 | + | }) | |||
| 165 | + | return nil | |||
| 166 | + | }) | |||
| 167 | + | } | |||
| 168 | + | } | |||
| 169 | + | ||||
| 170 | + | // Make newRoot the root | |||
| 171 | + | newRoot.parent = nil | |||
| 172 | + | wm.root = newRoot | |||
| 173 | + | ||||
| 174 | + | // Attach to window | |||
| 175 | + | newRoot.container.Execute(func(ptr uintptr) error { | |||
| 176 | + | wm.window.SetChild(ptr) | |||
| 177 | + | return nil | |||
| 178 | + | }) | |||
| 179 | + | ||||
| 180 | + | // Cleanup old root | |||
| 181 | + | wm.cleanupPane(oldRoot) | |||
| 182 | + | ||||
| 183 | + | return newRoot, nil | |||
| 184 | + | } | |||
| 185 | + | ``` | |||
| 186 | + | ||||
| 187 | + | #### cleanupPane - Safe Cleanup | |||
| 188 | + | ```go | |||
| 189 | + | func (wm *WorkspaceManager) cleanupPane(node *paneNode) { | |||
| 190 | + | if node == nil || node.widgetValid == false { | |||
| 191 | + | return // Already cleaned | |||
| 192 | + | } | |||
| 193 | + | ||||
| 194 | + | // Mark as invalid immediately | |||
| 195 | + | node.widgetValid = false | |||
| 196 | + | ||||
| 197 | + | // Clean up pane resources | |||
| 198 | + | if node.pane != nil { | |||
| 199 | + | node.pane.CleanupFromWorkspace(wm) | |||
| 200 | + | if node.pane.webView != nil { | |||
| 201 | + | node.pane.webView.Destroy() | |||
| 202 | + | } | |||
| 203 | + | } | |||
| 204 | + | ||||
| 205 | + | // Invalidate container | |||
| 206 | + | if node.container != nil { | |||
| 207 | + | node.container.Invalidate() | |||
| 208 | + | node.container = nil | |||
| 209 | + | } | |||
| 210 | + | ||||
| 211 | + | // Clear tree pointers | |||
| 212 | + | node.parent = nil | |||
| 213 | + | node.left = nil | |||
| 214 | + | node.right = nil | |||
| 215 | + | } | |||
| 216 | + | ``` | |||
| 217 | + | ||||
| 218 | + | ## PHASE 3: Stack Container Simplification | |||
| 219 | + | ||||
| 220 | + | ### 3.1 Simplified Stack Close | |||
| 221 | + | ```go | |||
| 222 | + | func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, error) { | |||
| 223 | + | stack := node.parent | |||
| 224 | + | ||||
| 225 | + | // Find index | |||
| 226 | + | index := -1 | |||
| 227 | + | for i, pane := range stack.stackedPanes { | |||
| 228 | + | if pane == node { | |||
| 229 | + | index = i | |||
| 230 | + | break | |||
| 231 | + | } | |||
| 232 | + | } | |||
| 233 | + | ||||
| 234 | + | if index == -1 { | |||
| 235 | + | return nil, errors.New("pane not in stack") | |||
| 236 | + | } | |||
| 237 | + | ||||
| 238 | + | // Remove from array | |||
| 239 | + | stack.stackedPanes = append( | |||
| 240 | + | stack.stackedPanes[:index], | |||
| 241 | + | stack.stackedPanes[index+1:]... | |||
| 242 | + | ) | |||
| 243 | + | ||||
| 244 | + | // If only one pane left, unstack it | |||
| 245 | + | if len(stack.stackedPanes) == 1 { | |||
| 246 | + | remaining := stack.stackedPanes[0] | |||
| 247 | + | ||||
| 248 | + | // Replace stack with remaining pane in tree | |||
| 249 | + | remaining.parent = stack.parent | |||
| 250 | + | if stack.parent != nil { | |||
| 251 | + | if stack.parent.left == stack { | |||
| 252 | + | stack.parent.left = remaining | |||
| 253 | + | } else { | |||
| 254 | + | stack.parent.right = remaining | |||
| 255 | + | } | |||
| 256 | + | ||||
| 257 | + | // Update GTK widgets | |||
| 258 | + | stack.parent.container.Execute(func(pPtr uintptr) error { | |||
| 259 | + | remaining.container.Execute(func(rPtr uintptr) error { | |||
| 260 | + | if stack.parent.left == remaining { | |||
| 261 | + | webkit.PanedSetStartChild(pPtr, rPtr) | |||
| 262 | + | } else { | |||
| 263 | + | webkit.PanedSetEndChild(pPtr, rPtr) | |||
| 264 | + | } | |||
| 265 | + | return nil | |||
| 266 | + | }) | |||
| 267 | + | return nil | |||
| 268 | + | }) | |||
| 269 | + | } else { | |||
| 270 | + | // Stack was root | |||
| 271 | + | wm.root = remaining | |||
| 272 | + | remaining.container.Execute(func(ptr uintptr) error { | |||
| 273 | + | wm.window.SetChild(ptr) | |||
| 274 | + | return nil | |||
| 275 | + | }) | |||
| 276 | + | } | |||
| 277 | + | ||||
| 278 | + | // Cleanup stack container | |||
| 279 | + | wm.cleanupPane(stack) | |||
| 280 | + | ||||
| 281 | + | return remaining, nil | |||
| 282 | + | } | |||
| 283 | + | ||||
| 284 | + | // Update active index if needed | |||
| 285 | + | if stack.activeStackIndex >= len(stack.stackedPanes) { | |||
| 286 | + | stack.activeStackIndex = len(stack.stackedPanes) - 1 | |||
| 287 | + | } | |||
| 288 | + | ||||
| 289 | + | // Remove widget from GTK box | |||
| 290 | + | stack.container.Execute(func(boxPtr uintptr) error { | |||
| 291 | + | node.container.Execute(func(nodePtr uintptr) error { | |||
| 292 | + | webkit.BoxRemove(boxPtr, nodePtr) | |||
| 293 | + | return nil | |||
| 294 | + | }) | |||
| 295 | + | return nil | |||
| 296 | + | }) | |||
| 297 | + | ||||
| 298 | + | // Show new active pane | |||
| 299 | + | if stack.activeStackIndex >= 0 { | |||
| 300 | + | active := stack.stackedPanes[stack.activeStackIndex] | |||
| 301 | + | active.container.Execute(func(ptr uintptr) error { | |||
| 302 | + | webkit.WidgetShow(ptr) | |||
| 303 | + | return nil | |||
| 304 | + | }) | |||
| 305 | + | } | |||
| 306 | + | ||||
| 307 | + | // Cleanup closed pane | |||
| 308 | + | wm.cleanupPane(node) | |||
| 309 | + | ||||
| 310 | + | return stack, nil | |||
| 311 | + | } | |||
| 312 | + | ``` | |||
| 313 | + | ||||
| 314 | + | ## PHASE 4: GTK4 Widget Lifecycle Rules | |||
| 315 | + | ||||
| 316 | + | ### 4.1 Key GTK4 Rules to Follow | |||
| 317 | + | 1. **Auto-unparenting**: When calling `gtk_paned_set_start_child(paned, new_child)`, GTK4 automatically: | |||
| 318 | + | - Unparents the old child (if any) | |||
| 319 | + | - Unparents new_child from its current parent (if any) | |||
| 320 | + | - Sets new_child as the start child | |||
| 321 | + | ||||
| 322 | + | 2. **Widget Validity**: Always check `GTK_IS_WIDGET(widget)` before operations | |||
| 323 | + | ||||
| 324 | + | 3. **Reference Counting**: GTK4 uses reference counting - widgets are destroyed when ref count reaches 0 | |||
| 325 | + | ||||
| 326 | + | ### 4.2 Safe Widget Operations Pattern | |||
| 327 | + | ```go | |||
| 328 | + | // NEVER do this: | |||
| 329 | + | webkit.WidgetUnparent(widget) // Manual unparent | |||
| 330 | + | webkit.PanedSetStartChild(paned, widget) // Then set | |||
| 331 | + | ||||
| 332 | + | // ALWAYS do this: | |||
| 333 | + | webkit.PanedSetStartChild(paned, widget) // GTK handles unparenting | |||
| 334 | + | ||||
| 335 | + | // For boxes: | |||
| 336 | + | webkit.BoxRemove(box, widget) // Removes and unparents | |||
| 337 | + | // OR | |||
| 338 | + | webkit.BoxAppend(box, widget) // Auto-unparents from old parent | |||
| 339 | + | ``` | |||
| 340 | + | ||||
| 341 | + | ## PHASE 5: Migration Strategy | |||
| 342 | + | ||||
| 343 | + | ### 5.1 Step-by-Step Migration | |||
| 344 | + | 1. **Week 1**: Implement new functions alongside old ones | |||
| 345 | + | 2. **Week 2**: Add comprehensive logging to both paths | |||
| 346 | + | 3. **Week 3**: A/B test with feature flag | |||
| 347 | + | 4. **Week 4**: Full migration after stability proven | |||
| 348 | + | ||||
| 349 | + | ### 5.2 Testing Scenarios | |||
| 350 | + | ```go | |||
| 351 | + | func TestClosePaneScenarios(t *testing.T) { | |||
| 352 | + | tests := []struct{ | |||
| 353 | + | name string | |||
| 354 | + | setup func() *paneNode | |||
| 355 | + | target string // path to target node | |||
| 356 | + | expectPanes int | |||
| 357 | + | expectRoot string | |||
| 358 | + | }{ | |||
| 359 | + | { | |||
| 360 | + | name: "close_right_child_simple_split", | |||
| 361 | + | // A[B,C] -> close C -> A becomes B | |||
| 362 | + | }, | |||
| 363 | + | { | |||
| 364 | + | name: "close_middle_nested_split", | |||
| 365 | + | // A[B[D,E],C] -> close B -> A[sibling(D,E),C] | |||
| 366 | + | }, | |||
| 367 | + | { | |||
| 368 | + | name: "close_in_stack", | |||
| 369 | + | // Stack[A,B,C] -> close B -> Stack[A,C] | |||
| 370 | + | }, | |||
| 371 | + | { | |||
| 372 | + | name: "close_last_in_stack", | |||
| 373 | + | // A[Stack[B,C],D] -> close C -> A[B,D] | |||
| 374 | + | }, | |||
| 375 | + | } | |||
| 376 | + | } | |||
| 377 | + | ``` | |||
| 378 | + | ||||
| 379 | + | ## PHASE 6: Validation & Safety | |||
| 380 | + | ||||
| 381 | + | ### 6.1 Add Assertions | |||
| 382 | + | ```go | |||
| 383 | + | // Add to every widget operation: | |||
| 384 | + | if !webkit.WidgetIsValid(ptr) { | |||
| 385 | + | return fmt.Errorf("invalid widget %#x", ptr) | |||
| 386 | + | } | |||
| 387 | + | ||||
| 388 | + | // Before tree operations: | |||
| 389 | + | if err := wm.validateTreeConsistency(); err != nil { | |||
| 390 | + | return nil, fmt.Errorf("tree inconsistent before close: %w", err) | |||
| 391 | + | } | |||
| 392 | + | ``` | |||
| 393 | + | ||||
| 394 | + | ### 6.2 Comprehensive Logging | |||
| 395 | + | ```go | |||
| 396 | + | // Before operation: | |||
| 397 | + | log.Printf("[CLOSE] Starting: node=%p parent=%p sibling=%p", | |||
| 398 | + | node, node.parent, sibling) | |||
| 399 | + | ||||
| 400 | + | // After tree update: | |||
| 401 | + | log.Printf("[CLOSE] Tree updated: new_root=%p promoted=%p", | |||
| 402 | + | wm.root, promoted) | |||
| 403 | + | ||||
| 404 | + | // After GTK update: | |||
| 405 | + | log.Printf("[CLOSE] GTK updated: widget=%#x parent=%#x", | |||
| 406 | + | widgetPtr, parentPtr) | |||
| 407 | + | ``` | |||
| 408 | + | ||||
| 409 | + | ## Expected Results | |||
| 410 | + | ||||
| 411 | + | ### Before (Current Issues): | |||
| 412 | + | - 400+ lines of complex code | |||
| 413 | + | - Segfaults on complex layouts | |||
| 414 | + | - Multiple panes closed incorrectly | |||
| 415 | + | - Widget corruption errors | |||
| 416 | + | ||||
| 417 | + | ### After (New Implementation): | |||
| 418 | + | - ~150 lines total | |||
| 419 | + | - Clean GTK4 lifecycle adherence | |||
| 420 | + | - Predictable single-pane closure | |||
| 421 | + | - No widget corruption | |||
| 422 | + | ||||
| 423 | + | ## Implementation Order | |||
| 424 | + | 1. Implement `closePaneV2` function | |||
| 425 | + | 2. Implement helper functions | |||
| 426 | + | 3. Add comprehensive logging | |||
| 427 | + | 4. Create test suite | |||
| 428 | + | 5. Run side-by-side with flag | |||
| 429 | + | 6. Migrate after proven stable | |||
| 430 | + | ||||
| 431 | + | ## Key Files to Modify | |||
| 432 | + | ||||
| 433 | + | 1. `workspace_types.go:8` - Add `widgetValid bool` field to `paneNode` | |||
| 434 | + | 2. `workspace_pane_ops.go:707` - Current `closePane` function (rename to `closePaneLegacy`) | |||
| 435 | + | 3. **NEW** `workspace_pane_close_v2.go` - Simplified implementation | |||
| 436 | + | 4. **NEW** `workspace_pane_close_test.go` - Comprehensive test suite | |||
| 437 | + | 5. `stacked_panes.go` - Simplify `CloseStackedPane` function | |||
| 438 | + | ||||
| 439 | + | ## Critical Success Metrics | |||
| 440 | + | ||||
| 441 | + | 1. **Stability**: No segfaults in any close scenario | |||
| 442 | + | 2. **Correctness**: Only requested pane closes (never multiple) | |||
| 443 | + | 3. **Performance**: Close operation completes in <50ms | |||
| 444 | + | 4. **Maintainability**: Code is <200 lines and easily understood⏎̸ | |||
M internal/app/browser/workspace_pane_ops.go +26 -8
| @@ -1010,32 +1010,50 @@ func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) { | |||
| 1010 | 1010 | wm.root = sibling | |
| 1011 | 1011 | sibling.parent = nil // Critical: clear parent pointer for new root | |
| 1012 | 1012 | ||
| 1013 | - | // For root promotion, must unparent first to satisfy GTK4 assertion | |
| 1013 | + | // For root promotion, handle the widget transition carefully | |
| 1014 | 1014 | if wm.window != nil && sibling.container != nil { | |
| 1015 | 1015 | sibling.container.Execute(func(containerPtr uintptr) error { | |
| 1016 | 1016 | parentBefore := webkit.WidgetGetParent(containerPtr) | |
| 1017 | 1017 | log.Printf("[workspace] root promotion: sibling parent before=%#x", parentBefore) | |
| 1018 | 1018 | ||
| 1019 | - | if parentBefore == 0 { | |
| 1020 | - | log.Printf("[workspace] root promotion: sibling already detached") | |
| 1021 | - | } else { | |
| 1019 | + | // Only unparent if it has a parent (the paned) | |
| 1020 | + | if parentBefore != 0 { | |
| 1021 | + | // Get info about the paned before we destroy it | |
| 1022 | 1022 | oldParentStart := webkit.PanedGetStartChild(parentBefore) | |
| 1023 | 1023 | oldParentEnd := webkit.PanedGetEndChild(parentBefore) | |
| 1024 | 1024 | log.Printf("[workspace] root promotion: old parent start=%#x end=%#x", oldParentStart, oldParentEnd) | |
| 1025 | - | } | |
| 1026 | 1025 | ||
| 1027 | - | // Unparent from current parent before setting as window child | |
| 1028 | - | if webkit.WidgetGetParent(containerPtr) != 0 { | |
| 1026 | + | // Unparent the sibling from the paned | |
| 1029 | 1027 | webkit.WidgetUnparent(containerPtr) | |
| 1028 | + | ||
| 1029 | + | // Ensure the widget is properly configured for window child | |
| 1030 | + | webkit.WidgetSetHExpand(containerPtr, true) | |
| 1031 | + | webkit.WidgetSetVExpand(containerPtr, true) | |
| 1030 | 1032 | } | |
| 1031 | 1033 | ||
| 1034 | + | // Set as window child | |
| 1032 | 1035 | wm.window.SetChild(containerPtr) | |
| 1036 | + | ||
| 1037 | + | // Queue draw and allocation to ensure proper rendering | |
| 1033 | 1038 | webkit.WidgetQueueAllocate(containerPtr) | |
| 1034 | 1039 | webkit.WidgetShow(containerPtr) | |
| 1035 | 1040 | ||
| 1041 | + | // Verify attachment succeeded | |
| 1036 | 1042 | parentAfter := webkit.WidgetGetParent(containerPtr) | |
| 1043 | + | if parentAfter == 0 { | |
| 1044 | + | log.Printf("[workspace] WARNING: root promotion attachment failed, widget has no parent") | |
| 1045 | + | // Force another attempt at setting the child | |
| 1046 | + | wm.window.SetChild(containerPtr) | |
| 1047 | + | webkit.WidgetQueueAllocate(containerPtr) | |
| 1048 | + | parentAfter = webkit.WidgetGetParent(containerPtr) | |
| 1049 | + | } | |
| 1050 | + | ||
| 1037 | 1051 | log.Printf("[workspace] root promotion: sibling parent after=%#x", parentAfter) | |
| 1038 | - | log.Printf("[workspace] successfully promoted sibling to root: %#x", containerPtr) | |
| 1052 | + | if parentAfter != 0 { | |
| 1053 | + | log.Printf("[workspace] successfully promoted sibling to root: %#x", containerPtr) | |
| 1054 | + | } else { | |
| 1055 | + | log.Printf("[workspace] ERROR: failed to attach promoted sibling as window child: %#x", containerPtr) | |
| 1056 | + | } | |
| 1039 | 1057 | return nil | |
| 1040 | 1058 | }) | |
| 1041 | 1059 | } | |
| @@ -1010,32 +1010,50 @@ func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) { | |||||
| 1010 | wm.root = sibling | 1010 | wm.root = sibling | ||
| 1011 | sibling.parent = nil // Critical: clear parent pointer for new root | 1011 | sibling.parent = nil // Critical: clear parent pointer for new root | ||
| 1012 | 1012 | ||||
| 1013 | - | // For root promotion, must unparent first to satisfy GTK4 assertion | 1013 | + | // For root promotion, handle the widget transition carefully |
| 1014 | if wm.window != nil && sibling.container != nil { | 1014 | if wm.window != nil && sibling.container != nil { | ||
| 1015 | sibling.container.Execute(func(containerPtr uintptr) error { | 1015 | sibling.container.Execute(func(containerPtr uintptr) error { | ||
| 1016 | parentBefore := webkit.WidgetGetParent(containerPtr) | 1016 | parentBefore := webkit.WidgetGetParent(containerPtr) | ||
| 1017 | log.Printf("[workspace] root promotion: sibling parent before=%#x", parentBefore) | 1017 | log.Printf("[workspace] root promotion: sibling parent before=%#x", parentBefore) | ||
| 1018 | 1018 | ||||
| 1019 | - | if parentBefore == 0 { | 1019 | + | // Only unparent if it has a parent (the paned) |
| 1020 | - | log.Printf("[workspace] root promotion: sibling already detached") | 1020 | + | if parentBefore != 0 { |
| 1021 | - | } else { | 1021 | + | // Get info about the paned before we destroy it |
| 1022 | oldParentStart := webkit.PanedGetStartChild(parentBefore) | 1022 | oldParentStart := webkit.PanedGetStartChild(parentBefore) | ||
| 1023 | oldParentEnd := webkit.PanedGetEndChild(parentBefore) | 1023 | oldParentEnd := webkit.PanedGetEndChild(parentBefore) | ||
| 1024 | log.Printf("[workspace] root promotion: old parent start=%#x end=%#x", oldParentStart, oldParentEnd) | 1024 | log.Printf("[workspace] root promotion: old parent start=%#x end=%#x", oldParentStart, oldParentEnd) | ||
| 1025 | - | } | |||
| 1026 | 1025 | ||||
| 1027 | - | // Unparent from current parent before setting as window child | 1026 | + | // Unparent the sibling from the paned |
| 1028 | - | if webkit.WidgetGetParent(containerPtr) != 0 { | |||
| 1029 | webkit.WidgetUnparent(containerPtr) | 1027 | webkit.WidgetUnparent(containerPtr) | ||
| 1028 | + | ||||
| 1029 | + | // Ensure the widget is properly configured for window child | |||
| 1030 | + | webkit.WidgetSetHExpand(containerPtr, true) | |||
| 1031 | + | webkit.WidgetSetVExpand(containerPtr, true) | |||
| 1030 | } | 1032 | } | ||
| 1031 | 1033 | ||||
| 1034 | + | // Set as window child | |||
| 1032 | wm.window.SetChild(containerPtr) | 1035 | wm.window.SetChild(containerPtr) | ||
| 1036 | + | ||||
| 1037 | + | // Queue draw and allocation to ensure proper rendering | |||
| 1033 | webkit.WidgetQueueAllocate(containerPtr) | 1038 | webkit.WidgetQueueAllocate(containerPtr) | ||
| 1034 | webkit.WidgetShow(containerPtr) | 1039 | webkit.WidgetShow(containerPtr) | ||
| 1035 | 1040 | ||||
| 1041 | + | // Verify attachment succeeded | |||
| 1036 | parentAfter := webkit.WidgetGetParent(containerPtr) | 1042 | parentAfter := webkit.WidgetGetParent(containerPtr) | ||
| 1043 | + | if parentAfter == 0 { | |||
| 1044 | + | log.Printf("[workspace] WARNING: root promotion attachment failed, widget has no parent") | |||
| 1045 | + | // Force another attempt at setting the child | |||
| 1046 | + | wm.window.SetChild(containerPtr) | |||
| 1047 | + | webkit.WidgetQueueAllocate(containerPtr) | |||
| 1048 | + | parentAfter = webkit.WidgetGetParent(containerPtr) | |||
| 1049 | + | } | |||
| 1050 | + | ||||
| 1037 | log.Printf("[workspace] root promotion: sibling parent after=%#x", parentAfter) | 1051 | log.Printf("[workspace] root promotion: sibling parent after=%#x", parentAfter) | ||
| 1038 | - | log.Printf("[workspace] successfully promoted sibling to root: %#x", containerPtr) | 1052 | + | if parentAfter != 0 { |
| 1053 | + | log.Printf("[workspace] successfully promoted sibling to root: %#x", containerPtr) | |||
| 1054 | + | } else { | |||
| 1055 | + | log.Printf("[workspace] ERROR: failed to attach promoted sibling as window child: %#x", containerPtr) | |||
| 1056 | + | } | |||
| 1039 | return nil | 1057 | return nil | ||
| 1040 | }) | 1058 | }) | ||
| 1041 | } | 1059 | } | ||
M internal/app/browser/workspace_tree_rebalancer.go +144 -8
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "fmt" | |
| 6 | 6 | "log" | |
| 7 | 7 | "math" | |
| 8 | + | "time" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/dumber/pkg/webkit" | |
| 10 | 11 | ) | |
| @@ -125,12 +126,8 @@ func (tr *TreeRebalancer) RebalanceAfterClose(closedNode *paneNode, promotedNode | |||
| 125 | 126 | tr.widgetTxManager.FinishTransaction(txID, true, "") | |
| 126 | 127 | ||
| 127 | 128 | if tr.wm != nil && tr.wm.geometryValidator != nil { | |
| 128 | - | geom := tr.wm.geometryValidator.GetPaneGeometry(promotedNode) | |
| 129 | - | if !geom.IsValid || geom.Width <= 0 || geom.Height <= 0 { | |
| 130 | - | log.Printf("[tree-rebalancer] WARNING: promoted pane geometry invalid after promotion (%dx%d)", geom.Width, geom.Height) | |
| 131 | - | } else { | |
| 132 | - | log.Printf("[tree-rebalancer] Promotion geometry validated: %dx%d", geom.Width, geom.Height) | |
| 133 | - | } | |
| 129 | + | tr.logInitialPromotionGeometry(promotedNode) | |
| 130 | + | tr.schedulePromotionValidation(promotedNode) | |
| 134 | 131 | } | |
| 135 | 132 | } | |
| 136 | 133 | ||
| @@ -576,6 +573,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||
| 576 | 573 | } | |
| 577 | 574 | ||
| 578 | 575 | log.Printf("[tree-rebalancer] Promoting node %p (parent=%p)", node, node.parent) | |
| 576 | + | promotionStart := time.Now() | |
| 579 | 577 | ||
| 580 | 578 | // Ensure the promoted widget can expand to occupy available space | |
| 581 | 579 | expandOp := &WidgetOperation{ | |
| @@ -583,6 +581,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||
| 583 | 581 | Description: "Ensure promoted pane expands to fill space", | |
| 584 | 582 | Priority: 200, | |
| 585 | 583 | Execute: func() error { | |
| 584 | + | log.Printf("[tree-rebalancer] promotion expand for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |
| 586 | 585 | return node.container.Execute(func(ptr uintptr) error { | |
| 587 | 586 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | |
| 588 | 587 | return fmt.Errorf("promotion expand: invalid widget pointer") | |
| @@ -607,15 +606,57 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||
| 607 | 606 | Description: "Attach promoted pane to window root", | |
| 608 | 607 | Priority: 190, | |
| 609 | 608 | Execute: func() error { | |
| 609 | + | log.Printf("[tree-rebalancer] promotion window attach for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |
| 610 | 610 | return node.container.Execute(func(ptr uintptr) error { | |
| 611 | 611 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | |
| 612 | 612 | return fmt.Errorf("promotion attach window: invalid widget pointer") | |
| 613 | 613 | } | |
| 614 | - | if webkit.WidgetGetParent(ptr) != 0 { | |
| 615 | - | webkit.WidgetUnparent(ptr) | |
| 614 | + | ||
| 615 | + | currentParent := webkit.WidgetGetParent(ptr) | |
| 616 | + | if currentParent != 0 { | |
| 617 | + | // Check if parent is a container widget (paned or box) that we need to unparent from | |
| 618 | + | if webkit.IsPaned(currentParent) { | |
| 619 | + | log.Printf("[tree-rebalancer] unparenting widget from paned %#x", currentParent) | |
| 620 | + | webkit.WidgetUnparent(ptr) | |
| 621 | + | } else if webkit.IsBox(currentParent) { | |
| 622 | + | log.Printf("[tree-rebalancer] unparenting widget from box (stack) %#x", currentParent) | |
| 623 | + | webkit.WidgetUnparent(ptr) | |
| 624 | + | } else { | |
| 625 | + | // Parent is not a container - likely already attached to window | |
| 626 | + | log.Printf("[tree-rebalancer] widget already attached to non-container parent %#x, skipping unparent", currentParent) | |
| 627 | + | // Just ensure proper expansion and visibility | |
| 628 | + | webkit.WidgetSetHExpand(ptr, true) | |
| 629 | + | webkit.WidgetSetVExpand(ptr, true) | |
| 630 | + | webkit.WidgetQueueAllocate(ptr) | |
| 631 | + | webkit.WidgetShow(ptr) | |
| 632 | + | log.Printf("[tree-rebalancer] promotion window attach successful: widget %#x already has parent %#x", ptr, currentParent) | |
| 633 | + | return nil | |
| 634 | + | } | |
| 616 | 635 | } | |
| 636 | + | ||
| 637 | + | // Ensure widget is configured for window child | |
| 638 | + | webkit.WidgetSetHExpand(ptr, true) | |
| 639 | + | webkit.WidgetSetVExpand(ptr, true) | |
| 640 | + | ||
| 641 | + | // Set as window child | |
| 617 | 642 | tr.wm.window.SetChild(ptr) | |
| 618 | 643 | webkit.WidgetQueueAllocate(ptr) | |
| 644 | + | webkit.WidgetShow(ptr) | |
| 645 | + | ||
| 646 | + | // Verify the attachment worked | |
| 647 | + | finalParent := webkit.WidgetGetParent(ptr) | |
| 648 | + | if finalParent == 0 { | |
| 649 | + | log.Printf("[tree-rebalancer] WARNING: SetChild failed, widget still has no parent") | |
| 650 | + | // Try one more time | |
| 651 | + | tr.wm.window.SetChild(ptr) | |
| 652 | + | webkit.WidgetQueueAllocate(ptr) | |
| 653 | + | finalParent = webkit.WidgetGetParent(ptr) | |
| 654 | + | if finalParent == 0 { | |
| 655 | + | return fmt.Errorf("failed to attach widget %#x to window after multiple attempts", ptr) | |
| 656 | + | } | |
| 657 | + | } | |
| 658 | + | ||
| 659 | + | log.Printf("[tree-rebalancer] promotion window attach successful: widget %#x now has parent %#x", ptr, finalParent) | |
| 619 | 660 | return nil | |
| 620 | 661 | }) | |
| 621 | 662 | }, | |
| @@ -637,6 +678,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||
| 637 | 678 | Description: "Attach promoted pane to parent container", | |
| 638 | 679 | Priority: 190, | |
| 639 | 680 | Execute: func() error { | |
| 681 | + | log.Printf("[tree-rebalancer] promotion reparent for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |
| 640 | 682 | return parent.container.Execute(func(parentPtr uintptr) error { | |
| 641 | 683 | if parentPtr == 0 || !webkit.WidgetIsValid(parentPtr) { | |
| 642 | 684 | return fmt.Errorf("promotion reparent: invalid parent widget") | |
| @@ -678,6 +720,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||
| 678 | 720 | Description: "Queue allocation for promoted pane ancestors", | |
| 679 | 721 | Priority: 150, | |
| 680 | 722 | Execute: func() error { | |
| 723 | + | log.Printf("[tree-rebalancer] promotion allocation queue for %p executing at +%s (ancestors=%d)", node, time.Since(promotionStart).Round(time.Millisecond), len(ancestorPtrs)) | |
| 681 | 724 | for _, ancestorPtr := range ancestorPtrs { | |
| 682 | 725 | if ancestorPtr == 0 || !webkit.WidgetIsValid(ancestorPtr) { | |
| 683 | 726 | continue | |
| @@ -698,6 +741,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||
| 698 | 741 | Description: "Validate promoted pane allocation", | |
| 699 | 742 | Priority: 100, | |
| 700 | 743 | Execute: func() error { | |
| 744 | + | log.Printf("[tree-rebalancer] promotion immediate validation for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |
| 701 | 745 | return node.container.Execute(func(ptr uintptr) error { | |
| 702 | 746 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | |
| 703 | 747 | return fmt.Errorf("promotion validate: invalid widget pointer") | |
| @@ -732,6 +776,98 @@ func (tr *TreeRebalancer) executeRestructure(node *paneNode, tx *WidgetTransacti | |||
| 732 | 776 | return fmt.Errorf("restructure operation not yet implemented") | |
| 733 | 777 | } | |
| 734 | 778 | ||
| 779 | + | func (tr *TreeRebalancer) logInitialPromotionGeometry(node *paneNode) { | |
| 780 | + | if tr == nil || tr.wm == nil || tr.wm.geometryValidator == nil || node == nil { | |
| 781 | + | return | |
| 782 | + | } | |
| 783 | + | if node.container == nil || !node.container.IsValid() { | |
| 784 | + | log.Printf("[tree-rebalancer] Promotion geometry skipped: container invalid for %p", node) | |
| 785 | + | return | |
| 786 | + | } | |
| 787 | + | tr.ensureRootAttachment(node) | |
| 788 | + | geom := tr.wm.geometryValidator.GetPaneGeometry(node) | |
| 789 | + | if !geom.IsValid || geom.Width <= 0 || geom.Height <= 0 { | |
| 790 | + | log.Printf("[tree-rebalancer] Promotion geometry pending after commit: valid=%v size=%dx%d", geom.IsValid, geom.Width, geom.Height) | |
| 791 | + | return | |
| 792 | + | } | |
| 793 | + | log.Printf("[tree-rebalancer] Promotion geometry validated immediately: %dx%d", geom.Width, geom.Height) | |
| 794 | + | } | |
| 795 | + | ||
| 796 | + | func (tr *TreeRebalancer) schedulePromotionValidation(node *paneNode) { | |
| 797 | + | if tr == nil || tr.wm == nil || tr.wm.geometryValidator == nil || node == nil { | |
| 798 | + | return | |
| 799 | + | } | |
| 800 | + | ||
| 801 | + | const maxAttempts = 5 | |
| 802 | + | start := time.Now() | |
| 803 | + | attempt := 0 | |
| 804 | + | ||
| 805 | + | var retry func() bool | |
| 806 | + | retry = func() bool { | |
| 807 | + | attempt++ | |
| 808 | + | elapsed := time.Since(start).Round(time.Millisecond) | |
| 809 | + | if node.container == nil || !node.container.IsValid() { | |
| 810 | + | log.Printf("[tree-rebalancer] Deferred promotion geometry aborted after %s (attempt %d): container invalid", elapsed, attempt) | |
| 811 | + | return false | |
| 812 | + | } | |
| 813 | + | tr.ensureRootAttachment(node) | |
| 814 | + | geom := tr.wm.geometryValidator.GetPaneGeometry(node) | |
| 815 | + | if geom.IsValid && geom.Width > 0 && geom.Height > 0 { | |
| 816 | + | log.Printf("[tree-rebalancer] Deferred promotion geometry validated after %s (attempt %d): %dx%d", elapsed, attempt, geom.Width, geom.Height) | |
| 817 | + | return false | |
| 818 | + | } | |
| 819 | + | ||
| 820 | + | if attempt >= maxAttempts { | |
| 821 | + | log.Printf("[tree-rebalancer] Deferred promotion geometry still invalid after %s (attempt %d): valid=%v size=%dx%d", elapsed, attempt, geom.IsValid, geom.Width, geom.Height) | |
| 822 | + | return false | |
| 823 | + | } | |
| 824 | + | ||
| 825 | + | log.Printf("[tree-rebalancer] Promotion geometry still pending after %s (attempt %d); rescheduling", elapsed, attempt) | |
| 826 | + | webkit.IdleAdd(retry) | |
| 827 | + | return false | |
| 828 | + | } | |
| 829 | + | ||
| 830 | + | webkit.IdleAdd(retry) | |
| 831 | + | } | |
| 832 | + | ||
| 833 | + | func (tr *TreeRebalancer) ensureRootAttachment(node *paneNode) { | |
| 834 | + | if tr == nil || tr.wm == nil || node == nil || node.parent != nil { | |
| 835 | + | return | |
| 836 | + | } | |
| 837 | + | if node.container == nil || !node.container.IsValid() { | |
| 838 | + | return | |
| 839 | + | } | |
| 840 | + | ||
| 841 | + | _ = node.container.Execute(func(ptr uintptr) error { | |
| 842 | + | parent := webkit.WidgetGetParent(ptr) | |
| 843 | + | if parent != 0 { | |
| 844 | + | // Widget is properly attached, no action needed | |
| 845 | + | return nil | |
| 846 | + | } | |
| 847 | + | if tr.wm.window == nil { | |
| 848 | + | log.Printf("[tree-rebalancer] Root pane %#x missing parent but window unavailable", ptr) | |
| 849 | + | return nil | |
| 850 | + | } | |
| 851 | + | ||
| 852 | + | // This should not happen if promotion worked correctly - log as error | |
| 853 | + | log.Printf("[tree-rebalancer] ERROR: Root pane %#x lost window attachment (this indicates a promotion bug); attempting recovery", ptr) | |
| 854 | + | ||
| 855 | + | // Recovery attempt | |
| 856 | + | tr.wm.window.SetChild(ptr) | |
| 857 | + | webkit.WidgetQueueAllocate(ptr) | |
| 858 | + | webkit.WidgetShow(ptr) | |
| 859 | + | ||
| 860 | + | // Verify recovery | |
| 861 | + | recoveredParent := webkit.WidgetGetParent(ptr) | |
| 862 | + | if recoveredParent == 0 { | |
| 863 | + | log.Printf("[tree-rebalancer] CRITICAL: Failed to recover window attachment for %#x", ptr) | |
| 864 | + | } else { | |
| 865 | + | log.Printf("[tree-rebalancer] Recovery successful: widget %#x now has parent %#x", ptr, recoveredParent) | |
| 866 | + | } | |
| 867 | + | return nil | |
| 868 | + | }) | |
| 869 | + | } | |
| 870 | + | ||
| 735 | 871 | // collectAncestorContainers returns the widget pointers for the promoted node and all ancestors up to the root. | |
| 736 | 872 | func (tr *TreeRebalancer) collectAncestorContainers(node *paneNode) []uintptr { | |
| 737 | 873 | var containers []uintptr | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "log" | 6 | "log" | ||
| 7 | "math" | 7 | "math" | ||
| 8 | + | "time" | |||
| 8 | 9 | ||||
| 9 | "github.com/bnema/dumber/pkg/webkit" | 10 | "github.com/bnema/dumber/pkg/webkit" | ||
| 10 | ) | 11 | ) | ||
| @@ -125,12 +126,8 @@ func (tr *TreeRebalancer) RebalanceAfterClose(closedNode *paneNode, promotedNode | |||||
| 125 | tr.widgetTxManager.FinishTransaction(txID, true, "") | 126 | tr.widgetTxManager.FinishTransaction(txID, true, "") | ||
| 126 | 127 | ||||
| 127 | if tr.wm != nil && tr.wm.geometryValidator != nil { | 128 | if tr.wm != nil && tr.wm.geometryValidator != nil { | ||
| 128 | - | geom := tr.wm.geometryValidator.GetPaneGeometry(promotedNode) | 129 | + | tr.logInitialPromotionGeometry(promotedNode) |
| 129 | - | if !geom.IsValid || geom.Width <= 0 || geom.Height <= 0 { | 130 | + | tr.schedulePromotionValidation(promotedNode) |
| 130 | - | log.Printf("[tree-rebalancer] WARNING: promoted pane geometry invalid after promotion (%dx%d)", geom.Width, geom.Height) | |||
| 131 | - | } else { | |||
| 132 | - | log.Printf("[tree-rebalancer] Promotion geometry validated: %dx%d", geom.Width, geom.Height) | |||
| 133 | - | } | |||
| 134 | } | 131 | } | ||
| 135 | } | 132 | } | ||
| 136 | 133 | ||||
| @@ -576,6 +573,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||||
| 576 | } | 573 | } | ||
| 577 | 574 | ||||
| 578 | log.Printf("[tree-rebalancer] Promoting node %p (parent=%p)", node, node.parent) | 575 | log.Printf("[tree-rebalancer] Promoting node %p (parent=%p)", node, node.parent) | ||
| 576 | + | promotionStart := time.Now() | |||
| 579 | 577 | ||||
| 580 | // Ensure the promoted widget can expand to occupy available space | 578 | // Ensure the promoted widget can expand to occupy available space | ||
| 581 | expandOp := &WidgetOperation{ | 579 | expandOp := &WidgetOperation{ | ||
| @@ -583,6 +581,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||||
| 583 | Description: "Ensure promoted pane expands to fill space", | 581 | Description: "Ensure promoted pane expands to fill space", | ||
| 584 | Priority: 200, | 582 | Priority: 200, | ||
| 585 | Execute: func() error { | 583 | Execute: func() error { | ||
| 584 | + | log.Printf("[tree-rebalancer] promotion expand for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |||
| 586 | return node.container.Execute(func(ptr uintptr) error { | 585 | return node.container.Execute(func(ptr uintptr) error { | ||
| 587 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | 586 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | ||
| 588 | return fmt.Errorf("promotion expand: invalid widget pointer") | 587 | return fmt.Errorf("promotion expand: invalid widget pointer") | ||
| @@ -607,15 +606,57 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||||
| 607 | Description: "Attach promoted pane to window root", | 606 | Description: "Attach promoted pane to window root", | ||
| 608 | Priority: 190, | 607 | Priority: 190, | ||
| 609 | Execute: func() error { | 608 | Execute: func() error { | ||
| 609 | + | log.Printf("[tree-rebalancer] promotion window attach for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |||
| 610 | return node.container.Execute(func(ptr uintptr) error { | 610 | return node.container.Execute(func(ptr uintptr) error { | ||
| 611 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | 611 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | ||
| 612 | return fmt.Errorf("promotion attach window: invalid widget pointer") | 612 | return fmt.Errorf("promotion attach window: invalid widget pointer") | ||
| 613 | } | 613 | } | ||
| 614 | - | if webkit.WidgetGetParent(ptr) != 0 { | 614 | + | |
| 615 | - | webkit.WidgetUnparent(ptr) | 615 | + | currentParent := webkit.WidgetGetParent(ptr) |
| 616 | + | if currentParent != 0 { | |||
| 617 | + | // Check if parent is a container widget (paned or box) that we need to unparent from | |||
| 618 | + | if webkit.IsPaned(currentParent) { | |||
| 619 | + | log.Printf("[tree-rebalancer] unparenting widget from paned %#x", currentParent) | |||
| 620 | + | webkit.WidgetUnparent(ptr) | |||
| 621 | + | } else if webkit.IsBox(currentParent) { | |||
| 622 | + | log.Printf("[tree-rebalancer] unparenting widget from box (stack) %#x", currentParent) | |||
| 623 | + | webkit.WidgetUnparent(ptr) | |||
| 624 | + | } else { | |||
| 625 | + | // Parent is not a container - likely already attached to window | |||
| 626 | + | log.Printf("[tree-rebalancer] widget already attached to non-container parent %#x, skipping unparent", currentParent) | |||
| 627 | + | // Just ensure proper expansion and visibility | |||
| 628 | + | webkit.WidgetSetHExpand(ptr, true) | |||
| 629 | + | webkit.WidgetSetVExpand(ptr, true) | |||
| 630 | + | webkit.WidgetQueueAllocate(ptr) | |||
| 631 | + | webkit.WidgetShow(ptr) | |||
| 632 | + | log.Printf("[tree-rebalancer] promotion window attach successful: widget %#x already has parent %#x", ptr, currentParent) | |||
| 633 | + | return nil | |||
| 634 | + | } | |||
| 616 | } | 635 | } | ||
| 636 | + | ||||
| 637 | + | // Ensure widget is configured for window child | |||
| 638 | + | webkit.WidgetSetHExpand(ptr, true) | |||
| 639 | + | webkit.WidgetSetVExpand(ptr, true) | |||
| 640 | + | ||||
| 641 | + | // Set as window child | |||
| 617 | tr.wm.window.SetChild(ptr) | 642 | tr.wm.window.SetChild(ptr) | ||
| 618 | webkit.WidgetQueueAllocate(ptr) | 643 | webkit.WidgetQueueAllocate(ptr) | ||
| 644 | + | webkit.WidgetShow(ptr) | |||
| 645 | + | ||||
| 646 | + | // Verify the attachment worked | |||
| 647 | + | finalParent := webkit.WidgetGetParent(ptr) | |||
| 648 | + | if finalParent == 0 { | |||
| 649 | + | log.Printf("[tree-rebalancer] WARNING: SetChild failed, widget still has no parent") | |||
| 650 | + | // Try one more time | |||
| 651 | + | tr.wm.window.SetChild(ptr) | |||
| 652 | + | webkit.WidgetQueueAllocate(ptr) | |||
| 653 | + | finalParent = webkit.WidgetGetParent(ptr) | |||
| 654 | + | if finalParent == 0 { | |||
| 655 | + | return fmt.Errorf("failed to attach widget %#x to window after multiple attempts", ptr) | |||
| 656 | + | } | |||
| 657 | + | } | |||
| 658 | + | ||||
| 659 | + | log.Printf("[tree-rebalancer] promotion window attach successful: widget %#x now has parent %#x", ptr, finalParent) | |||
| 619 | return nil | 660 | return nil | ||
| 620 | }) | 661 | }) | ||
| 621 | }, | 662 | }, | ||
| @@ -637,6 +678,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||||
| 637 | Description: "Attach promoted pane to parent container", | 678 | Description: "Attach promoted pane to parent container", | ||
| 638 | Priority: 190, | 679 | Priority: 190, | ||
| 639 | Execute: func() error { | 680 | Execute: func() error { | ||
| 681 | + | log.Printf("[tree-rebalancer] promotion reparent for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |||
| 640 | return parent.container.Execute(func(parentPtr uintptr) error { | 682 | return parent.container.Execute(func(parentPtr uintptr) error { | ||
| 641 | if parentPtr == 0 || !webkit.WidgetIsValid(parentPtr) { | 683 | if parentPtr == 0 || !webkit.WidgetIsValid(parentPtr) { | ||
| 642 | return fmt.Errorf("promotion reparent: invalid parent widget") | 684 | return fmt.Errorf("promotion reparent: invalid parent widget") | ||
| @@ -678,6 +720,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||||
| 678 | Description: "Queue allocation for promoted pane ancestors", | 720 | Description: "Queue allocation for promoted pane ancestors", | ||
| 679 | Priority: 150, | 721 | Priority: 150, | ||
| 680 | Execute: func() error { | 722 | Execute: func() error { | ||
| 723 | + | log.Printf("[tree-rebalancer] promotion allocation queue for %p executing at +%s (ancestors=%d)", node, time.Since(promotionStart).Round(time.Millisecond), len(ancestorPtrs)) | |||
| 681 | for _, ancestorPtr := range ancestorPtrs { | 724 | for _, ancestorPtr := range ancestorPtrs { | ||
| 682 | if ancestorPtr == 0 || !webkit.WidgetIsValid(ancestorPtr) { | 725 | if ancestorPtr == 0 || !webkit.WidgetIsValid(ancestorPtr) { | ||
| 683 | continue | 726 | continue | ||
| @@ -698,6 +741,7 @@ func (tr *TreeRebalancer) executePromotion(node *paneNode, tx *WidgetTransaction | |||||
| 698 | Description: "Validate promoted pane allocation", | 741 | Description: "Validate promoted pane allocation", | ||
| 699 | Priority: 100, | 742 | Priority: 100, | ||
| 700 | Execute: func() error { | 743 | Execute: func() error { | ||
| 744 | + | log.Printf("[tree-rebalancer] promotion immediate validation for %p executing at +%s", node, time.Since(promotionStart).Round(time.Millisecond)) | |||
| 701 | return node.container.Execute(func(ptr uintptr) error { | 745 | return node.container.Execute(func(ptr uintptr) error { | ||
| 702 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | 746 | if ptr == 0 || !webkit.WidgetIsValid(ptr) { | ||
| 703 | return fmt.Errorf("promotion validate: invalid widget pointer") | 747 | return fmt.Errorf("promotion validate: invalid widget pointer") | ||
| @@ -732,6 +776,98 @@ func (tr *TreeRebalancer) executeRestructure(node *paneNode, tx *WidgetTransacti | |||||
| 732 | return fmt.Errorf("restructure operation not yet implemented") | 776 | return fmt.Errorf("restructure operation not yet implemented") | ||
| 733 | } | 777 | } | ||
| 734 | 778 | ||||
| 779 | + | func (tr *TreeRebalancer) logInitialPromotionGeometry(node *paneNode) { | |||
| 780 | + | if tr == nil || tr.wm == nil || tr.wm.geometryValidator == nil || node == nil { | |||
| 781 | + | return | |||
| 782 | + | } | |||
| 783 | + | if node.container == nil || !node.container.IsValid() { | |||
| 784 | + | log.Printf("[tree-rebalancer] Promotion geometry skipped: container invalid for %p", node) | |||
| 785 | + | return | |||
| 786 | + | } | |||
| 787 | + | tr.ensureRootAttachment(node) | |||
| 788 | + | geom := tr.wm.geometryValidator.GetPaneGeometry(node) | |||
| 789 | + | if !geom.IsValid || geom.Width <= 0 || geom.Height <= 0 { | |||
| 790 | + | log.Printf("[tree-rebalancer] Promotion geometry pending after commit: valid=%v size=%dx%d", geom.IsValid, geom.Width, geom.Height) | |||
| 791 | + | return | |||
| 792 | + | } | |||
| 793 | + | log.Printf("[tree-rebalancer] Promotion geometry validated immediately: %dx%d", geom.Width, geom.Height) | |||
| 794 | + | } | |||
| 795 | + | ||||
| 796 | + | func (tr *TreeRebalancer) schedulePromotionValidation(node *paneNode) { | |||
| 797 | + | if tr == nil || tr.wm == nil || tr.wm.geometryValidator == nil || node == nil { | |||
| 798 | + | return | |||
| 799 | + | } | |||
| 800 | + | ||||
| 801 | + | const maxAttempts = 5 | |||
| 802 | + | start := time.Now() | |||
| 803 | + | attempt := 0 | |||
| 804 | + | ||||
| 805 | + | var retry func() bool | |||
| 806 | + | retry = func() bool { | |||
| 807 | + | attempt++ | |||
| 808 | + | elapsed := time.Since(start).Round(time.Millisecond) | |||
| 809 | + | if node.container == nil || !node.container.IsValid() { | |||
| 810 | + | log.Printf("[tree-rebalancer] Deferred promotion geometry aborted after %s (attempt %d): container invalid", elapsed, attempt) | |||
| 811 | + | return false | |||
| 812 | + | } | |||
| 813 | + | tr.ensureRootAttachment(node) | |||
| 814 | + | geom := tr.wm.geometryValidator.GetPaneGeometry(node) | |||
| 815 | + | if geom.IsValid && geom.Width > 0 && geom.Height > 0 { | |||
| 816 | + | log.Printf("[tree-rebalancer] Deferred promotion geometry validated after %s (attempt %d): %dx%d", elapsed, attempt, geom.Width, geom.Height) | |||
| 817 | + | return false | |||
| 818 | + | } | |||
| 819 | + | ||||
| 820 | + | if attempt >= maxAttempts { | |||
| 821 | + | log.Printf("[tree-rebalancer] Deferred promotion geometry still invalid after %s (attempt %d): valid=%v size=%dx%d", elapsed, attempt, geom.IsValid, geom.Width, geom.Height) | |||
| 822 | + | return false | |||
| 823 | + | } | |||
| 824 | + | ||||
| 825 | + | log.Printf("[tree-rebalancer] Promotion geometry still pending after %s (attempt %d); rescheduling", elapsed, attempt) | |||
| 826 | + | webkit.IdleAdd(retry) | |||
| 827 | + | return false | |||
| 828 | + | } | |||
| 829 | + | ||||
| 830 | + | webkit.IdleAdd(retry) | |||
| 831 | + | } | |||
| 832 | + | ||||
| 833 | + | func (tr *TreeRebalancer) ensureRootAttachment(node *paneNode) { | |||
| 834 | + | if tr == nil || tr.wm == nil || node == nil || node.parent != nil { | |||
| 835 | + | return | |||
| 836 | + | } | |||
| 837 | + | if node.container == nil || !node.container.IsValid() { | |||
| 838 | + | return | |||
| 839 | + | } | |||
| 840 | + | ||||
| 841 | + | _ = node.container.Execute(func(ptr uintptr) error { | |||
| 842 | + | parent := webkit.WidgetGetParent(ptr) | |||
| 843 | + | if parent != 0 { | |||
| 844 | + | // Widget is properly attached, no action needed | |||
| 845 | + | return nil | |||
| 846 | + | } | |||
| 847 | + | if tr.wm.window == nil { | |||
| 848 | + | log.Printf("[tree-rebalancer] Root pane %#x missing parent but window unavailable", ptr) | |||
| 849 | + | return nil | |||
| 850 | + | } | |||
| 851 | + | ||||
| 852 | + | // This should not happen if promotion worked correctly - log as error | |||
| 853 | + | log.Printf("[tree-rebalancer] ERROR: Root pane %#x lost window attachment (this indicates a promotion bug); attempting recovery", ptr) | |||
| 854 | + | ||||
| 855 | + | // Recovery attempt | |||
| 856 | + | tr.wm.window.SetChild(ptr) | |||
| 857 | + | webkit.WidgetQueueAllocate(ptr) | |||
| 858 | + | webkit.WidgetShow(ptr) | |||
| 859 | + | ||||
| 860 | + | // Verify recovery | |||
| 861 | + | recoveredParent := webkit.WidgetGetParent(ptr) | |||
| 862 | + | if recoveredParent == 0 { | |||
| 863 | + | log.Printf("[tree-rebalancer] CRITICAL: Failed to recover window attachment for %#x", ptr) | |||
| 864 | + | } else { | |||
| 865 | + | log.Printf("[tree-rebalancer] Recovery successful: widget %#x now has parent %#x", ptr, recoveredParent) | |||
| 866 | + | } | |||
| 867 | + | return nil | |||
| 868 | + | }) | |||
| 869 | + | } | |||
| 870 | + | ||||
| 735 | // collectAncestorContainers returns the widget pointers for the promoted node and all ancestors up to the root. | 871 | // collectAncestorContainers returns the widget pointers for the promoted node and all ancestors up to the root. | ||
| 736 | func (tr *TreeRebalancer) collectAncestorContainers(node *paneNode) []uintptr { | 872 | func (tr *TreeRebalancer) collectAncestorContainers(node *paneNode) []uintptr { | ||
| 737 | var containers []uintptr | 873 | var containers []uintptr | ||
M pkg/webkit/workspace_widgets_cgo.go +27 -0
| @@ -6,6 +6,8 @@ package webkit | |||
| 6 | 6 | #cgo pkg-config: gtk4 | |
| 7 | 7 | #include <gtk/gtk.h> | |
| 8 | 8 | #include <gtk/gtktestutils.h> | |
| 9 | + | #include <gtk/gtkbox.h> | |
| 10 | + | #include <gtk/gtkpaned.h> | |
| 9 | 11 | #include <gdk/gdk.h> | |
| 10 | 12 | #include <glib.h> | |
| 11 | 13 | #include <glib-object.h> | |
| @@ -17,6 +19,15 @@ package webkit | |||
| 17 | 19 | // Logging helper for diagnostics | |
| 18 | 20 | #define WORKSPACE_LOG(fmt, ...) g_printerr("[workspace-cgo] " fmt "\n", ##__VA_ARGS__) | |
| 19 | 21 | ||
| 22 | + | // Helper functions for widget type checking | |
| 23 | + | static gboolean is_paned_widget(GtkWidget *widget) { | |
| 24 | + | return GTK_IS_PANED(widget); | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | static gboolean is_box_widget(GtkWidget *widget) { | |
| 28 | + | return GTK_IS_BOX(widget); | |
| 29 | + | } | |
| 30 | + | ||
| 20 | 31 | // Callback function for idle handling following the existing pattern | |
| 21 | 32 | extern gboolean goIdleCallback(uintptr_t handle); | |
| 22 | 33 | extern void goHoverCallback(uintptr_t handle); | |
| @@ -431,6 +442,22 @@ func WidgetIsValid(widget uintptr) bool { | |||
| 431 | 442 | return widgetIsValid(widget) | |
| 432 | 443 | } | |
| 433 | 444 | ||
| 445 | + | // IsPaned checks if a widget is a GtkPaned container. | |
| 446 | + | func IsPaned(widget uintptr) bool { | |
| 447 | + | if widget == 0 { | |
| 448 | + | return false | |
| 449 | + | } | |
| 450 | + | return C.is_paned_widget((*C.GtkWidget)(unsafe.Pointer(widget))) != 0 | |
| 451 | + | } | |
| 452 | + | ||
| 453 | + | // IsBox checks if a widget is a GtkBox container. | |
| 454 | + | func IsBox(widget uintptr) bool { | |
| 455 | + | if widget == 0 { | |
| 456 | + | return false | |
| 457 | + | } | |
| 458 | + | return C.is_box_widget((*C.GtkWidget)(unsafe.Pointer(widget))) != 0 | |
| 459 | + | } | |
| 460 | + | ||
| 434 | 461 | // WidgetRefCount exposes the widget reference count for debugging. | |
| 435 | 462 | func WidgetRefCount(widget uintptr) uint { | |
| 436 | 463 | return widgetRefCount(widget) | |
| @@ -6,6 +6,8 @@ package webkit | |||||
| 6 | #cgo pkg-config: gtk4 | 6 | #cgo pkg-config: gtk4 | ||
| 7 | #include <gtk/gtk.h> | 7 | #include <gtk/gtk.h> | ||
| 8 | #include <gtk/gtktestutils.h> | 8 | #include <gtk/gtktestutils.h> | ||
| 9 | + | #include <gtk/gtkbox.h> | |||
| 10 | + | #include <gtk/gtkpaned.h> | |||
| 9 | #include <gdk/gdk.h> | 11 | #include <gdk/gdk.h> | ||
| 10 | #include <glib.h> | 12 | #include <glib.h> | ||
| 11 | #include <glib-object.h> | 13 | #include <glib-object.h> | ||
| @@ -17,6 +19,15 @@ package webkit | |||||
| 17 | // Logging helper for diagnostics | 19 | // Logging helper for diagnostics | ||
| 18 | #define WORKSPACE_LOG(fmt, ...) g_printerr("[workspace-cgo] " fmt "\n", ##__VA_ARGS__) | 20 | #define WORKSPACE_LOG(fmt, ...) g_printerr("[workspace-cgo] " fmt "\n", ##__VA_ARGS__) | ||
| 19 | 21 | ||||
| 22 | + | // Helper functions for widget type checking | |||
| 23 | + | static gboolean is_paned_widget(GtkWidget *widget) { | |||
| 24 | + | return GTK_IS_PANED(widget); | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | static gboolean is_box_widget(GtkWidget *widget) { | |||
| 28 | + | return GTK_IS_BOX(widget); | |||
| 29 | + | } | |||
| 30 | + | ||||
| 20 | // Callback function for idle handling following the existing pattern | 31 | // Callback function for idle handling following the existing pattern | ||
| 21 | extern gboolean goIdleCallback(uintptr_t handle); | 32 | extern gboolean goIdleCallback(uintptr_t handle); | ||
| 22 | extern void goHoverCallback(uintptr_t handle); | 33 | extern void goHoverCallback(uintptr_t handle); | ||
| @@ -431,6 +442,22 @@ func WidgetIsValid(widget uintptr) bool { | |||||
| 431 | return widgetIsValid(widget) | 442 | return widgetIsValid(widget) | ||
| 432 | } | 443 | } | ||
| 433 | 444 | ||||
| 445 | + | // IsPaned checks if a widget is a GtkPaned container. | |||
| 446 | + | func IsPaned(widget uintptr) bool { | |||
| 447 | + | if widget == 0 { | |||
| 448 | + | return false | |||
| 449 | + | } | |||
| 450 | + | return C.is_paned_widget((*C.GtkWidget)(unsafe.Pointer(widget))) != 0 | |||
| 451 | + | } | |||
| 452 | + | ||||
| 453 | + | // IsBox checks if a widget is a GtkBox container. | |||
| 454 | + | func IsBox(widget uintptr) bool { | |||
| 455 | + | if widget == 0 { | |||
| 456 | + | return false | |||
| 457 | + | } | |||
| 458 | + | return C.is_box_widget((*C.GtkWidget)(unsafe.Pointer(widget))) != 0 | |||
| 459 | + | } | |||
| 460 | + | ||||
| 434 | // WidgetRefCount exposes the widget reference count for debugging. | 461 | // WidgetRefCount exposes the widget reference count for debugging. | ||
| 435 | func WidgetRefCount(widget uintptr) uint { | 462 | func WidgetRefCount(widget uintptr) uint { | ||
| 436 | return widgetRefCount(widget) | 463 | return widgetRefCount(widget) | ||
M pkg/webkit/workspace_widgets_stub.go +47 -2
| @@ -38,6 +38,7 @@ type widgetStub struct { | |||
| 38 | 38 | parent uintptr | |
| 39 | 39 | isDestroyed bool | |
| 40 | 40 | refCount uint | |
| 41 | + | widgetType string // Track widget type for IsPaned, etc. | |
| 41 | 42 | // GTK focus management simulation | |
| 42 | 43 | hasFocus bool | |
| 43 | 44 | canFocus bool | |
| @@ -93,7 +94,16 @@ func ResetWidgetStubsForTesting() { | |||
| 93 | 94 | } | |
| 94 | 95 | ||
| 95 | 96 | func NewPaned(orientation Orientation) uintptr { | |
| 96 | - | return newWidgetHandle() | |
| 97 | + | widgetMu.Lock() | |
| 98 | + | defer widgetMu.Unlock() | |
| 99 | + | id := nextWidgetID | |
| 100 | + | nextWidgetID++ | |
| 101 | + | widgetState[id] = &widgetStub{ | |
| 102 | + | refCount: 1, | |
| 103 | + | canFocus: true, | |
| 104 | + | widgetType: "paned", // Mark as paned widget | |
| 105 | + | } | |
| 106 | + | return id | |
| 97 | 107 | } | |
| 98 | 108 | ||
| 99 | 109 | func PanedSetStartChild(paned uintptr, child uintptr) { | |
| @@ -295,6 +305,32 @@ func WidgetIsValid(widget uintptr) bool { | |||
| 295 | 305 | return false | |
| 296 | 306 | } | |
| 297 | 307 | ||
| 308 | + | // IsPaned checks if a widget is a GtkPaned container. | |
| 309 | + | func IsPaned(widget uintptr) bool { | |
| 310 | + | widgetMu.Lock() | |
| 311 | + | defer widgetMu.Unlock() | |
| 312 | + | if widget == 0 { | |
| 313 | + | return false | |
| 314 | + | } | |
| 315 | + | if stub, ok := widgetState[widget]; ok && !stub.isDestroyed { | |
| 316 | + | return stub.widgetType == "paned" | |
| 317 | + | } | |
| 318 | + | return false | |
| 319 | + | } | |
| 320 | + | ||
| 321 | + | // IsBox checks if a widget is a GtkBox container. | |
| 322 | + | func IsBox(widget uintptr) bool { | |
| 323 | + | widgetMu.Lock() | |
| 324 | + | defer widgetMu.Unlock() | |
| 325 | + | if widget == 0 { | |
| 326 | + | return false | |
| 327 | + | } | |
| 328 | + | if stub, ok := widgetState[widget]; ok && !stub.isDestroyed { | |
| 329 | + | return stub.widgetType == "box" | |
| 330 | + | } | |
| 331 | + | return false | |
| 332 | + | } | |
| 333 | + | ||
| 298 | 334 | func WidgetRefCount(widget uintptr) uint { | |
| 299 | 335 | if WidgetIsValid(widget) { | |
| 300 | 336 | return 1 | |
| @@ -410,7 +446,16 @@ func SimulateFocusForTesting(widget uintptr) { | |||
| 410 | 446 | // GtkBox functions for stacked panes (stub implementations) | |
| 411 | 447 | ||
| 412 | 448 | func NewBox(orientation Orientation, spacing int) uintptr { | |
| 413 | - | return newWidgetHandle() | |
| 449 | + | widgetMu.Lock() | |
| 450 | + | defer widgetMu.Unlock() | |
| 451 | + | id := nextWidgetID | |
| 452 | + | nextWidgetID++ | |
| 453 | + | widgetState[id] = &widgetStub{ | |
| 454 | + | refCount: 1, | |
| 455 | + | canFocus: true, | |
| 456 | + | widgetType: "box", // Mark as box widget | |
| 457 | + | } | |
| 458 | + | return id | |
| 414 | 459 | } | |
| 415 | 460 | ||
| 416 | 461 | func BoxAppend(box uintptr, child uintptr) { | |
| @@ -38,6 +38,7 @@ type widgetStub struct { | |||||
| 38 | parent uintptr | 38 | parent uintptr | ||
| 39 | isDestroyed bool | 39 | isDestroyed bool | ||
| 40 | refCount uint | 40 | refCount uint | ||
| 41 | + | widgetType string // Track widget type for IsPaned, etc. | |||
| 41 | // GTK focus management simulation | 42 | // GTK focus management simulation | ||
| 42 | hasFocus bool | 43 | hasFocus bool | ||
| 43 | canFocus bool | 44 | canFocus bool | ||
| @@ -93,7 +94,16 @@ func ResetWidgetStubsForTesting() { | |||||
| 93 | } | 94 | } | ||
| 94 | 95 | ||||
| 95 | func NewPaned(orientation Orientation) uintptr { | 96 | func NewPaned(orientation Orientation) uintptr { | ||
| 96 | - | return newWidgetHandle() | 97 | + | widgetMu.Lock() |
| 98 | + | defer widgetMu.Unlock() | |||
| 99 | + | id := nextWidgetID | |||
| 100 | + | nextWidgetID++ | |||
| 101 | + | widgetState[id] = &widgetStub{ | |||
| 102 | + | refCount: 1, | |||
| 103 | + | canFocus: true, | |||
| 104 | + | widgetType: "paned", // Mark as paned widget | |||
| 105 | + | } | |||
| 106 | + | return id | |||
| 97 | } | 107 | } | ||
| 98 | 108 | ||||
| 99 | func PanedSetStartChild(paned uintptr, child uintptr) { | 109 | func PanedSetStartChild(paned uintptr, child uintptr) { | ||
| @@ -295,6 +305,32 @@ func WidgetIsValid(widget uintptr) bool { | |||||
| 295 | return false | 305 | return false | ||
| 296 | } | 306 | } | ||
| 297 | 307 | ||||
| 308 | + | // IsPaned checks if a widget is a GtkPaned container. | |||
| 309 | + | func IsPaned(widget uintptr) bool { | |||
| 310 | + | widgetMu.Lock() | |||
| 311 | + | defer widgetMu.Unlock() | |||
| 312 | + | if widget == 0 { | |||
| 313 | + | return false | |||
| 314 | + | } | |||
| 315 | + | if stub, ok := widgetState[widget]; ok && !stub.isDestroyed { | |||
| 316 | + | return stub.widgetType == "paned" | |||
| 317 | + | } | |||
| 318 | + | return false | |||
| 319 | + | } | |||
| 320 | + | ||||
| 321 | + | // IsBox checks if a widget is a GtkBox container. | |||
| 322 | + | func IsBox(widget uintptr) bool { | |||
| 323 | + | widgetMu.Lock() | |||
| 324 | + | defer widgetMu.Unlock() | |||
| 325 | + | if widget == 0 { | |||
| 326 | + | return false | |||
| 327 | + | } | |||
| 328 | + | if stub, ok := widgetState[widget]; ok && !stub.isDestroyed { | |||
| 329 | + | return stub.widgetType == "box" | |||
| 330 | + | } | |||
| 331 | + | return false | |||
| 332 | + | } | |||
| 333 | + | ||||
| 298 | func WidgetRefCount(widget uintptr) uint { | 334 | func WidgetRefCount(widget uintptr) uint { | ||
| 299 | if WidgetIsValid(widget) { | 335 | if WidgetIsValid(widget) { | ||
| 300 | return 1 | 336 | return 1 | ||
| @@ -410,7 +446,16 @@ func SimulateFocusForTesting(widget uintptr) { | |||||
| 410 | // GtkBox functions for stacked panes (stub implementations) | 446 | // GtkBox functions for stacked panes (stub implementations) | ||
| 411 | 447 | ||||
| 412 | func NewBox(orientation Orientation, spacing int) uintptr { | 448 | func NewBox(orientation Orientation, spacing int) uintptr { | ||
| 413 | - | return newWidgetHandle() | 449 | + | widgetMu.Lock() |
| 450 | + | defer widgetMu.Unlock() | |||
| 451 | + | id := nextWidgetID | |||
| 452 | + | nextWidgetID++ | |||
| 453 | + | widgetState[id] = &widgetStub{ | |||
| 454 | + | refCount: 1, | |||
| 455 | + | canFocus: true, | |||
| 456 | + | widgetType: "box", // Mark as box widget | |||
| 457 | + | } | |||
| 458 | + | return id | |||
| 414 | } | 459 | } | ||
| 415 | 460 | ||||
| 416 | func BoxAppend(box uintptr, child uintptr) { | 461 | func BoxAppend(box uintptr, child uintptr) { | ||