fix: improve focus management and cleanup for stacked panes
- commit
- e7e2362104ed3f20d803e230f96b3a94bfb34003
- author
- bamen · · 2025-09-25 19:27 UTC
- committer
- bamen · · 2025-09-25 20:44 UTC
- parent
- 2027e70
- signature
- unsigned
5 files · +141 -251
M internal/app/browser/focus_manager.go +16 -24
| @@ -64,7 +64,7 @@ func (fm *FocusManager) SetActivePane(node *paneNode) { | |||
| 64 | 64 | fm.setGTKFocus(node) | |
| 65 | 65 | ||
| 66 | 66 | // Step 2: Update visual indicators | |
| 67 | - | fm.updateVisualState(oldPane, node) | |
| 67 | + | fm.updateVisualState(node) | |
| 68 | 68 | ||
| 69 | 69 | // Step 3: Notify JavaScript bridge | |
| 70 | 70 | fm.notifyJavaScript(oldPane, node) | |
| @@ -106,17 +106,20 @@ func (fm *FocusManager) setGTKFocus(node *paneNode) { | |||
| 106 | 106 | } | |
| 107 | 107 | ||
| 108 | 108 | // updateVisualState manages CSS classes and visibility | |
| 109 | - | func (fm *FocusManager) updateVisualState(oldPane, newPane *paneNode) { | |
| 109 | + | func (fm *FocusManager) updateVisualState(newPane *paneNode) { | |
| 110 | 110 | activePaneClass := "workspace-pane-active" | |
| 111 | 111 | ||
| 112 | - | // Remove active class from old pane | |
| 113 | - | if oldPane != nil && oldPane.container != 0 { | |
| 114 | - | webkit.WidgetRemoveCSSClass(oldPane.container, activePaneClass) | |
| 115 | - | webkit.WidgetQueueDraw(oldPane.container) | |
| 112 | + | // ROBUST: Remove active class from ALL leaf panes to prevent multiple active states | |
| 113 | + | // This ensures we never have a race condition or missed cleanup | |
| 114 | + | leaves := fm.wm.collectLeaves() | |
| 115 | + | for _, leaf := range leaves { | |
| 116 | + | if leaf != nil && leaf.container != 0 { | |
| 117 | + | webkit.WidgetRemoveCSSClass(leaf.container, activePaneClass) | |
| 118 | + | } | |
| 116 | 119 | } | |
| 117 | 120 | ||
| 118 | - | // Add active class to new pane | |
| 119 | - | if newPane.container != 0 { | |
| 121 | + | // Add active class to new pane only | |
| 122 | + | if newPane != nil && newPane.container != 0 { | |
| 120 | 123 | webkit.WidgetAddCSSClass(newPane.container, activePaneClass) | |
| 121 | 124 | webkit.WidgetQueueDraw(newPane.container) | |
| 122 | 125 | } | |
| @@ -182,7 +185,7 @@ func (fm *FocusManager) notifyJavaScript(oldPane, newPane *paneNode) { | |||
| 182 | 185 | if oldPane != nil && oldPane.pane != nil && oldPane.pane.webView != nil { | |
| 183 | 186 | // Update WebView internal active state | |
| 184 | 187 | oldPane.pane.webView.SetActive(false) | |
| 185 | - | ||
| 188 | + | ||
| 186 | 189 | oldDetail := map[string]any{ | |
| 187 | 190 | "active": false, | |
| 188 | 191 | "paneId": fm.getPaneID(oldPane), | |
| @@ -196,7 +199,7 @@ func (fm *FocusManager) notifyJavaScript(oldPane, newPane *paneNode) { | |||
| 196 | 199 | if newPane != nil && newPane.pane != nil && newPane.pane.webView != nil { | |
| 197 | 200 | // Update WebView internal active state | |
| 198 | 201 | newPane.pane.webView.SetActive(true) | |
| 199 | - | ||
| 202 | + | ||
| 200 | 203 | newDetail := map[string]any{ | |
| 201 | 204 | "active": true, | |
| 202 | 205 | "paneId": fm.getPaneID(newPane), | |
| @@ -233,20 +236,9 @@ func (fm *FocusManager) isValidNode(node *paneNode) bool { | |||
| 233 | 236 | ||
| 234 | 237 | // findStackContainer finds the stack container that contains this node | |
| 235 | 238 | func (fm *FocusManager) findStackContainer(node *paneNode) *paneNode { | |
| 236 | - | // Traverse up the tree looking for a stack container | |
| 237 | - | current := node | |
| 238 | - | for current != nil { | |
| 239 | - | if current.stackedPanes != nil { | |
| 240 | - | // This is a stack container, check if it contains our node | |
| 241 | - | for _, stackedPane := range current.stackedPanes { | |
| 242 | - | if stackedPane == node { | |
| 243 | - | return current | |
| 244 | - | } | |
| 245 | - | } | |
| 246 | - | } | |
| 247 | - | // Move up the tree (we'd need parent pointers for this) | |
| 248 | - | // For now, traverse the entire tree | |
| 249 | - | break | |
| 239 | + | // Check if the immediate parent is a stack container | |
| 240 | + | if node != nil && node.parent != nil && node.parent.isStacked { | |
| 241 | + | return node.parent | |
| 250 | 242 | } | |
| 251 | 243 | return nil | |
| 252 | 244 | } | |
| @@ -64,7 +64,7 @@ func (fm *FocusManager) SetActivePane(node *paneNode) { | |||||
| 64 | fm.setGTKFocus(node) | 64 | fm.setGTKFocus(node) | ||
| 65 | 65 | ||||
| 66 | // Step 2: Update visual indicators | 66 | // Step 2: Update visual indicators | ||
| 67 | - | fm.updateVisualState(oldPane, node) | 67 | + | fm.updateVisualState(node) |
| 68 | 68 | ||||
| 69 | // Step 3: Notify JavaScript bridge | 69 | // Step 3: Notify JavaScript bridge | ||
| 70 | fm.notifyJavaScript(oldPane, node) | 70 | fm.notifyJavaScript(oldPane, node) | ||
| @@ -106,17 +106,20 @@ func (fm *FocusManager) setGTKFocus(node *paneNode) { | |||||
| 106 | } | 106 | } | ||
| 107 | 107 | ||||
| 108 | // updateVisualState manages CSS classes and visibility | 108 | // updateVisualState manages CSS classes and visibility | ||
| 109 | - | func (fm *FocusManager) updateVisualState(oldPane, newPane *paneNode) { | 109 | + | func (fm *FocusManager) updateVisualState(newPane *paneNode) { |
| 110 | activePaneClass := "workspace-pane-active" | 110 | activePaneClass := "workspace-pane-active" | ||
| 111 | 111 | ||||
| 112 | - | // Remove active class from old pane | 112 | + | // ROBUST: Remove active class from ALL leaf panes to prevent multiple active states |
| 113 | - | if oldPane != nil && oldPane.container != 0 { | 113 | + | // This ensures we never have a race condition or missed cleanup |
| 114 | - | webkit.WidgetRemoveCSSClass(oldPane.container, activePaneClass) | 114 | + | leaves := fm.wm.collectLeaves() |
| 115 | - | webkit.WidgetQueueDraw(oldPane.container) | 115 | + | for _, leaf := range leaves { |
| 116 | + | if leaf != nil && leaf.container != 0 { | |||
| 117 | + | webkit.WidgetRemoveCSSClass(leaf.container, activePaneClass) | |||
| 118 | + | } | |||
| 116 | } | 119 | } | ||
| 117 | 120 | ||||
| 118 | - | // Add active class to new pane | 121 | + | // Add active class to new pane only |
| 119 | - | if newPane.container != 0 { | 122 | + | if newPane != nil && newPane.container != 0 { |
| 120 | webkit.WidgetAddCSSClass(newPane.container, activePaneClass) | 123 | webkit.WidgetAddCSSClass(newPane.container, activePaneClass) | ||
| 121 | webkit.WidgetQueueDraw(newPane.container) | 124 | webkit.WidgetQueueDraw(newPane.container) | ||
| 122 | } | 125 | } | ||
| @@ -182,7 +185,7 @@ func (fm *FocusManager) notifyJavaScript(oldPane, newPane *paneNode) { | |||||
| 182 | if oldPane != nil && oldPane.pane != nil && oldPane.pane.webView != nil { | 185 | if oldPane != nil && oldPane.pane != nil && oldPane.pane.webView != nil { | ||
| 183 | // Update WebView internal active state | 186 | // Update WebView internal active state | ||
| 184 | oldPane.pane.webView.SetActive(false) | 187 | oldPane.pane.webView.SetActive(false) | ||
| 185 | - | 188 | + | ||
| 186 | oldDetail := map[string]any{ | 189 | oldDetail := map[string]any{ | ||
| 187 | "active": false, | 190 | "active": false, | ||
| 188 | "paneId": fm.getPaneID(oldPane), | 191 | "paneId": fm.getPaneID(oldPane), | ||
| @@ -196,7 +199,7 @@ func (fm *FocusManager) notifyJavaScript(oldPane, newPane *paneNode) { | |||||
| 196 | if newPane != nil && newPane.pane != nil && newPane.pane.webView != nil { | 199 | if newPane != nil && newPane.pane != nil && newPane.pane.webView != nil { | ||
| 197 | // Update WebView internal active state | 200 | // Update WebView internal active state | ||
| 198 | newPane.pane.webView.SetActive(true) | 201 | newPane.pane.webView.SetActive(true) | ||
| 199 | - | 202 | + | ||
| 200 | newDetail := map[string]any{ | 203 | newDetail := map[string]any{ | ||
| 201 | "active": true, | 204 | "active": true, | ||
| 202 | "paneId": fm.getPaneID(newPane), | 205 | "paneId": fm.getPaneID(newPane), | ||
| @@ -233,20 +236,9 @@ func (fm *FocusManager) isValidNode(node *paneNode) bool { | |||||
| 233 | 236 | ||||
| 234 | // findStackContainer finds the stack container that contains this node | 237 | // findStackContainer finds the stack container that contains this node | ||
| 235 | func (fm *FocusManager) findStackContainer(node *paneNode) *paneNode { | 238 | func (fm *FocusManager) findStackContainer(node *paneNode) *paneNode { | ||
| 236 | - | // Traverse up the tree looking for a stack container | 239 | + | // Check if the immediate parent is a stack container |
| 237 | - | current := node | 240 | + | if node != nil && node.parent != nil && node.parent.isStacked { |
| 238 | - | for current != nil { | 241 | + | return node.parent |
| 239 | - | if current.stackedPanes != nil { | |||
| 240 | - | // This is a stack container, check if it contains our node | |||
| 241 | - | for _, stackedPane := range current.stackedPanes { | |||
| 242 | - | if stackedPane == node { | |||
| 243 | - | return current | |||
| 244 | - | } | |||
| 245 | - | } | |||
| 246 | - | } | |||
| 247 | - | // Move up the tree (we'd need parent pointers for this) | |||
| 248 | - | // For now, traverse the entire tree | |||
| 249 | - | break | |||
| 250 | } | 242 | } | ||
| 251 | return nil | 243 | return nil | ||
| 252 | } | 244 | } | ||
M internal/app/browser/pane.go +18 -0
| @@ -125,3 +125,21 @@ func (p *BrowserPane) Cleanup() { | |||
| 125 | 125 | p.guiComponents = nil | |
| 126 | 126 | p.hasGUI = false | |
| 127 | 127 | } | |
| 128 | + | ||
| 129 | + | // CleanupFromWorkspace removes this pane from workspace tracking maps and app.panes slice | |
| 130 | + | // This should be called before Cleanup() to ensure proper workspace state management | |
| 131 | + | func (p *BrowserPane) CleanupFromWorkspace(wm interface{}) { | |
| 132 | + | if p == nil || p.webView == nil { | |
| 133 | + | return | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | // Type assert to avoid circular import - workspace manager will pass itself | |
| 137 | + | if workspaceManager, ok := wm.(interface { | |
| 138 | + | removeFromMaps(*webkit.WebView) | |
| 139 | + | removeFromAppPanes(*BrowserPane) | |
| 140 | + | }); ok { | |
| 141 | + | workspaceManager.removeFromMaps(p.webView) | |
| 142 | + | workspaceManager.removeFromAppPanes(p) | |
| 143 | + | log.Printf("[pane-%s] Cleaned up from workspace tracking", p.id) | |
| 144 | + | } | |
| 145 | + | } | |
| @@ -125,3 +125,21 @@ func (p *BrowserPane) Cleanup() { | |||||
| 125 | p.guiComponents = nil | 125 | p.guiComponents = nil | ||
| 126 | p.hasGUI = false | 126 | p.hasGUI = false | ||
| 127 | } | 127 | } | ||
| 128 | + | ||||
| 129 | + | // CleanupFromWorkspace removes this pane from workspace tracking maps and app.panes slice | |||
| 130 | + | // This should be called before Cleanup() to ensure proper workspace state management | |||
| 131 | + | func (p *BrowserPane) CleanupFromWorkspace(wm interface{}) { | |||
| 132 | + | if p == nil || p.webView == nil { | |||
| 133 | + | return | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | // Type assert to avoid circular import - workspace manager will pass itself | |||
| 137 | + | if workspaceManager, ok := wm.(interface { | |||
| 138 | + | removeFromMaps(*webkit.WebView) | |||
| 139 | + | removeFromAppPanes(*BrowserPane) | |||
| 140 | + | }); ok { | |||
| 141 | + | workspaceManager.removeFromMaps(p.webView) | |||
| 142 | + | workspaceManager.removeFromAppPanes(p) | |||
| 143 | + | log.Printf("[pane-%s] Cleaned up from workspace tracking", p.id) | |||
| 144 | + | } | |||
| 145 | + | } | |||
M internal/app/browser/stacked_panes.go +13 -21
| @@ -5,7 +5,6 @@ import ( | |||
| 5 | 5 | "fmt" | |
| 6 | 6 | "log" | |
| 7 | 7 | "time" | |
| 8 | - | "unsafe" | |
| 9 | 8 | ||
| 10 | 9 | "github.com/bnema/dumber/pkg/webkit" | |
| 11 | 10 | ) | |
| @@ -32,7 +31,7 @@ func (spm *StackedPaneManager) StackPane(target *paneNode) (*paneNode, error) { | |||
| 32 | 31 | // Create the new pane first | |
| 33 | 32 | newLeaf, err := spm.prepareNewStackedPane() | |
| 34 | 33 | if err != nil { | |
| 35 | - | return nil, fmt.Errorf("failed to prepare new stacked pane: %v", err) | |
| 34 | + | return nil, fmt.Errorf("failed to prepare new stacked pane: %w", err) | |
| 36 | 35 | } | |
| 37 | 36 | ||
| 38 | 37 | var stackNode *paneNode | |
| @@ -43,13 +42,13 @@ func (spm *StackedPaneManager) StackPane(target *paneNode) (*paneNode, error) { | |||
| 43 | 42 | // Target is already in a stack - add to existing stack | |
| 44 | 43 | stackNode, insertIndex, err = spm.addPaneToExistingStack(target, newLeaf) | |
| 45 | 44 | if err != nil { | |
| 46 | - | return nil, fmt.Errorf("failed to add pane to existing stack: %v", err) | |
| 45 | + | return nil, fmt.Errorf("failed to add pane to existing stack: %w", err) | |
| 47 | 46 | } | |
| 48 | 47 | } else { | |
| 49 | 48 | // Target is not stacked - create initial stack | |
| 50 | 49 | stackNode, insertIndex, err = spm.convertToStackedContainer(target, newLeaf) | |
| 51 | 50 | if err != nil { | |
| 52 | - | return nil, fmt.Errorf("failed to convert to stacked container: %v", err) | |
| 51 | + | return nil, fmt.Errorf("failed to convert to stacked container: %w", err) | |
| 53 | 52 | } | |
| 54 | 53 | } | |
| 55 | 54 | ||
| @@ -150,7 +149,7 @@ func (spm *StackedPaneManager) addPaneToExistingStack(target, newLeaf *paneNode) | |||
| 150 | 149 | webkit.BoxInsertChildAfter(stackNode.stackWrapper, newLeaf.titleBar, insertAfterWidget) | |
| 151 | 150 | // Insert container after the newly inserted titleBar | |
| 152 | 151 | webkit.BoxInsertChildAfter(stackNode.stackWrapper, newLeaf.container, newLeaf.titleBar) | |
| 153 | - | log.Printf("[workspace] inserted widgets at position %d (after widget %p)", insertIndex, insertAfterWidget) | |
| 152 | + | log.Printf("[workspace] inserted widgets at position %d (after widget %#x)", insertIndex, insertAfterWidget) | |
| 154 | 153 | } else { | |
| 155 | 154 | // Insert at the beginning (insertIndex = 0) | |
| 156 | 155 | webkit.BoxPrepend(stackNode.stackWrapper, newLeaf.container) | |
| @@ -172,6 +171,7 @@ func (spm *StackedPaneManager) convertToStackedContainer(target, newLeaf *paneNo | |||
| 172 | 171 | } | |
| 173 | 172 | webkit.WidgetSetHExpand(stackWrapperContainer, true) | |
| 174 | 173 | webkit.WidgetSetVExpand(stackWrapperContainer, true) | |
| 174 | + | webkit.WidgetRealizeInContainer(stackWrapperContainer) // Ensures size request is set like regular panes | |
| 175 | 175 | ||
| 176 | 176 | // Create the internal box for the actual stacked widgets (titles + webviews) | |
| 177 | 177 | stackInternalBox := webkit.NewBox(webkit.OrientationVertical, 0) | |
| @@ -551,7 +551,7 @@ func (spm *StackedPaneManager) updateTitleBarLabel(node *paneNode, title string) | |||
| 551 | 551 | } | |
| 552 | 552 | ||
| 553 | 553 | // Create new title bar with updated title | |
| 554 | - | newTitleBar := spm.createTitleBarWithTitle(node, displayTitle) | |
| 554 | + | newTitleBar := spm.createTitleBarWithTitle(displayTitle) | |
| 555 | 555 | if newTitleBar == 0 { | |
| 556 | 556 | log.Printf("[workspace] failed to create new title bar") | |
| 557 | 557 | return | |
| @@ -589,7 +589,7 @@ func (spm *StackedPaneManager) updateTitleBarLabel(node *paneNode, title string) | |||
| 589 | 589 | } | |
| 590 | 590 | ||
| 591 | 591 | // createTitleBarWithTitle creates a title bar with a specific title | |
| 592 | - | func (spm *StackedPaneManager) createTitleBarWithTitle(pane *paneNode, title string) uintptr { | |
| 592 | + | func (spm *StackedPaneManager) createTitleBarWithTitle(title string) uintptr { | |
| 593 | 593 | titleBox := webkit.NewBox(webkit.OrientationHorizontal, 8) | |
| 594 | 594 | if titleBox == 0 { | |
| 595 | 595 | return 0 | |
| @@ -641,15 +641,7 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||
| 641 | 641 | ||
| 642 | 642 | // Clean up the pane | |
| 643 | 643 | spm.wm.detachHover(node) | |
| 644 | - | delete(spm.wm.viewToNode, node.pane.webView) | |
| 645 | - | ||
| 646 | - | // Remove from app.panes | |
| 647 | - | for i, pane := range spm.wm.app.panes { | |
| 648 | - | if pane == node.pane { | |
| 649 | - | spm.wm.app.panes = append(spm.wm.app.panes[:i], spm.wm.app.panes[i+1:]...) | |
| 650 | - | break | |
| 651 | - | } | |
| 652 | - | } | |
| 644 | + | node.pane.CleanupFromWorkspace(spm.wm) | |
| 653 | 645 | ||
| 654 | 646 | // Remove the pane's widgets from the stack container | |
| 655 | 647 | stackBox := stackNode.stackWrapper | |
| @@ -723,7 +715,7 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||
| 723 | 715 | // Check if currentlyFocused is part of this stack (including the pane being closed) | |
| 724 | 716 | shouldFocus := false | |
| 725 | 717 | if spm.wm.currentlyFocused != nil { | |
| 726 | - | log.Printf("[workspace] DEBUG: currentlyFocused=%p, node being closed=%p, lastPane=%p", | |
| 718 | + | log.Printf("[workspace] DEBUG: currentlyFocused=%p, node being closed=%p, lastPane=%p", | |
| 727 | 719 | spm.wm.currentlyFocused, node, lastPane) | |
| 728 | 720 | // Check if currentlyFocused is the pane being closed | |
| 729 | 721 | if spm.wm.currentlyFocused == node { | |
| @@ -742,8 +734,8 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||
| 742 | 734 | // Log widget state before focus operations | |
| 743 | 735 | if lastPaneWidget := lastPane.pane.webView.Widget(); lastPaneWidget != 0 { | |
| 744 | 736 | visible := webkit.WidgetGetVisible(lastPaneWidget) | |
| 745 | - | log.Printf("[workspace] Widget state before focus: widget=%p visible=%v", | |
| 746 | - | unsafe.Pointer(lastPaneWidget), visible) | |
| 737 | + | log.Printf("[workspace] Widget state before focus: widget=%#x visible=%v", | |
| 738 | + | lastPaneWidget, visible) | |
| 747 | 739 | } | |
| 748 | 740 | ||
| 749 | 741 | // Update CSS classes (mirror finalizeStackCreation) | |
| @@ -764,8 +756,8 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||
| 764 | 756 | ||
| 765 | 757 | // Log widget state after focus operations | |
| 766 | 758 | visible := webkit.WidgetGetVisible(lastPaneWidget) | |
| 767 | - | log.Printf("[workspace] Widget state after focus: widget=%p visible=%v", | |
| 768 | - | unsafe.Pointer(lastPaneWidget), visible) | |
| 759 | + | log.Printf("[workspace] Widget state after focus: widget=%#x visible=%v", | |
| 760 | + | lastPaneWidget, visible) | |
| 769 | 761 | } | |
| 770 | 762 | ||
| 771 | 763 | log.Printf("[workspace] DEBUG: applied finalizeStackCreation focus logic in reverse") | |
| @@ -5,7 +5,6 @@ import ( | |||||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "log" | 6 | "log" | ||
| 7 | "time" | 7 | "time" | ||
| 8 | - | "unsafe" | |||
| 9 | 8 | ||||
| 10 | "github.com/bnema/dumber/pkg/webkit" | 9 | "github.com/bnema/dumber/pkg/webkit" | ||
| 11 | ) | 10 | ) | ||
| @@ -32,7 +31,7 @@ func (spm *StackedPaneManager) StackPane(target *paneNode) (*paneNode, error) { | |||||
| 32 | // Create the new pane first | 31 | // Create the new pane first | ||
| 33 | newLeaf, err := spm.prepareNewStackedPane() | 32 | newLeaf, err := spm.prepareNewStackedPane() | ||
| 34 | if err != nil { | 33 | if err != nil { | ||
| 35 | - | return nil, fmt.Errorf("failed to prepare new stacked pane: %v", err) | 34 | + | return nil, fmt.Errorf("failed to prepare new stacked pane: %w", err) |
| 36 | } | 35 | } | ||
| 37 | 36 | ||||
| 38 | var stackNode *paneNode | 37 | var stackNode *paneNode | ||
| @@ -43,13 +42,13 @@ func (spm *StackedPaneManager) StackPane(target *paneNode) (*paneNode, error) { | |||||
| 43 | // Target is already in a stack - add to existing stack | 42 | // Target is already in a stack - add to existing stack | ||
| 44 | stackNode, insertIndex, err = spm.addPaneToExistingStack(target, newLeaf) | 43 | stackNode, insertIndex, err = spm.addPaneToExistingStack(target, newLeaf) | ||
| 45 | if err != nil { | 44 | if err != nil { | ||
| 46 | - | return nil, fmt.Errorf("failed to add pane to existing stack: %v", err) | 45 | + | return nil, fmt.Errorf("failed to add pane to existing stack: %w", err) |
| 47 | } | 46 | } | ||
| 48 | } else { | 47 | } else { | ||
| 49 | // Target is not stacked - create initial stack | 48 | // Target is not stacked - create initial stack | ||
| 50 | stackNode, insertIndex, err = spm.convertToStackedContainer(target, newLeaf) | 49 | stackNode, insertIndex, err = spm.convertToStackedContainer(target, newLeaf) | ||
| 51 | if err != nil { | 50 | if err != nil { | ||
| 52 | - | return nil, fmt.Errorf("failed to convert to stacked container: %v", err) | 51 | + | return nil, fmt.Errorf("failed to convert to stacked container: %w", err) |
| 53 | } | 52 | } | ||
| 54 | } | 53 | } | ||
| 55 | 54 | ||||
| @@ -150,7 +149,7 @@ func (spm *StackedPaneManager) addPaneToExistingStack(target, newLeaf *paneNode) | |||||
| 150 | webkit.BoxInsertChildAfter(stackNode.stackWrapper, newLeaf.titleBar, insertAfterWidget) | 149 | webkit.BoxInsertChildAfter(stackNode.stackWrapper, newLeaf.titleBar, insertAfterWidget) | ||
| 151 | // Insert container after the newly inserted titleBar | 150 | // Insert container after the newly inserted titleBar | ||
| 152 | webkit.BoxInsertChildAfter(stackNode.stackWrapper, newLeaf.container, newLeaf.titleBar) | 151 | webkit.BoxInsertChildAfter(stackNode.stackWrapper, newLeaf.container, newLeaf.titleBar) | ||
| 153 | - | log.Printf("[workspace] inserted widgets at position %d (after widget %p)", insertIndex, insertAfterWidget) | 152 | + | log.Printf("[workspace] inserted widgets at position %d (after widget %#x)", insertIndex, insertAfterWidget) |
| 154 | } else { | 153 | } else { | ||
| 155 | // Insert at the beginning (insertIndex = 0) | 154 | // Insert at the beginning (insertIndex = 0) | ||
| 156 | webkit.BoxPrepend(stackNode.stackWrapper, newLeaf.container) | 155 | webkit.BoxPrepend(stackNode.stackWrapper, newLeaf.container) | ||
| @@ -172,6 +171,7 @@ func (spm *StackedPaneManager) convertToStackedContainer(target, newLeaf *paneNo | |||||
| 172 | } | 171 | } | ||
| 173 | webkit.WidgetSetHExpand(stackWrapperContainer, true) | 172 | webkit.WidgetSetHExpand(stackWrapperContainer, true) | ||
| 174 | webkit.WidgetSetVExpand(stackWrapperContainer, true) | 173 | webkit.WidgetSetVExpand(stackWrapperContainer, true) | ||
| 174 | + | webkit.WidgetRealizeInContainer(stackWrapperContainer) // Ensures size request is set like regular panes | |||
| 175 | 175 | ||||
| 176 | // Create the internal box for the actual stacked widgets (titles + webviews) | 176 | // Create the internal box for the actual stacked widgets (titles + webviews) | ||
| 177 | stackInternalBox := webkit.NewBox(webkit.OrientationVertical, 0) | 177 | stackInternalBox := webkit.NewBox(webkit.OrientationVertical, 0) | ||
| @@ -551,7 +551,7 @@ func (spm *StackedPaneManager) updateTitleBarLabel(node *paneNode, title string) | |||||
| 551 | } | 551 | } | ||
| 552 | 552 | ||||
| 553 | // Create new title bar with updated title | 553 | // Create new title bar with updated title | ||
| 554 | - | newTitleBar := spm.createTitleBarWithTitle(node, displayTitle) | 554 | + | newTitleBar := spm.createTitleBarWithTitle(displayTitle) |
| 555 | if newTitleBar == 0 { | 555 | if newTitleBar == 0 { | ||
| 556 | log.Printf("[workspace] failed to create new title bar") | 556 | log.Printf("[workspace] failed to create new title bar") | ||
| 557 | return | 557 | return | ||
| @@ -589,7 +589,7 @@ func (spm *StackedPaneManager) updateTitleBarLabel(node *paneNode, title string) | |||||
| 589 | } | 589 | } | ||
| 590 | 590 | ||||
| 591 | // createTitleBarWithTitle creates a title bar with a specific title | 591 | // createTitleBarWithTitle creates a title bar with a specific title | ||
| 592 | - | func (spm *StackedPaneManager) createTitleBarWithTitle(pane *paneNode, title string) uintptr { | 592 | + | func (spm *StackedPaneManager) createTitleBarWithTitle(title string) uintptr { |
| 593 | titleBox := webkit.NewBox(webkit.OrientationHorizontal, 8) | 593 | titleBox := webkit.NewBox(webkit.OrientationHorizontal, 8) | ||
| 594 | if titleBox == 0 { | 594 | if titleBox == 0 { | ||
| 595 | return 0 | 595 | return 0 | ||
| @@ -641,15 +641,7 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||||
| 641 | 641 | ||||
| 642 | // Clean up the pane | 642 | // Clean up the pane | ||
| 643 | spm.wm.detachHover(node) | 643 | spm.wm.detachHover(node) | ||
| 644 | - | delete(spm.wm.viewToNode, node.pane.webView) | 644 | + | node.pane.CleanupFromWorkspace(spm.wm) |
| 645 | - | ||||
| 646 | - | // Remove from app.panes | |||
| 647 | - | for i, pane := range spm.wm.app.panes { | |||
| 648 | - | if pane == node.pane { | |||
| 649 | - | spm.wm.app.panes = append(spm.wm.app.panes[:i], spm.wm.app.panes[i+1:]...) | |||
| 650 | - | break | |||
| 651 | - | } | |||
| 652 | - | } | |||
| 653 | 645 | ||||
| 654 | // Remove the pane's widgets from the stack container | 646 | // Remove the pane's widgets from the stack container | ||
| 655 | stackBox := stackNode.stackWrapper | 647 | stackBox := stackNode.stackWrapper | ||
| @@ -723,7 +715,7 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||||
| 723 | // Check if currentlyFocused is part of this stack (including the pane being closed) | 715 | // Check if currentlyFocused is part of this stack (including the pane being closed) | ||
| 724 | shouldFocus := false | 716 | shouldFocus := false | ||
| 725 | if spm.wm.currentlyFocused != nil { | 717 | if spm.wm.currentlyFocused != nil { | ||
| 726 | - | log.Printf("[workspace] DEBUG: currentlyFocused=%p, node being closed=%p, lastPane=%p", | 718 | + | log.Printf("[workspace] DEBUG: currentlyFocused=%p, node being closed=%p, lastPane=%p", |
| 727 | spm.wm.currentlyFocused, node, lastPane) | 719 | spm.wm.currentlyFocused, node, lastPane) | ||
| 728 | // Check if currentlyFocused is the pane being closed | 720 | // Check if currentlyFocused is the pane being closed | ||
| 729 | if spm.wm.currentlyFocused == node { | 721 | if spm.wm.currentlyFocused == node { | ||
| @@ -742,8 +734,8 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||||
| 742 | // Log widget state before focus operations | 734 | // Log widget state before focus operations | ||
| 743 | if lastPaneWidget := lastPane.pane.webView.Widget(); lastPaneWidget != 0 { | 735 | if lastPaneWidget := lastPane.pane.webView.Widget(); lastPaneWidget != 0 { | ||
| 744 | visible := webkit.WidgetGetVisible(lastPaneWidget) | 736 | visible := webkit.WidgetGetVisible(lastPaneWidget) | ||
| 745 | - | log.Printf("[workspace] Widget state before focus: widget=%p visible=%v", | 737 | + | log.Printf("[workspace] Widget state before focus: widget=%#x visible=%v", |
| 746 | - | unsafe.Pointer(lastPaneWidget), visible) | 738 | + | lastPaneWidget, visible) |
| 747 | } | 739 | } | ||
| 748 | 740 | ||||
| 749 | // Update CSS classes (mirror finalizeStackCreation) | 741 | // Update CSS classes (mirror finalizeStackCreation) | ||
| @@ -764,8 +756,8 @@ func (spm *StackedPaneManager) CloseStackedPane(node *paneNode) error { | |||||
| 764 | 756 | ||||
| 765 | // Log widget state after focus operations | 757 | // Log widget state after focus operations | ||
| 766 | visible := webkit.WidgetGetVisible(lastPaneWidget) | 758 | visible := webkit.WidgetGetVisible(lastPaneWidget) | ||
| 767 | - | log.Printf("[workspace] Widget state after focus: widget=%p visible=%v", | 759 | + | log.Printf("[workspace] Widget state after focus: widget=%#x visible=%v", |
| 768 | - | unsafe.Pointer(lastPaneWidget), visible) | 760 | + | lastPaneWidget, visible) |
| 769 | } | 761 | } | ||
| 770 | 762 | ||||
| 771 | log.Printf("[workspace] DEBUG: applied finalizeStackCreation focus logic in reverse") | 763 | log.Printf("[workspace] DEBUG: applied finalizeStackCreation focus logic in reverse") | ||
M internal/app/browser/workspace_manager.go +89 -202
| @@ -77,9 +77,8 @@ type WorkspaceManager struct { | |||
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | 79 | const ( | |
| 80 | - | activePaneClass = "workspace-pane-active" | |
| 81 | - | basePaneClass = "workspace-pane" | |
| 82 | - | multiPaneClass = "workspace-multi-pane" | |
| 80 | + | basePaneClass = "workspace-pane" | |
| 81 | + | multiPaneClass = "workspace-multi-pane" | |
| 83 | 82 | ) | |
| 84 | 83 | ||
| 85 | 84 | // Workspace navigation shortcuts are now handled globally by WindowShortcutHandler | |
| @@ -976,10 +975,14 @@ func (wm *WorkspaceManager) collectLeavesFrom(node *paneNode) []*paneNode { | |||
| 976 | 975 | return | |
| 977 | 976 | } | |
| 978 | 977 | ||
| 979 | - | // Handle stacked panes - traverse the stacked panes list | |
| 978 | + | // Handle stacked panes - only include the currently ACTIVE pane as a leaf | |
| 979 | + | // This prevents multiple CSS classes and ensures correct focus management | |
| 980 | 980 | if n.isStacked && len(n.stackedPanes) > 0 { | |
| 981 | - | for _, stackedPane := range n.stackedPanes { | |
| 982 | - | walk(stackedPane, depth+1) | |
| 981 | + | activeIndex := n.activeStackIndex | |
| 982 | + | if activeIndex >= 0 && activeIndex < len(n.stackedPanes) { | |
| 983 | + | // Only the active pane in the stack counts as a leaf for focus/navigation | |
| 984 | + | activePaneInStack := n.stackedPanes[activeIndex] | |
| 985 | + | walk(activePaneInStack, depth+1) | |
| 983 | 986 | } | |
| 984 | 987 | return | |
| 985 | 988 | } | |
| @@ -1071,8 +1074,11 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 1071 | 1074 | webkit.WidgetSetVExpand(splitTargetContainer, true) | |
| 1072 | 1075 | webkit.WidgetRealizeInContainer(splitTargetContainer) | |
| 1073 | 1076 | } else if splitTarget.isStacked { | |
| 1074 | - | // For stacked containers, the wrapper should already be properly configured | |
| 1075 | - | log.Printf("[workspace] skipping GTK setup for stack wrapper: %#x", splitTargetContainer) | |
| 1077 | + | // Stack containers need the same setup as regular panes for proper splitting | |
| 1078 | + | webkit.WidgetSetHExpand(splitTargetContainer, true) | |
| 1079 | + | webkit.WidgetSetVExpand(splitTargetContainer, true) | |
| 1080 | + | webkit.WidgetRealizeInContainer(splitTargetContainer) | |
| 1081 | + | log.Printf("[workspace] configured stack wrapper for split: %#x", splitTargetContainer) | |
| 1076 | 1082 | } | |
| 1077 | 1083 | ||
| 1078 | 1084 | orientation, existingFirst := mapDirection(direction) | |
| @@ -1184,10 +1190,10 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 1184 | 1190 | wm.viewToNode[newPane.webView] = newLeaf | |
| 1185 | 1191 | wm.ensureHover(newLeaf) | |
| 1186 | 1192 | ||
| 1187 | - | // For stacked panes, we need to ensure hover on the original target, not the stack container | |
| 1188 | - | if originalTarget != target { | |
| 1189 | - | wm.ensureHover(originalTarget) | |
| 1190 | - | } else { | |
| 1193 | + | // Don't ensure hover on originalTarget when splitting from stack - it competes with new pane focus | |
| 1194 | + | // The originalTarget is still in the stack and should not interfere with the new split pane | |
| 1195 | + | if originalTarget == target { | |
| 1196 | + | // Only ensure hover on target if it's a regular (non-stack) split | |
| 1191 | 1197 | wm.ensureHover(target) | |
| 1192 | 1198 | } | |
| 1193 | 1199 | ||
| @@ -1338,19 +1344,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1338 | 1344 | wm.detachHover(node) | |
| 1339 | 1345 | wm.detachHover(sibling) | |
| 1340 | 1346 | ||
| 1341 | - | delete(wm.viewToNode, node.pane.webView) | |
| 1342 | - | delete(wm.viewToNode, sibling.pane.webView) | |
| 1343 | - | delete(wm.lastSplitMsg, node.pane.webView) | |
| 1344 | - | delete(wm.lastSplitMsg, sibling.pane.webView) | |
| 1345 | - | delete(wm.lastExitMsg, node.pane.webView) | |
| 1346 | - | delete(wm.lastExitMsg, sibling.pane.webView) | |
| 1347 | - | ||
| 1348 | - | for i := 0; i < len(wm.app.panes); i++ { | |
| 1349 | - | if wm.app.panes[i] == node.pane || wm.app.panes[i] == sibling.pane { | |
| 1350 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |
| 1351 | - | i-- | |
| 1352 | - | } | |
| 1353 | - | } | |
| 1347 | + | // Clean up workspace tracking for both panes | |
| 1348 | + | node.pane.CleanupFromWorkspace(wm) | |
| 1349 | + | sibling.pane.CleanupFromWorkspace(wm) | |
| 1354 | 1350 | ||
| 1355 | 1351 | if node.pane.webView != nil { | |
| 1356 | 1352 | if err := node.pane.webView.Destroy(); err != nil { | |
| @@ -1407,17 +1403,8 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1407 | 1403 | // We have a replacement - delegate root status and close this pane | |
| 1408 | 1404 | log.Printf("[workspace] delegating root status from node=%#x to replacement=%#x", node.container, replacement.container) | |
| 1409 | 1405 | ||
| 1410 | - | // Clean up references first | |
| 1411 | - | delete(wm.viewToNode, node.pane.webView) | |
| 1412 | - | delete(wm.lastSplitMsg, node.pane.webView) | |
| 1413 | - | delete(wm.lastExitMsg, node.pane.webView) | |
| 1414 | - | ||
| 1415 | - | for i, pane := range wm.app.panes { | |
| 1416 | - | if pane == node.pane { | |
| 1417 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |
| 1418 | - | break | |
| 1419 | - | } | |
| 1420 | - | } | |
| 1406 | + | // Clean up workspace tracking | |
| 1407 | + | node.pane.CleanupFromWorkspace(wm) | |
| 1421 | 1408 | ||
| 1422 | 1409 | if wm.mainPane == node { | |
| 1423 | 1410 | wm.mainPane = nil | |
| @@ -1493,25 +1480,19 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1493 | 1480 | focusTarget = wm.leftmostLeaf(wm.root) | |
| 1494 | 1481 | } | |
| 1495 | 1482 | ||
| 1496 | - | // Clean up references first | |
| 1497 | - | delete(wm.viewToNode, node.pane.webView) | |
| 1498 | - | delete(wm.lastSplitMsg, node.pane.webView) | |
| 1499 | - | delete(wm.lastExitMsg, node.pane.webView) | |
| 1500 | - | ||
| 1501 | - | for i, pane := range wm.app.panes { | |
| 1502 | - | if pane == node.pane { | |
| 1503 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |
| 1504 | - | break | |
| 1505 | - | } | |
| 1506 | - | } | |
| 1483 | + | // Clean up workspace tracking | |
| 1484 | + | node.pane.CleanupFromWorkspace(wm) | |
| 1507 | 1485 | ||
| 1508 | 1486 | if wm.mainPane == node { | |
| 1509 | 1487 | wm.mainPane = nil | |
| 1510 | 1488 | } | |
| 1511 | 1489 | ||
| 1512 | - | // Clear current active if it's the node being closed | |
| 1490 | + | // Clear current active if it's the node being closed and immediately set focus to target | |
| 1513 | 1491 | if wm.currentlyFocused == node { | |
| 1514 | - | wm.currentlyFocused = nil | |
| 1492 | + | wm.currentlyFocused = focusTarget | |
| 1493 | + | if focusTarget != nil { | |
| 1494 | + | wm.focusManager.SetActivePane(focusTarget) | |
| 1495 | + | } | |
| 1515 | 1496 | } | |
| 1516 | 1497 | ||
| 1517 | 1498 | grand := parent.parent | |
| @@ -1519,29 +1500,46 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1519 | 1500 | // Parent is the root node. Promote sibling to become the new root. | |
| 1520 | 1501 | // The sibling can be either a leaf (when only 2 panes total) or a branch (when more panes exist) | |
| 1521 | 1502 | log.Printf("[workspace] promoting sibling to root: container=%#x, isLeaf=%v", sibling.container, sibling.isLeaf) | |
| 1503 | + | ||
| 1504 | + | // CRITICAL: Check widget validity before any GTK operations to prevent segfault | |
| 1505 | + | if !webkit.WidgetIsValid(sibling.container) { | |
| 1506 | + | log.Printf("[workspace] ERROR: sibling container %#x is not a valid GTK widget, skipping root promotion", sibling.container) | |
| 1507 | + | return errors.New("sibling container is not a valid GTK widget") | |
| 1508 | + | } | |
| 1509 | + | ||
| 1522 | 1510 | wm.root = sibling | |
| 1523 | 1511 | sibling.parent = nil | |
| 1524 | 1512 | ||
| 1525 | 1513 | // Unparent the sibling from the paned first, then set it as window child | |
| 1526 | 1514 | if wm.window != nil { | |
| 1527 | 1515 | // GTK requires explicit unparenting before reparenting | |
| 1528 | - | if parent.left == sibling { | |
| 1529 | - | webkit.PanedSetStartChild(parent.container, 0) | |
| 1530 | - | } else { | |
| 1531 | - | webkit.PanedSetEndChild(parent.container, 0) | |
| 1516 | + | if parent.container != 0 && webkit.WidgetIsValid(parent.container) { | |
| 1517 | + | if parent.left == sibling { | |
| 1518 | + | webkit.PanedSetStartChild(parent.container, 0) | |
| 1519 | + | } else { | |
| 1520 | + | webkit.PanedSetEndChild(parent.container, 0) | |
| 1521 | + | } | |
| 1532 | 1522 | } | |
| 1533 | - | // Now we can safely set it as window child | |
| 1523 | + | // Now we can safely set it as window child (widget validity already checked above) | |
| 1534 | 1524 | wm.window.SetChild(sibling.container) | |
| 1535 | - | if sibling.container != 0 { | |
| 1536 | - | webkit.WidgetQueueAllocate(sibling.container) | |
| 1537 | - | } | |
| 1525 | + | webkit.WidgetQueueAllocate(sibling.container) | |
| 1538 | 1526 | } | |
| 1539 | 1527 | } else { | |
| 1540 | 1528 | // Parent has a grandparent, so promote sibling to take parent's place | |
| 1541 | 1529 | log.Printf("[workspace] promoting sibling to parent's position: sibling=%#x grand=%#x", sibling.container, grand.container) | |
| 1542 | 1530 | ||
| 1531 | + | // CRITICAL: Check widget validity before any GTK operations to prevent segfault | |
| 1532 | + | if !webkit.WidgetIsValid(sibling.container) { | |
| 1533 | + | log.Printf("[workspace] ERROR: sibling container %#x is not a valid GTK widget, skipping promotion", sibling.container) | |
| 1534 | + | return errors.New("sibling container is not a valid GTK widget") | |
| 1535 | + | } | |
| 1536 | + | if !webkit.WidgetIsValid(grand.container) { | |
| 1537 | + | log.Printf("[workspace] ERROR: grand container %#x is not a valid GTK widget, skipping promotion", grand.container) | |
| 1538 | + | return errors.New("grand container is not a valid GTK widget") | |
| 1539 | + | } | |
| 1540 | + | ||
| 1543 | 1541 | // First unparent the sibling from its current parent to avoid GTK-CRITICAL errors | |
| 1544 | - | if parent.container != 0 && !parent.isLeaf { | |
| 1542 | + | if parent.container != 0 && !parent.isLeaf && webkit.WidgetIsValid(parent.container) { | |
| 1545 | 1543 | if parent.left == sibling { | |
| 1546 | 1544 | webkit.PanedSetStartChild(parent.container, 0) | |
| 1547 | 1545 | } else if parent.right == sibling { | |
| @@ -1553,7 +1551,7 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1553 | 1551 | // GTK4 automatically handles focus state synchronization during reparenting | |
| 1554 | 1552 | ||
| 1555 | 1553 | sibling.parent = grand | |
| 1556 | - | if grand.container != 0 && !grand.isLeaf { | |
| 1554 | + | if grand.container != 0 && !grand.isLeaf && webkit.WidgetIsValid(grand.container) && webkit.WidgetIsValid(sibling.container) { | |
| 1557 | 1555 | if grand.left == parent { | |
| 1558 | 1556 | grand.left = sibling | |
| 1559 | 1557 | webkit.PanedSetStartChild(grand.container, sibling.container) | |
| @@ -1566,7 +1564,7 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1566 | 1564 | } | |
| 1567 | 1565 | ||
| 1568 | 1566 | // Keep sibling subtree visible | |
| 1569 | - | if sibling.container != 0 { | |
| 1567 | + | if sibling.container != 0 && webkit.WidgetIsValid(sibling.container) { | |
| 1570 | 1568 | webkit.WidgetShow(sibling.container) | |
| 1571 | 1569 | } | |
| 1572 | 1570 | ||
| @@ -1575,11 +1573,7 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||
| 1575 | 1573 | parent.right = nil | |
| 1576 | 1574 | node.parent = nil | |
| 1577 | 1575 | ||
| 1578 | - | // Focus the target pane | |
| 1579 | - | if focusTarget != nil && focusTarget != node { | |
| 1580 | - | wm.currentlyFocused = focusTarget | |
| 1581 | - | wm.focusManager.SetActivePane(focusTarget) | |
| 1582 | - | } | |
| 1576 | + | // Focus handling was moved earlier to avoid state gaps | |
| 1583 | 1577 | ||
| 1584 | 1578 | // Destroy the webview and detach hover AFTER all hierarchy changes are complete | |
| 1585 | 1579 | ensureCleanup() | |
| @@ -1659,143 +1653,6 @@ func (wm *WorkspaceManager) findReplacementRoot(excludeNode *paneNode) *paneNode | |||
| 1659 | 1653 | return nil | |
| 1660 | 1654 | } | |
| 1661 | 1655 | ||
| 1662 | - | // closeStackedPane handles closing a pane that is part of a stack. | |
| 1663 | - | func (wm *WorkspaceManager) closeStackedPane(node *paneNode) error { | |
| 1664 | - | if node.parent == nil || !node.parent.isStacked { | |
| 1665 | - | return errors.New("node is not part of a stacked pane") | |
| 1666 | - | } | |
| 1667 | - | ||
| 1668 | - | stackNode := node.parent | |
| 1669 | - | ||
| 1670 | - | // Find the index of the node to be closed | |
| 1671 | - | nodeIndex := -1 | |
| 1672 | - | for i, stackedPane := range stackNode.stackedPanes { | |
| 1673 | - | if stackedPane == node { | |
| 1674 | - | nodeIndex = i | |
| 1675 | - | break | |
| 1676 | - | } | |
| 1677 | - | } | |
| 1678 | - | ||
| 1679 | - | if nodeIndex == -1 { | |
| 1680 | - | return errors.New("node not found in stack") | |
| 1681 | - | } | |
| 1682 | - | ||
| 1683 | - | log.Printf("[workspace] closing stacked pane: index=%d stackSize=%d", nodeIndex, len(stackNode.stackedPanes)) | |
| 1684 | - | ||
| 1685 | - | // Clean up the pane | |
| 1686 | - | wm.detachHover(node) | |
| 1687 | - | delete(wm.viewToNode, node.pane.webView) | |
| 1688 | - | ||
| 1689 | - | // Remove from app.panes | |
| 1690 | - | for i, pane := range wm.app.panes { | |
| 1691 | - | if pane == node.pane { | |
| 1692 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |
| 1693 | - | break | |
| 1694 | - | } | |
| 1695 | - | } | |
| 1696 | - | ||
| 1697 | - | // Remove the pane's widgets from the stack container | |
| 1698 | - | stackBox := stackNode.stackWrapper | |
| 1699 | - | if stackBox == 0 { | |
| 1700 | - | stackBox = stackNode.container | |
| 1701 | - | } | |
| 1702 | - | ||
| 1703 | - | if node.titleBar != 0 { | |
| 1704 | - | webkit.BoxRemove(stackBox, node.titleBar) | |
| 1705 | - | } | |
| 1706 | - | if node.container != 0 { | |
| 1707 | - | webkit.BoxRemove(stackBox, node.container) | |
| 1708 | - | // BoxRemove automatically unparents in GTK4, no manual unparent needed | |
| 1709 | - | } | |
| 1710 | - | ||
| 1711 | - | // Clean up the pane | |
| 1712 | - | node.pane.Cleanup() | |
| 1713 | - | ||
| 1714 | - | // Remove from the stack | |
| 1715 | - | stackNode.stackedPanes = append(stackNode.stackedPanes[:nodeIndex], stackNode.stackedPanes[nodeIndex+1:]...) | |
| 1716 | - | ||
| 1717 | - | // Handle the remaining panes in the stack | |
| 1718 | - | if len(stackNode.stackedPanes) == 0 { | |
| 1719 | - | // No panes left in stack - this should not happen normally | |
| 1720 | - | log.Printf("[workspace] warning: empty stack after closing pane") | |
| 1721 | - | return errors.New("stack became empty") | |
| 1722 | - | } else if len(stackNode.stackedPanes) == 1 { | |
| 1723 | - | // Only one pane left - convert back to a regular pane | |
| 1724 | - | lastPane := stackNode.stackedPanes[0] | |
| 1725 | - | parent := stackNode.parent | |
| 1726 | - | lastPaneContainer := lastPane.container | |
| 1727 | - | ||
| 1728 | - | // Remove the title bar since we're going back to single pane | |
| 1729 | - | if lastPane.titleBar != 0 { | |
| 1730 | - | webkit.BoxRemove(stackBox, lastPane.titleBar) | |
| 1731 | - | } | |
| 1732 | - | ||
| 1733 | - | // Unparent the pane container from stack wrapper | |
| 1734 | - | webkit.BoxRemove(stackBox, lastPaneContainer) | |
| 1735 | - | ||
| 1736 | - | // CRITICAL FIX: Replace the stackNode completely with lastPane | |
| 1737 | - | // Update parent child references FIRST | |
| 1738 | - | if parent == nil { | |
| 1739 | - | wm.root = lastPane | |
| 1740 | - | if wm.window != nil { | |
| 1741 | - | wm.window.SetChild(lastPaneContainer) | |
| 1742 | - | } | |
| 1743 | - | } else { | |
| 1744 | - | if parent.left == stackNode { | |
| 1745 | - | parent.left = lastPane | |
| 1746 | - | webkit.PanedSetStartChild(parent.container, lastPaneContainer) | |
| 1747 | - | } else if parent.right == stackNode { | |
| 1748 | - | parent.right = lastPane | |
| 1749 | - | webkit.PanedSetEndChild(parent.container, lastPaneContainer) | |
| 1750 | - | } | |
| 1751 | - | } | |
| 1752 | - | ||
| 1753 | - | // Convert lastPane back to regular pane | |
| 1754 | - | lastPane.parent = parent | |
| 1755 | - | lastPane.isStacked = false | |
| 1756 | - | lastPane.stackedPanes = nil | |
| 1757 | - | lastPane.titleBar = 0 | |
| 1758 | - | ||
| 1759 | - | // Update the viewToNode mapping to point to the lastPane, not stackNode | |
| 1760 | - | wm.viewToNode[lastPane.pane.webView] = lastPane | |
| 1761 | - | ||
| 1762 | - | // Focus the remaining pane if it was the active one being closed | |
| 1763 | - | if wm.currentlyFocused == node { | |
| 1764 | - | wm.currentlyFocused = lastPane | |
| 1765 | - | wm.focusManager.SetActivePane(lastPane) | |
| 1766 | - | } | |
| 1767 | - | ||
| 1768 | - | log.Printf("[workspace] converted single-pane stack back to regular pane") | |
| 1769 | - | } else { | |
| 1770 | - | // Multiple panes remain in stack - adjust active index | |
| 1771 | - | if stackNode.activeStackIndex >= nodeIndex && stackNode.activeStackIndex > 0 { | |
| 1772 | - | stackNode.activeStackIndex-- | |
| 1773 | - | } | |
| 1774 | - | if stackNode.activeStackIndex >= len(stackNode.stackedPanes) { | |
| 1775 | - | stackNode.activeStackIndex = len(stackNode.stackedPanes) - 1 | |
| 1776 | - | } | |
| 1777 | - | ||
| 1778 | - | // Update visibility for remaining panes | |
| 1779 | - | wm.stackedPaneManager.UpdateStackVisibility(stackNode) | |
| 1780 | - | ||
| 1781 | - | // Focus the new active pane if we closed the currently active one | |
| 1782 | - | if wm.currentlyFocused == node { | |
| 1783 | - | newActivePaneInStack := stackNode.stackedPanes[stackNode.activeStackIndex] | |
| 1784 | - | wm.currentlyFocused = newActivePaneInStack | |
| 1785 | - | wm.focusManager.SetActivePane(newActivePaneInStack) | |
| 1786 | - | } | |
| 1787 | - | ||
| 1788 | - | log.Printf("[workspace] closed pane from stack: remaining=%d activeIndex=%d", | |
| 1789 | - | len(stackNode.stackedPanes), stackNode.activeStackIndex) | |
| 1790 | - | } | |
| 1791 | - | ||
| 1792 | - | // Update CSS classes after pane count changes | |
| 1793 | - | wm.ensurePaneBaseClasses() | |
| 1794 | - | log.Printf("[workspace] stacked pane closed; panes remaining=%d", len(wm.app.panes)) | |
| 1795 | - | ||
| 1796 | - | return nil | |
| 1797 | - | } | |
| 1798 | - | ||
| 1799 | 1656 | func (wm *WorkspaceManager) updateMainPane() { | |
| 1800 | 1657 | if len(wm.app.panes) == 1 { | |
| 1801 | 1658 | if leaf := wm.viewToNode[wm.app.panes[0].webView]; leaf != nil { | |
| @@ -2586,3 +2443,33 @@ func mapDirection(direction string) (webkit.Orientation, bool) { | |||
| 2586 | 2443 | return webkit.OrientationHorizontal, true | |
| 2587 | 2444 | } | |
| 2588 | 2445 | } | |
| 2446 | + | ||
| 2447 | + | // Helper methods to support clean pane removal from workspace tracking | |
| 2448 | + | // These methods implement the interface expected by BrowserPane.CleanupFromWorkspace() | |
| 2449 | + | ||
| 2450 | + | // removeFromMaps removes a WebView from all workspace tracking maps | |
| 2451 | + | func (wm *WorkspaceManager) removeFromMaps(webView *webkit.WebView) { | |
| 2452 | + | if wm == nil || webView == nil { | |
| 2453 | + | return | |
| 2454 | + | } | |
| 2455 | + | ||
| 2456 | + | delete(wm.viewToNode, webView) | |
| 2457 | + | delete(wm.lastSplitMsg, webView) | |
| 2458 | + | delete(wm.lastExitMsg, webView) | |
| 2459 | + | log.Printf("[workspace] removed webview %p from tracking maps", webView) | |
| 2460 | + | } | |
| 2461 | + | ||
| 2462 | + | // removeFromAppPanes removes a BrowserPane from the app.panes slice | |
| 2463 | + | func (wm *WorkspaceManager) removeFromAppPanes(pane *BrowserPane) { | |
| 2464 | + | if wm == nil || wm.app == nil || pane == nil { | |
| 2465 | + | return | |
| 2466 | + | } | |
| 2467 | + | ||
| 2468 | + | for i, p := range wm.app.panes { | |
| 2469 | + | if p == pane { | |
| 2470 | + | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |
| 2471 | + | log.Printf("[workspace] removed pane from app.panes slice, remaining: %d", len(wm.app.panes)) | |
| 2472 | + | return | |
| 2473 | + | } | |
| 2474 | + | } | |
| 2475 | + | } | |
| @@ -77,9 +77,8 @@ type WorkspaceManager struct { | |||||
| 77 | } | 77 | } | ||
| 78 | 78 | ||||
| 79 | const ( | 79 | const ( | ||
| 80 | - | activePaneClass = "workspace-pane-active" | 80 | + | basePaneClass = "workspace-pane" |
| 81 | - | basePaneClass = "workspace-pane" | 81 | + | multiPaneClass = "workspace-multi-pane" |
| 82 | - | multiPaneClass = "workspace-multi-pane" | |||
| 83 | ) | 82 | ) | ||
| 84 | 83 | ||||
| 85 | // Workspace navigation shortcuts are now handled globally by WindowShortcutHandler | 84 | // Workspace navigation shortcuts are now handled globally by WindowShortcutHandler | ||
| @@ -976,10 +975,14 @@ func (wm *WorkspaceManager) collectLeavesFrom(node *paneNode) []*paneNode { | |||||
| 976 | return | 975 | return | ||
| 977 | } | 976 | } | ||
| 978 | 977 | ||||
| 979 | - | // Handle stacked panes - traverse the stacked panes list | 978 | + | // Handle stacked panes - only include the currently ACTIVE pane as a leaf |
| 979 | + | // This prevents multiple CSS classes and ensures correct focus management | |||
| 980 | if n.isStacked && len(n.stackedPanes) > 0 { | 980 | if n.isStacked && len(n.stackedPanes) > 0 { | ||
| 981 | - | for _, stackedPane := range n.stackedPanes { | 981 | + | activeIndex := n.activeStackIndex |
| 982 | - | walk(stackedPane, depth+1) | 982 | + | if activeIndex >= 0 && activeIndex < len(n.stackedPanes) { |
| 983 | + | // Only the active pane in the stack counts as a leaf for focus/navigation | |||
| 984 | + | activePaneInStack := n.stackedPanes[activeIndex] | |||
| 985 | + | walk(activePaneInStack, depth+1) | |||
| 983 | } | 986 | } | ||
| 984 | return | 987 | return | ||
| 985 | } | 988 | } | ||
| @@ -1071,8 +1074,11 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 1071 | webkit.WidgetSetVExpand(splitTargetContainer, true) | 1074 | webkit.WidgetSetVExpand(splitTargetContainer, true) | ||
| 1072 | webkit.WidgetRealizeInContainer(splitTargetContainer) | 1075 | webkit.WidgetRealizeInContainer(splitTargetContainer) | ||
| 1073 | } else if splitTarget.isStacked { | 1076 | } else if splitTarget.isStacked { | ||
| 1074 | - | // For stacked containers, the wrapper should already be properly configured | 1077 | + | // Stack containers need the same setup as regular panes for proper splitting |
| 1075 | - | log.Printf("[workspace] skipping GTK setup for stack wrapper: %#x", splitTargetContainer) | 1078 | + | webkit.WidgetSetHExpand(splitTargetContainer, true) |
| 1079 | + | webkit.WidgetSetVExpand(splitTargetContainer, true) | |||
| 1080 | + | webkit.WidgetRealizeInContainer(splitTargetContainer) | |||
| 1081 | + | log.Printf("[workspace] configured stack wrapper for split: %#x", splitTargetContainer) | |||
| 1076 | } | 1082 | } | ||
| 1077 | 1083 | ||||
| 1078 | orientation, existingFirst := mapDirection(direction) | 1084 | orientation, existingFirst := mapDirection(direction) | ||
| @@ -1184,10 +1190,10 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 1184 | wm.viewToNode[newPane.webView] = newLeaf | 1190 | wm.viewToNode[newPane.webView] = newLeaf | ||
| 1185 | wm.ensureHover(newLeaf) | 1191 | wm.ensureHover(newLeaf) | ||
| 1186 | 1192 | ||||
| 1187 | - | // For stacked panes, we need to ensure hover on the original target, not the stack container | 1193 | + | // Don't ensure hover on originalTarget when splitting from stack - it competes with new pane focus |
| 1188 | - | if originalTarget != target { | 1194 | + | // The originalTarget is still in the stack and should not interfere with the new split pane |
| 1189 | - | wm.ensureHover(originalTarget) | 1195 | + | if originalTarget == target { |
| 1190 | - | } else { | 1196 | + | // Only ensure hover on target if it's a regular (non-stack) split |
| 1191 | wm.ensureHover(target) | 1197 | wm.ensureHover(target) | ||
| 1192 | } | 1198 | } | ||
| 1193 | 1199 | ||||
| @@ -1338,19 +1344,9 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1338 | wm.detachHover(node) | 1344 | wm.detachHover(node) | ||
| 1339 | wm.detachHover(sibling) | 1345 | wm.detachHover(sibling) | ||
| 1340 | 1346 | ||||
| 1341 | - | delete(wm.viewToNode, node.pane.webView) | 1347 | + | // Clean up workspace tracking for both panes |
| 1342 | - | delete(wm.viewToNode, sibling.pane.webView) | 1348 | + | node.pane.CleanupFromWorkspace(wm) |
| 1343 | - | delete(wm.lastSplitMsg, node.pane.webView) | 1349 | + | sibling.pane.CleanupFromWorkspace(wm) |
| 1344 | - | delete(wm.lastSplitMsg, sibling.pane.webView) | |||
| 1345 | - | delete(wm.lastExitMsg, node.pane.webView) | |||
| 1346 | - | delete(wm.lastExitMsg, sibling.pane.webView) | |||
| 1347 | - | ||||
| 1348 | - | for i := 0; i < len(wm.app.panes); i++ { | |||
| 1349 | - | if wm.app.panes[i] == node.pane || wm.app.panes[i] == sibling.pane { | |||
| 1350 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |||
| 1351 | - | i-- | |||
| 1352 | - | } | |||
| 1353 | - | } | |||
| 1354 | 1350 | ||||
| 1355 | if node.pane.webView != nil { | 1351 | if node.pane.webView != nil { | ||
| 1356 | if err := node.pane.webView.Destroy(); err != nil { | 1352 | if err := node.pane.webView.Destroy(); err != nil { | ||
| @@ -1407,17 +1403,8 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1407 | // We have a replacement - delegate root status and close this pane | 1403 | // We have a replacement - delegate root status and close this pane | ||
| 1408 | log.Printf("[workspace] delegating root status from node=%#x to replacement=%#x", node.container, replacement.container) | 1404 | log.Printf("[workspace] delegating root status from node=%#x to replacement=%#x", node.container, replacement.container) | ||
| 1409 | 1405 | ||||
| 1410 | - | // Clean up references first | 1406 | + | // Clean up workspace tracking |
| 1411 | - | delete(wm.viewToNode, node.pane.webView) | 1407 | + | node.pane.CleanupFromWorkspace(wm) |
| 1412 | - | delete(wm.lastSplitMsg, node.pane.webView) | |||
| 1413 | - | delete(wm.lastExitMsg, node.pane.webView) | |||
| 1414 | - | ||||
| 1415 | - | for i, pane := range wm.app.panes { | |||
| 1416 | - | if pane == node.pane { | |||
| 1417 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |||
| 1418 | - | break | |||
| 1419 | - | } | |||
| 1420 | - | } | |||
| 1421 | 1408 | ||||
| 1422 | if wm.mainPane == node { | 1409 | if wm.mainPane == node { | ||
| 1423 | wm.mainPane = nil | 1410 | wm.mainPane = nil | ||
| @@ -1493,25 +1480,19 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1493 | focusTarget = wm.leftmostLeaf(wm.root) | 1480 | focusTarget = wm.leftmostLeaf(wm.root) | ||
| 1494 | } | 1481 | } | ||
| 1495 | 1482 | ||||
| 1496 | - | // Clean up references first | 1483 | + | // Clean up workspace tracking |
| 1497 | - | delete(wm.viewToNode, node.pane.webView) | 1484 | + | node.pane.CleanupFromWorkspace(wm) |
| 1498 | - | delete(wm.lastSplitMsg, node.pane.webView) | |||
| 1499 | - | delete(wm.lastExitMsg, node.pane.webView) | |||
| 1500 | - | ||||
| 1501 | - | for i, pane := range wm.app.panes { | |||
| 1502 | - | if pane == node.pane { | |||
| 1503 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |||
| 1504 | - | break | |||
| 1505 | - | } | |||
| 1506 | - | } | |||
| 1507 | 1485 | ||||
| 1508 | if wm.mainPane == node { | 1486 | if wm.mainPane == node { | ||
| 1509 | wm.mainPane = nil | 1487 | wm.mainPane = nil | ||
| 1510 | } | 1488 | } | ||
| 1511 | 1489 | ||||
| 1512 | - | // Clear current active if it's the node being closed | 1490 | + | // Clear current active if it's the node being closed and immediately set focus to target |
| 1513 | if wm.currentlyFocused == node { | 1491 | if wm.currentlyFocused == node { | ||
| 1514 | - | wm.currentlyFocused = nil | 1492 | + | wm.currentlyFocused = focusTarget |
| 1493 | + | if focusTarget != nil { | |||
| 1494 | + | wm.focusManager.SetActivePane(focusTarget) | |||
| 1495 | + | } | |||
| 1515 | } | 1496 | } | ||
| 1516 | 1497 | ||||
| 1517 | grand := parent.parent | 1498 | grand := parent.parent | ||
| @@ -1519,29 +1500,46 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1519 | // Parent is the root node. Promote sibling to become the new root. | 1500 | // Parent is the root node. Promote sibling to become the new root. | ||
| 1520 | // The sibling can be either a leaf (when only 2 panes total) or a branch (when more panes exist) | 1501 | // The sibling can be either a leaf (when only 2 panes total) or a branch (when more panes exist) | ||
| 1521 | log.Printf("[workspace] promoting sibling to root: container=%#x, isLeaf=%v", sibling.container, sibling.isLeaf) | 1502 | log.Printf("[workspace] promoting sibling to root: container=%#x, isLeaf=%v", sibling.container, sibling.isLeaf) | ||
| 1503 | + | ||||
| 1504 | + | // CRITICAL: Check widget validity before any GTK operations to prevent segfault | |||
| 1505 | + | if !webkit.WidgetIsValid(sibling.container) { | |||
| 1506 | + | log.Printf("[workspace] ERROR: sibling container %#x is not a valid GTK widget, skipping root promotion", sibling.container) | |||
| 1507 | + | return errors.New("sibling container is not a valid GTK widget") | |||
| 1508 | + | } | |||
| 1509 | + | ||||
| 1522 | wm.root = sibling | 1510 | wm.root = sibling | ||
| 1523 | sibling.parent = nil | 1511 | sibling.parent = nil | ||
| 1524 | 1512 | ||||
| 1525 | // Unparent the sibling from the paned first, then set it as window child | 1513 | // Unparent the sibling from the paned first, then set it as window child | ||
| 1526 | if wm.window != nil { | 1514 | if wm.window != nil { | ||
| 1527 | // GTK requires explicit unparenting before reparenting | 1515 | // GTK requires explicit unparenting before reparenting | ||
| 1528 | - | if parent.left == sibling { | 1516 | + | if parent.container != 0 && webkit.WidgetIsValid(parent.container) { |
| 1529 | - | webkit.PanedSetStartChild(parent.container, 0) | 1517 | + | if parent.left == sibling { |
| 1530 | - | } else { | 1518 | + | webkit.PanedSetStartChild(parent.container, 0) |
| 1531 | - | webkit.PanedSetEndChild(parent.container, 0) | 1519 | + | } else { |
| 1520 | + | webkit.PanedSetEndChild(parent.container, 0) | |||
| 1521 | + | } | |||
| 1532 | } | 1522 | } | ||
| 1533 | - | // Now we can safely set it as window child | 1523 | + | // Now we can safely set it as window child (widget validity already checked above) |
| 1534 | wm.window.SetChild(sibling.container) | 1524 | wm.window.SetChild(sibling.container) | ||
| 1535 | - | if sibling.container != 0 { | 1525 | + | webkit.WidgetQueueAllocate(sibling.container) |
| 1536 | - | webkit.WidgetQueueAllocate(sibling.container) | |||
| 1537 | - | } | |||
| 1538 | } | 1526 | } | ||
| 1539 | } else { | 1527 | } else { | ||
| 1540 | // Parent has a grandparent, so promote sibling to take parent's place | 1528 | // Parent has a grandparent, so promote sibling to take parent's place | ||
| 1541 | log.Printf("[workspace] promoting sibling to parent's position: sibling=%#x grand=%#x", sibling.container, grand.container) | 1529 | log.Printf("[workspace] promoting sibling to parent's position: sibling=%#x grand=%#x", sibling.container, grand.container) | ||
| 1542 | 1530 | ||||
| 1531 | + | // CRITICAL: Check widget validity before any GTK operations to prevent segfault | |||
| 1532 | + | if !webkit.WidgetIsValid(sibling.container) { | |||
| 1533 | + | log.Printf("[workspace] ERROR: sibling container %#x is not a valid GTK widget, skipping promotion", sibling.container) | |||
| 1534 | + | return errors.New("sibling container is not a valid GTK widget") | |||
| 1535 | + | } | |||
| 1536 | + | if !webkit.WidgetIsValid(grand.container) { | |||
| 1537 | + | log.Printf("[workspace] ERROR: grand container %#x is not a valid GTK widget, skipping promotion", grand.container) | |||
| 1538 | + | return errors.New("grand container is not a valid GTK widget") | |||
| 1539 | + | } | |||
| 1540 | + | ||||
| 1543 | // First unparent the sibling from its current parent to avoid GTK-CRITICAL errors | 1541 | // First unparent the sibling from its current parent to avoid GTK-CRITICAL errors | ||
| 1544 | - | if parent.container != 0 && !parent.isLeaf { | 1542 | + | if parent.container != 0 && !parent.isLeaf && webkit.WidgetIsValid(parent.container) { |
| 1545 | if parent.left == sibling { | 1543 | if parent.left == sibling { | ||
| 1546 | webkit.PanedSetStartChild(parent.container, 0) | 1544 | webkit.PanedSetStartChild(parent.container, 0) | ||
| 1547 | } else if parent.right == sibling { | 1545 | } else if parent.right == sibling { | ||
| @@ -1553,7 +1551,7 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1553 | // GTK4 automatically handles focus state synchronization during reparenting | 1551 | // GTK4 automatically handles focus state synchronization during reparenting | ||
| 1554 | 1552 | ||||
| 1555 | sibling.parent = grand | 1553 | sibling.parent = grand | ||
| 1556 | - | if grand.container != 0 && !grand.isLeaf { | 1554 | + | if grand.container != 0 && !grand.isLeaf && webkit.WidgetIsValid(grand.container) && webkit.WidgetIsValid(sibling.container) { |
| 1557 | if grand.left == parent { | 1555 | if grand.left == parent { | ||
| 1558 | grand.left = sibling | 1556 | grand.left = sibling | ||
| 1559 | webkit.PanedSetStartChild(grand.container, sibling.container) | 1557 | webkit.PanedSetStartChild(grand.container, sibling.container) | ||
| @@ -1566,7 +1564,7 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1566 | } | 1564 | } | ||
| 1567 | 1565 | ||||
| 1568 | // Keep sibling subtree visible | 1566 | // Keep sibling subtree visible | ||
| 1569 | - | if sibling.container != 0 { | 1567 | + | if sibling.container != 0 && webkit.WidgetIsValid(sibling.container) { |
| 1570 | webkit.WidgetShow(sibling.container) | 1568 | webkit.WidgetShow(sibling.container) | ||
| 1571 | } | 1569 | } | ||
| 1572 | 1570 | ||||
| @@ -1575,11 +1573,7 @@ func (wm *WorkspaceManager) closePane(node *paneNode) error { | |||||
| 1575 | parent.right = nil | 1573 | parent.right = nil | ||
| 1576 | node.parent = nil | 1574 | node.parent = nil | ||
| 1577 | 1575 | ||||
| 1578 | - | // Focus the target pane | 1576 | + | // Focus handling was moved earlier to avoid state gaps |
| 1579 | - | if focusTarget != nil && focusTarget != node { | |||
| 1580 | - | wm.currentlyFocused = focusTarget | |||
| 1581 | - | wm.focusManager.SetActivePane(focusTarget) | |||
| 1582 | - | } | |||
| 1583 | 1577 | ||||
| 1584 | // Destroy the webview and detach hover AFTER all hierarchy changes are complete | 1578 | // Destroy the webview and detach hover AFTER all hierarchy changes are complete | ||
| 1585 | ensureCleanup() | 1579 | ensureCleanup() | ||
| @@ -1659,143 +1653,6 @@ func (wm *WorkspaceManager) findReplacementRoot(excludeNode *paneNode) *paneNode | |||||
| 1659 | return nil | 1653 | return nil | ||
| 1660 | } | 1654 | } | ||
| 1661 | 1655 | ||||
| 1662 | - | // closeStackedPane handles closing a pane that is part of a stack. | |||
| 1663 | - | func (wm *WorkspaceManager) closeStackedPane(node *paneNode) error { | |||
| 1664 | - | if node.parent == nil || !node.parent.isStacked { | |||
| 1665 | - | return errors.New("node is not part of a stacked pane") | |||
| 1666 | - | } | |||
| 1667 | - | ||||
| 1668 | - | stackNode := node.parent | |||
| 1669 | - | ||||
| 1670 | - | // Find the index of the node to be closed | |||
| 1671 | - | nodeIndex := -1 | |||
| 1672 | - | for i, stackedPane := range stackNode.stackedPanes { | |||
| 1673 | - | if stackedPane == node { | |||
| 1674 | - | nodeIndex = i | |||
| 1675 | - | break | |||
| 1676 | - | } | |||
| 1677 | - | } | |||
| 1678 | - | ||||
| 1679 | - | if nodeIndex == -1 { | |||
| 1680 | - | return errors.New("node not found in stack") | |||
| 1681 | - | } | |||
| 1682 | - | ||||
| 1683 | - | log.Printf("[workspace] closing stacked pane: index=%d stackSize=%d", nodeIndex, len(stackNode.stackedPanes)) | |||
| 1684 | - | ||||
| 1685 | - | // Clean up the pane | |||
| 1686 | - | wm.detachHover(node) | |||
| 1687 | - | delete(wm.viewToNode, node.pane.webView) | |||
| 1688 | - | ||||
| 1689 | - | // Remove from app.panes | |||
| 1690 | - | for i, pane := range wm.app.panes { | |||
| 1691 | - | if pane == node.pane { | |||
| 1692 | - | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |||
| 1693 | - | break | |||
| 1694 | - | } | |||
| 1695 | - | } | |||
| 1696 | - | ||||
| 1697 | - | // Remove the pane's widgets from the stack container | |||
| 1698 | - | stackBox := stackNode.stackWrapper | |||
| 1699 | - | if stackBox == 0 { | |||
| 1700 | - | stackBox = stackNode.container | |||
| 1701 | - | } | |||
| 1702 | - | ||||
| 1703 | - | if node.titleBar != 0 { | |||
| 1704 | - | webkit.BoxRemove(stackBox, node.titleBar) | |||
| 1705 | - | } | |||
| 1706 | - | if node.container != 0 { | |||
| 1707 | - | webkit.BoxRemove(stackBox, node.container) | |||
| 1708 | - | // BoxRemove automatically unparents in GTK4, no manual unparent needed | |||
| 1709 | - | } | |||
| 1710 | - | ||||
| 1711 | - | // Clean up the pane | |||
| 1712 | - | node.pane.Cleanup() | |||
| 1713 | - | ||||
| 1714 | - | // Remove from the stack | |||
| 1715 | - | stackNode.stackedPanes = append(stackNode.stackedPanes[:nodeIndex], stackNode.stackedPanes[nodeIndex+1:]...) | |||
| 1716 | - | ||||
| 1717 | - | // Handle the remaining panes in the stack | |||
| 1718 | - | if len(stackNode.stackedPanes) == 0 { | |||
| 1719 | - | // No panes left in stack - this should not happen normally | |||
| 1720 | - | log.Printf("[workspace] warning: empty stack after closing pane") | |||
| 1721 | - | return errors.New("stack became empty") | |||
| 1722 | - | } else if len(stackNode.stackedPanes) == 1 { | |||
| 1723 | - | // Only one pane left - convert back to a regular pane | |||
| 1724 | - | lastPane := stackNode.stackedPanes[0] | |||
| 1725 | - | parent := stackNode.parent | |||
| 1726 | - | lastPaneContainer := lastPane.container | |||
| 1727 | - | ||||
| 1728 | - | // Remove the title bar since we're going back to single pane | |||
| 1729 | - | if lastPane.titleBar != 0 { | |||
| 1730 | - | webkit.BoxRemove(stackBox, lastPane.titleBar) | |||
| 1731 | - | } | |||
| 1732 | - | ||||
| 1733 | - | // Unparent the pane container from stack wrapper | |||
| 1734 | - | webkit.BoxRemove(stackBox, lastPaneContainer) | |||
| 1735 | - | ||||
| 1736 | - | // CRITICAL FIX: Replace the stackNode completely with lastPane | |||
| 1737 | - | // Update parent child references FIRST | |||
| 1738 | - | if parent == nil { | |||
| 1739 | - | wm.root = lastPane | |||
| 1740 | - | if wm.window != nil { | |||
| 1741 | - | wm.window.SetChild(lastPaneContainer) | |||
| 1742 | - | } | |||
| 1743 | - | } else { | |||
| 1744 | - | if parent.left == stackNode { | |||
| 1745 | - | parent.left = lastPane | |||
| 1746 | - | webkit.PanedSetStartChild(parent.container, lastPaneContainer) | |||
| 1747 | - | } else if parent.right == stackNode { | |||
| 1748 | - | parent.right = lastPane | |||
| 1749 | - | webkit.PanedSetEndChild(parent.container, lastPaneContainer) | |||
| 1750 | - | } | |||
| 1751 | - | } | |||
| 1752 | - | ||||
| 1753 | - | // Convert lastPane back to regular pane | |||
| 1754 | - | lastPane.parent = parent | |||
| 1755 | - | lastPane.isStacked = false | |||
| 1756 | - | lastPane.stackedPanes = nil | |||
| 1757 | - | lastPane.titleBar = 0 | |||
| 1758 | - | ||||
| 1759 | - | // Update the viewToNode mapping to point to the lastPane, not stackNode | |||
| 1760 | - | wm.viewToNode[lastPane.pane.webView] = lastPane | |||
| 1761 | - | ||||
| 1762 | - | // Focus the remaining pane if it was the active one being closed | |||
| 1763 | - | if wm.currentlyFocused == node { | |||
| 1764 | - | wm.currentlyFocused = lastPane | |||
| 1765 | - | wm.focusManager.SetActivePane(lastPane) | |||
| 1766 | - | } | |||
| 1767 | - | ||||
| 1768 | - | log.Printf("[workspace] converted single-pane stack back to regular pane") | |||
| 1769 | - | } else { | |||
| 1770 | - | // Multiple panes remain in stack - adjust active index | |||
| 1771 | - | if stackNode.activeStackIndex >= nodeIndex && stackNode.activeStackIndex > 0 { | |||
| 1772 | - | stackNode.activeStackIndex-- | |||
| 1773 | - | } | |||
| 1774 | - | if stackNode.activeStackIndex >= len(stackNode.stackedPanes) { | |||
| 1775 | - | stackNode.activeStackIndex = len(stackNode.stackedPanes) - 1 | |||
| 1776 | - | } | |||
| 1777 | - | ||||
| 1778 | - | // Update visibility for remaining panes | |||
| 1779 | - | wm.stackedPaneManager.UpdateStackVisibility(stackNode) | |||
| 1780 | - | ||||
| 1781 | - | // Focus the new active pane if we closed the currently active one | |||
| 1782 | - | if wm.currentlyFocused == node { | |||
| 1783 | - | newActivePaneInStack := stackNode.stackedPanes[stackNode.activeStackIndex] | |||
| 1784 | - | wm.currentlyFocused = newActivePaneInStack | |||
| 1785 | - | wm.focusManager.SetActivePane(newActivePaneInStack) | |||
| 1786 | - | } | |||
| 1787 | - | ||||
| 1788 | - | log.Printf("[workspace] closed pane from stack: remaining=%d activeIndex=%d", | |||
| 1789 | - | len(stackNode.stackedPanes), stackNode.activeStackIndex) | |||
| 1790 | - | } | |||
| 1791 | - | ||||
| 1792 | - | // Update CSS classes after pane count changes | |||
| 1793 | - | wm.ensurePaneBaseClasses() | |||
| 1794 | - | log.Printf("[workspace] stacked pane closed; panes remaining=%d", len(wm.app.panes)) | |||
| 1795 | - | ||||
| 1796 | - | return nil | |||
| 1797 | - | } | |||
| 1798 | - | ||||
| 1799 | func (wm *WorkspaceManager) updateMainPane() { | 1656 | func (wm *WorkspaceManager) updateMainPane() { | ||
| 1800 | if len(wm.app.panes) == 1 { | 1657 | if len(wm.app.panes) == 1 { | ||
| 1801 | if leaf := wm.viewToNode[wm.app.panes[0].webView]; leaf != nil { | 1658 | if leaf := wm.viewToNode[wm.app.panes[0].webView]; leaf != nil { | ||
| @@ -2586,3 +2443,33 @@ func mapDirection(direction string) (webkit.Orientation, bool) { | |||||
| 2586 | return webkit.OrientationHorizontal, true | 2443 | return webkit.OrientationHorizontal, true | ||
| 2587 | } | 2444 | } | ||
| 2588 | } | 2445 | } | ||
| 2446 | + | ||||
| 2447 | + | // Helper methods to support clean pane removal from workspace tracking | |||
| 2448 | + | // These methods implement the interface expected by BrowserPane.CleanupFromWorkspace() | |||
| 2449 | + | ||||
| 2450 | + | // removeFromMaps removes a WebView from all workspace tracking maps | |||
| 2451 | + | func (wm *WorkspaceManager) removeFromMaps(webView *webkit.WebView) { | |||
| 2452 | + | if wm == nil || webView == nil { | |||
| 2453 | + | return | |||
| 2454 | + | } | |||
| 2455 | + | ||||
| 2456 | + | delete(wm.viewToNode, webView) | |||
| 2457 | + | delete(wm.lastSplitMsg, webView) | |||
| 2458 | + | delete(wm.lastExitMsg, webView) | |||
| 2459 | + | log.Printf("[workspace] removed webview %p from tracking maps", webView) | |||
| 2460 | + | } | |||
| 2461 | + | ||||
| 2462 | + | // removeFromAppPanes removes a BrowserPane from the app.panes slice | |||
| 2463 | + | func (wm *WorkspaceManager) removeFromAppPanes(pane *BrowserPane) { | |||
| 2464 | + | if wm == nil || wm.app == nil || pane == nil { | |||
| 2465 | + | return | |||
| 2466 | + | } | |||
| 2467 | + | ||||
| 2468 | + | for i, p := range wm.app.panes { | |||
| 2469 | + | if p == pane { | |||
| 2470 | + | wm.app.panes = append(wm.app.panes[:i], wm.app.panes[i+1:]...) | |||
| 2471 | + | log.Printf("[workspace] removed pane from app.panes slice, remaining: %d", len(wm.app.panes)) | |||
| 2472 | + | return | |||
| 2473 | + | } | |||
| 2474 | + | } | |||
| 2475 | + | } | |||
M internal/app/browser/workspace_test.go +5 -4
| @@ -537,8 +537,9 @@ func TestRelativePaneOperations(t *testing.T) { | |||
| 537 | 537 | ||
| 538 | 538 | t.Run("Pane relationships", func(t *testing.T) { | |
| 539 | 539 | // Test parent-child relationships | |
| 540 | - | if twm.root.parent != twm.root.parent { | |
| 540 | + | if twm.root != nil && twm.root.parent != nil { | |
| 541 | 541 | // Root's parent should be consistent (either nil or split container) | |
| 542 | + | t.Logf("Root has parent: %p", twm.root.parent) | |
| 542 | 543 | } | |
| 543 | 544 | ||
| 544 | 545 | // Test that all leaves have proper parent relationships | |
| @@ -686,7 +687,7 @@ func TestAutoDestackingBehavior(t *testing.T) { | |||
| 686 | 687 | ||
| 687 | 688 | // Close 2 panes, leaving only 1 | |
| 688 | 689 | paneToClose1 := stackContainer.stackedPanes[0] | |
| 689 | - | err := twm.closeStackedPane(paneToClose1) | |
| 690 | + | err := twm.stackedPaneManager.CloseStackedPane(paneToClose1) | |
| 690 | 691 | if err != nil { | |
| 691 | 692 | t.Fatalf("Failed to close first stacked pane: %v", err) | |
| 692 | 693 | } | |
| @@ -701,7 +702,7 @@ func TestAutoDestackingBehavior(t *testing.T) { | |||
| 701 | 702 | ||
| 702 | 703 | // Close another pane - this should trigger auto-destacking | |
| 703 | 704 | paneToClose2 := stackContainer.stackedPanes[0] | |
| 704 | - | err = twm.closeStackedPane(paneToClose2) | |
| 705 | + | err = twm.stackedPaneManager.CloseStackedPane(paneToClose2) | |
| 705 | 706 | if err != nil { | |
| 706 | 707 | t.Fatalf("Failed to close second stacked pane: %v", err) | |
| 707 | 708 | } | |
| @@ -740,7 +741,7 @@ func TestAutoDestackingBehavior(t *testing.T) { | |||
| 740 | 741 | } | |
| 741 | 742 | ||
| 742 | 743 | // Close one pane - should trigger auto-destacking | |
| 743 | - | err := twm.closeStackedPane(pane1) | |
| 744 | + | err := twm.stackedPaneManager.CloseStackedPane(pane1) | |
| 744 | 745 | if err != nil { | |
| 745 | 746 | t.Fatalf("Failed to close pane: %v", err) | |
| 746 | 747 | } | |
| @@ -537,8 +537,9 @@ func TestRelativePaneOperations(t *testing.T) { | |||||
| 537 | 537 | ||||
| 538 | t.Run("Pane relationships", func(t *testing.T) { | 538 | t.Run("Pane relationships", func(t *testing.T) { | ||
| 539 | // Test parent-child relationships | 539 | // Test parent-child relationships | ||
| 540 | - | if twm.root.parent != twm.root.parent { | 540 | + | if twm.root != nil && twm.root.parent != nil { |
| 541 | // Root's parent should be consistent (either nil or split container) | 541 | // Root's parent should be consistent (either nil or split container) | ||
| 542 | + | t.Logf("Root has parent: %p", twm.root.parent) | |||
| 542 | } | 543 | } | ||
| 543 | 544 | ||||
| 544 | // Test that all leaves have proper parent relationships | 545 | // Test that all leaves have proper parent relationships | ||
| @@ -686,7 +687,7 @@ func TestAutoDestackingBehavior(t *testing.T) { | |||||
| 686 | 687 | ||||
| 687 | // Close 2 panes, leaving only 1 | 688 | // Close 2 panes, leaving only 1 | ||
| 688 | paneToClose1 := stackContainer.stackedPanes[0] | 689 | paneToClose1 := stackContainer.stackedPanes[0] | ||
| 689 | - | err := twm.closeStackedPane(paneToClose1) | 690 | + | err := twm.stackedPaneManager.CloseStackedPane(paneToClose1) |
| 690 | if err != nil { | 691 | if err != nil { | ||
| 691 | t.Fatalf("Failed to close first stacked pane: %v", err) | 692 | t.Fatalf("Failed to close first stacked pane: %v", err) | ||
| 692 | } | 693 | } | ||
| @@ -701,7 +702,7 @@ func TestAutoDestackingBehavior(t *testing.T) { | |||||
| 701 | 702 | ||||
| 702 | // Close another pane - this should trigger auto-destacking | 703 | // Close another pane - this should trigger auto-destacking | ||
| 703 | paneToClose2 := stackContainer.stackedPanes[0] | 704 | paneToClose2 := stackContainer.stackedPanes[0] | ||
| 704 | - | err = twm.closeStackedPane(paneToClose2) | 705 | + | err = twm.stackedPaneManager.CloseStackedPane(paneToClose2) |
| 705 | if err != nil { | 706 | if err != nil { | ||
| 706 | t.Fatalf("Failed to close second stacked pane: %v", err) | 707 | t.Fatalf("Failed to close second stacked pane: %v", err) | ||
| 707 | } | 708 | } | ||
| @@ -740,7 +741,7 @@ func TestAutoDestackingBehavior(t *testing.T) { | |||||
| 740 | } | 741 | } | ||
| 741 | 742 | ||||
| 742 | // Close one pane - should trigger auto-destacking | 743 | // Close one pane - should trigger auto-destacking | ||
| 743 | - | err := twm.closeStackedPane(pane1) | 744 | + | err := twm.stackedPaneManager.CloseStackedPane(pane1) |
| 744 | if err != nil { | 745 | if err != nil { | ||
| 745 | t.Fatalf("Failed to close pane: %v", err) | 746 | t.Fatalf("Failed to close pane: %v", err) | ||
| 746 | } | 747 | } | ||