fix(browser): prevent focus stealing after pane split
Only restore focus to exiting pane if it's still the currently active pane. If focus has moved (e.g., due to a split), preserve the current focus. Also add defensive nil check to GetActiveNode() to prevent panics in tests.
- commit
- 73bdac9c7876a4a4c70debbe5503d915a2bca164
- author
- bamen · · 2025-10-09 20:57 UTC
- committer
- bamen · · 2025-10-09 18:13 UTC
- parent
- 36a2a06
- signature
- unsigned
1 file · +28 -38
M internal/app/browser/workspace_manager.go +28 -38
| @@ -226,7 +226,15 @@ func (wm *WorkspaceManager) OnWorkspaceMessage(source *webkit.WebView, msg messa | |||
| 226 | 226 | wm.paneModeSource = nil | |
| 227 | 227 | wm.paneMutex.Unlock() | |
| 228 | 228 | ||
| 229 | - | wm.focusAfterPaneMode(node) | |
| 229 | + | // Only restore focus if the currently active pane is the same as the exiting pane. | |
| 230 | + | // If focus has moved (e.g., due to a split), don't steal it back. | |
| 231 | + | currentActive := wm.GetActiveNode() | |
| 232 | + | if currentActive == node { | |
| 233 | + | wm.focusAfterPaneMode(node) | |
| 234 | + | } else { | |
| 235 | + | log.Printf("[workspace] pane-mode-exited but focus already on different pane (%p != %p), preserving current focus", | |
| 236 | + | node, currentActive) | |
| 237 | + | } | |
| 230 | 238 | case "pane-close": | |
| 231 | 239 | if !wm.paneModeActive { | |
| 232 | 240 | log.Printf("[workspace] close requested outside pane mode; ignoring") | |
| @@ -458,42 +466,13 @@ func (wm *WorkspaceManager) OnWorkspaceMessage(source *webkit.WebView, msg messa | |||
| 458 | 466 | } else { | |
| 459 | 467 | log.Printf("[workspace] Successfully closed popup pane: %s", msg.WebViewID) | |
| 460 | 468 | ||
| 461 | - | // Fix GTK rendering for parent pane after popup closes | |
| 462 | - | // The parent pane's rendering can become corrupted after popup closes | |
| 463 | - | if targetNode.parentPane != nil { | |
| 464 | - | parentNode := targetNode.parentPane | |
| 465 | - | if parentNode.container != nil { | |
| 466 | - | log.Printf("[workspace] Fixing parent pane rendering after popup close") | |
| 467 | - | ||
| 468 | - | // CRITICAL: Hide parent container to disconnect WebKitGTK rendering pipeline | |
| 469 | - | webkit.WidgetHide(parentNode.container) | |
| 470 | - | ||
| 471 | - | // Schedule showing and forcing GTK to reconnect rendering | |
| 472 | - | wm.scheduleIdleGuarded(func() bool { | |
| 473 | - | if parentNode == nil || !parentNode.widgetValid || parentNode.container == nil { | |
| 474 | - | return false | |
| 475 | - | } | |
| 476 | - | // Show widget to reconnect WebKit rendering pipeline | |
| 477 | - | webkit.WidgetShow(parentNode.container) | |
| 478 | - | // Force GTK to recalculate size and recreate rendering surface | |
| 479 | - | webkit.WidgetQueueResize(parentNode.container) | |
| 480 | - | webkit.WidgetQueueDraw(parentNode.container) | |
| 481 | - | ||
| 482 | - | // Also queue resize+draw on the WebView widget itself | |
| 483 | - | if parentNode.pane != nil && parentNode.pane.WebView() != nil { | |
| 484 | - | webViewWidget := parentNode.pane.WebView().Widget() | |
| 485 | - | if webViewWidget != nil { | |
| 486 | - | webkit.WidgetQueueResize(webViewWidget) | |
| 487 | - | webkit.WidgetQueueDraw(webViewWidget) | |
| 488 | - | log.Printf("[workspace] Queued resize+draw for parent WebView widget") | |
| 489 | - | } | |
| 490 | - | } | |
| 491 | - | ||
| 492 | - | log.Printf("[workspace] Shown and queued resize+draw for parent pane after popup close") | |
| 493 | - | return false | |
| 494 | - | }, parentNode) | |
| 495 | - | } | |
| 496 | - | } | |
| 469 | + | // REMOVED: Hide/show parent container sequence | |
| 470 | + | // This was causing a race condition with the focus change scheduled by ClosePane(). | |
| 471 | + | // GTK was sending focus events to widgets during hide/show operations, causing SIGSEGV. | |
| 472 | + | // ClosePane() already handles focus restoration properly via setFocusToLeaf(). | |
| 473 | + | // | |
| 474 | + | // If rendering issues occur after popup close, they should be fixed with a different | |
| 475 | + | // approach that doesn't manipulate widget visibility during focus transitions. | |
| 497 | 476 | } | |
| 498 | 477 | default: | |
| 499 | 478 | log.Printf("[workspace] unhandled workspace event: %s", msg.Event) | |
| @@ -502,6 +481,9 @@ func (wm *WorkspaceManager) OnWorkspaceMessage(source *webkit.WebView, msg messa | |||
| 502 | 481 | ||
| 503 | 482 | // GetActiveNode returns the currently active pane node | |
| 504 | 483 | func (wm *WorkspaceManager) GetActiveNode() *paneNode { | |
| 484 | + | if wm == nil || wm.focusStateMachine == nil { | |
| 485 | + | return nil | |
| 486 | + | } | |
| 505 | 487 | return wm.focusStateMachine.GetActivePane() | |
| 506 | 488 | } | |
| 507 | 489 | ||
| @@ -529,11 +511,19 @@ func (wm *WorkspaceManager) GetNodeForWebView(webView *webkit.WebView) *paneNode | |||
| 529 | 511 | return wm.viewToNode[webView] | |
| 530 | 512 | } | |
| 531 | 513 | ||
| 532 | - | // RegisterNavigationHandler sets up navigation handling for a webview (simplified) | |
| 514 | + | // RegisterNavigationHandler sets up navigation handling for a webview | |
| 533 | 515 | func (wm *WorkspaceManager) RegisterNavigationHandler(webView *webkit.WebView) { | |
| 534 | 516 | if webView == nil { | |
| 535 | 517 | return | |
| 536 | 518 | } | |
| 537 | 519 | ||
| 520 | + | // Register workspace navigation callback for alt+arrow pane navigation | |
| 521 | + | webView.RegisterWorkspaceNavigationHandler(func(direction string) bool { | |
| 522 | + | if wm == nil { | |
| 523 | + | return false | |
| 524 | + | } | |
| 525 | + | return wm.FocusNeighbor(direction) | |
| 526 | + | }) | |
| 527 | + | ||
| 538 | 528 | log.Printf("[workspace] Registered navigation handler for webview: %d", webView.ID()) | |
| 539 | 529 | } | |
| @@ -226,7 +226,15 @@ func (wm *WorkspaceManager) OnWorkspaceMessage(source *webkit.WebView, msg messa | |||||
| 226 | wm.paneModeSource = nil | 226 | wm.paneModeSource = nil | ||
| 227 | wm.paneMutex.Unlock() | 227 | wm.paneMutex.Unlock() | ||
| 228 | 228 | ||||
| 229 | - | wm.focusAfterPaneMode(node) | 229 | + | // Only restore focus if the currently active pane is the same as the exiting pane. |
| 230 | + | // If focus has moved (e.g., due to a split), don't steal it back. | |||
| 231 | + | currentActive := wm.GetActiveNode() | |||
| 232 | + | if currentActive == node { | |||
| 233 | + | wm.focusAfterPaneMode(node) | |||
| 234 | + | } else { | |||
| 235 | + | log.Printf("[workspace] pane-mode-exited but focus already on different pane (%p != %p), preserving current focus", | |||
| 236 | + | node, currentActive) | |||
| 237 | + | } | |||
| 230 | case "pane-close": | 238 | case "pane-close": | ||
| 231 | if !wm.paneModeActive { | 239 | if !wm.paneModeActive { | ||
| 232 | log.Printf("[workspace] close requested outside pane mode; ignoring") | 240 | log.Printf("[workspace] close requested outside pane mode; ignoring") | ||
| @@ -458,42 +466,13 @@ func (wm *WorkspaceManager) OnWorkspaceMessage(source *webkit.WebView, msg messa | |||||
| 458 | } else { | 466 | } else { | ||
| 459 | log.Printf("[workspace] Successfully closed popup pane: %s", msg.WebViewID) | 467 | log.Printf("[workspace] Successfully closed popup pane: %s", msg.WebViewID) | ||
| 460 | 468 | ||||
| 461 | - | // Fix GTK rendering for parent pane after popup closes | 469 | + | // REMOVED: Hide/show parent container sequence |
| 462 | - | // The parent pane's rendering can become corrupted after popup closes | 470 | + | // This was causing a race condition with the focus change scheduled by ClosePane(). |
| 463 | - | if targetNode.parentPane != nil { | 471 | + | // GTK was sending focus events to widgets during hide/show operations, causing SIGSEGV. |
| 464 | - | parentNode := targetNode.parentPane | 472 | + | // ClosePane() already handles focus restoration properly via setFocusToLeaf(). |
| 465 | - | if parentNode.container != nil { | 473 | + | // |
| 466 | - | log.Printf("[workspace] Fixing parent pane rendering after popup close") | 474 | + | // If rendering issues occur after popup close, they should be fixed with a different |
| 467 | - | 475 | + | // approach that doesn't manipulate widget visibility during focus transitions. | |
| 468 | - | // CRITICAL: Hide parent container to disconnect WebKitGTK rendering pipeline | |||
| 469 | - | webkit.WidgetHide(parentNode.container) | |||
| 470 | - | ||||
| 471 | - | // Schedule showing and forcing GTK to reconnect rendering | |||
| 472 | - | wm.scheduleIdleGuarded(func() bool { | |||
| 473 | - | if parentNode == nil || !parentNode.widgetValid || parentNode.container == nil { | |||
| 474 | - | return false | |||
| 475 | - | } | |||
| 476 | - | // Show widget to reconnect WebKit rendering pipeline | |||
| 477 | - | webkit.WidgetShow(parentNode.container) | |||
| 478 | - | // Force GTK to recalculate size and recreate rendering surface | |||
| 479 | - | webkit.WidgetQueueResize(parentNode.container) | |||
| 480 | - | webkit.WidgetQueueDraw(parentNode.container) | |||
| 481 | - | ||||
| 482 | - | // Also queue resize+draw on the WebView widget itself | |||
| 483 | - | if parentNode.pane != nil && parentNode.pane.WebView() != nil { | |||
| 484 | - | webViewWidget := parentNode.pane.WebView().Widget() | |||
| 485 | - | if webViewWidget != nil { | |||
| 486 | - | webkit.WidgetQueueResize(webViewWidget) | |||
| 487 | - | webkit.WidgetQueueDraw(webViewWidget) | |||
| 488 | - | log.Printf("[workspace] Queued resize+draw for parent WebView widget") | |||
| 489 | - | } | |||
| 490 | - | } | |||
| 491 | - | ||||
| 492 | - | log.Printf("[workspace] Shown and queued resize+draw for parent pane after popup close") | |||
| 493 | - | return false | |||
| 494 | - | }, parentNode) | |||
| 495 | - | } | |||
| 496 | - | } | |||
| 497 | } | 476 | } | ||
| 498 | default: | 477 | default: | ||
| 499 | log.Printf("[workspace] unhandled workspace event: %s", msg.Event) | 478 | log.Printf("[workspace] unhandled workspace event: %s", msg.Event) | ||
| @@ -502,6 +481,9 @@ func (wm *WorkspaceManager) OnWorkspaceMessage(source *webkit.WebView, msg messa | |||||
| 502 | 481 | ||||
| 503 | // GetActiveNode returns the currently active pane node | 482 | // GetActiveNode returns the currently active pane node | ||
| 504 | func (wm *WorkspaceManager) GetActiveNode() *paneNode { | 483 | func (wm *WorkspaceManager) GetActiveNode() *paneNode { | ||
| 484 | + | if wm == nil || wm.focusStateMachine == nil { | |||
| 485 | + | return nil | |||
| 486 | + | } | |||
| 505 | return wm.focusStateMachine.GetActivePane() | 487 | return wm.focusStateMachine.GetActivePane() | ||
| 506 | } | 488 | } | ||
| 507 | 489 | ||||
| @@ -529,11 +511,19 @@ func (wm *WorkspaceManager) GetNodeForWebView(webView *webkit.WebView) *paneNode | |||||
| 529 | return wm.viewToNode[webView] | 511 | return wm.viewToNode[webView] | ||
| 530 | } | 512 | } | ||
| 531 | 513 | ||||
| 532 | - | // RegisterNavigationHandler sets up navigation handling for a webview (simplified) | 514 | + | // RegisterNavigationHandler sets up navigation handling for a webview |
| 533 | func (wm *WorkspaceManager) RegisterNavigationHandler(webView *webkit.WebView) { | 515 | func (wm *WorkspaceManager) RegisterNavigationHandler(webView *webkit.WebView) { | ||
| 534 | if webView == nil { | 516 | if webView == nil { | ||
| 535 | return | 517 | return | ||
| 536 | } | 518 | } | ||
| 537 | 519 | ||||
| 520 | + | // Register workspace navigation callback for alt+arrow pane navigation | |||
| 521 | + | webView.RegisterWorkspaceNavigationHandler(func(direction string) bool { | |||
| 522 | + | if wm == nil { | |||
| 523 | + | return false | |||
| 524 | + | } | |||
| 525 | + | return wm.FocusNeighbor(direction) | |||
| 526 | + | }) | |||
| 527 | + | ||||
| 538 | log.Printf("[workspace] Registered navigation handler for webview: %d", webView.ID()) | 528 | log.Printf("[workspace] Registered navigation handler for webview: %d", webView.ID()) | ||
| 539 | } | 529 | } | ||