refactor(workspace): unify popup insertion with splitNode to fix rendering corruption
PROBLEM: When inserting popup panes, insertPopupPane() was reparenting the EXISTING parent WebView container, which caused WebKit's rendering surface to disconnect and never reconnect, resulting in gray/corrupted homepage after popup insertion. ROOT CAUSE: - splitNode() works: creates NEW WebView, no active rendering surface - insertPopupPane() corrupts: moves EXISTING parent WebView with active rendering, WebKit rendering surface disconnects SOLUTION (KISS principle): Unified all splitting into a single splitNode() function that accepts an optional existingPane parameter: - nil: creates new WebView (normal splits) - provided: uses existing pane (popup insertion) CHANGES: - Modified splitNode() signature to accept optional *BrowserPane parameter - When existingPane provided: parent WebView NEVER MOVES, only new pane gets inserted into the split - Updated all popup insertion paths to use splitNode() - Removed insertPopupPane() entirely (~293 lines deleted) - Updated normal split callers to pass nil for new parameter BENEFITS: - Single responsibility: one function handles all splitting - No duplication: deleted 293 lines of complex reparenting logic - No rendering corruption: parent WebView never moves - True KISS: clear semantics with explicit parameter All tests pass (10/10 popup tests passing).
- commit
- d244331004be7d6d766360001e455e647fb7db94
- author
- bamen · · 2025-10-12 11:10 UTC
- parent
- 78194ac
- signature
- unsigned
3 files · +60 -302
M internal/app/browser/workspace_operations.go +2 -2
| @@ -54,7 +54,7 @@ func (wm *WorkspaceManager) SplitPane(target *paneNode, direction string) (*pane | |||
| 54 | 54 | // Step 2: Execute directly if we're already on the GTK main thread | |
| 55 | 55 | if webkit.IsMainThread() { | |
| 56 | 56 | log.Printf("[workspace] Already on main thread, executing split directly") | |
| 57 | - | newNode, err := wm.splitNode(target, direction) | |
| 57 | + | newNode, err := wm.splitNode(target, direction, nil) | |
| 58 | 58 | if err != nil { | |
| 59 | 59 | return nil, err | |
| 60 | 60 | } | |
| @@ -78,7 +78,7 @@ func (wm *WorkspaceManager) SplitPane(target *paneNode, direction string) (*pane | |||
| 78 | 78 | done := make(chan struct{}) | |
| 79 | 79 | ||
| 80 | 80 | _ = webkit.IdleAdd(func() bool { | |
| 81 | - | newNode, splitErr = wm.splitNode(target, direction) | |
| 81 | + | newNode, splitErr = wm.splitNode(target, direction, nil) | |
| 82 | 82 | if splitErr == nil { | |
| 83 | 83 | if wm.debugLevel >= DebugBasic { | |
| 84 | 84 | if verr := wm.treeValidator.ValidateTree(wm.root, "after_split"); verr != nil { | |
| @@ -54,7 +54,7 @@ func (wm *WorkspaceManager) SplitPane(target *paneNode, direction string) (*pane | |||||
| 54 | // Step 2: Execute directly if we're already on the GTK main thread | 54 | // Step 2: Execute directly if we're already on the GTK main thread | ||
| 55 | if webkit.IsMainThread() { | 55 | if webkit.IsMainThread() { | ||
| 56 | log.Printf("[workspace] Already on main thread, executing split directly") | 56 | log.Printf("[workspace] Already on main thread, executing split directly") | ||
| 57 | - | newNode, err := wm.splitNode(target, direction) | 57 | + | newNode, err := wm.splitNode(target, direction, nil) |
| 58 | if err != nil { | 58 | if err != nil { | ||
| 59 | return nil, err | 59 | return nil, err | ||
| 60 | } | 60 | } | ||
| @@ -78,7 +78,7 @@ func (wm *WorkspaceManager) SplitPane(target *paneNode, direction string) (*pane | |||||
| 78 | done := make(chan struct{}) | 78 | done := make(chan struct{}) | ||
| 79 | 79 | ||||
| 80 | _ = webkit.IdleAdd(func() bool { | 80 | _ = webkit.IdleAdd(func() bool { | ||
| 81 | - | newNode, splitErr = wm.splitNode(target, direction) | 81 | + | newNode, splitErr = wm.splitNode(target, direction, nil) |
| 82 | if splitErr == nil { | 82 | if splitErr == nil { | ||
| 83 | if wm.debugLevel >= DebugBasic { | 83 | if wm.debugLevel >= DebugBasic { | ||
| 84 | if verr := wm.treeValidator.ValidateTree(wm.root, "after_split"); verr != nil { | 84 | if verr := wm.treeValidator.ValidateTree(wm.root, "after_split"); verr != nil { | ||
M internal/app/browser/workspace_pane_ops.go +47 -291
| @@ -291,287 +291,15 @@ func (wm *WorkspaceManager) removeFromAppPanes(pane *BrowserPane) { | |||
| 291 | 291 | } | |
| 292 | 292 | } | |
| 293 | 293 | ||
| 294 | - | // insertPopupPane inserts a pre-created popup pane into the workspace | |
| 295 | - | func (wm *WorkspaceManager) insertPopupPane(target *paneNode, newPane *BrowserPane, direction string) error { | |
| 296 | - | if target == nil { | |
| 297 | - | return errors.New("insert target cannot be nil") | |
| 298 | - | } | |
| 299 | - | ||
| 300 | - | // Accept both leaf panes and stacked panes as valid targets | |
| 301 | - | if !target.isLeaf && !target.isStacked { | |
| 302 | - | return errors.New("insert target must be a leaf pane or stacked pane") | |
| 303 | - | } | |
| 304 | - | ||
| 305 | - | // For leaf panes, require a pane | |
| 306 | - | if target.isLeaf && target.pane == nil { | |
| 307 | - | return errors.New("leaf target missing pane") | |
| 308 | - | } | |
| 309 | - | ||
| 310 | - | // For stacked panes, we trust the caller - production stacks have varied structures | |
| 311 | - | // Do not validate internal stack structure here | |
| 312 | - | ||
| 313 | - | if newPane == nil || newPane.webView == nil { | |
| 314 | - | return errors.New("new pane missing webview") | |
| 315 | - | } | |
| 316 | - | ||
| 317 | - | if handler := newPane.MessageHandler(); handler != nil { | |
| 318 | - | handler.SetWorkspaceObserver(wm) | |
| 319 | - | } | |
| 320 | - | ||
| 321 | - | newContainer := newPane.webView.RootWidget() | |
| 322 | - | if newContainer == nil { | |
| 323 | - | return errors.New("new pane missing container") | |
| 324 | - | } | |
| 325 | - | ||
| 326 | - | webkit.WidgetSetHExpand(newContainer, true) | |
| 327 | - | webkit.WidgetSetVExpand(newContainer, true) | |
| 328 | - | ||
| 329 | - | // CRITICAL: Handle stacked panes correctly - same logic as splitNode | |
| 330 | - | // When the target is inside a stacked pane, we need to split around the entire stack, | |
| 331 | - | // not the individual pane within the stack | |
| 332 | - | originalTarget := target | |
| 333 | - | var stackContainer *paneNode | |
| 334 | - | if target.parent != nil && target.parent.isStacked { | |
| 335 | - | // Target is in a stack - we'll create the split around the entire stack | |
| 336 | - | stackContainer = target.parent | |
| 337 | - | log.Printf("[workspace] target is in stack, will split around the stack: originalTarget=%p stackContainer=%p", originalTarget, stackContainer) | |
| 338 | - | } | |
| 339 | - | ||
| 340 | - | // Determine what we're actually splitting | |
| 341 | - | var splitTarget *paneNode | |
| 342 | - | var splitTargetContainer gtk.Widgetter | |
| 343 | - | ||
| 344 | - | if stackContainer != nil { | |
| 345 | - | // Case: splitting from inside a stack - we split around the entire stack | |
| 346 | - | splitTarget = stackContainer | |
| 347 | - | splitTargetContainer = stackContainer.container | |
| 348 | - | log.Printf("[workspace] splitting around stack: stackContainer=%p", splitTarget) | |
| 349 | - | } else { | |
| 350 | - | // Case: normal split from a simple pane or direct stack target | |
| 351 | - | splitTarget = target | |
| 352 | - | splitTargetContainer = target.container | |
| 353 | - | log.Printf("[workspace] normal split from pane: target=%p", splitTarget) | |
| 354 | - | } | |
| 355 | - | ||
| 356 | - | if splitTargetContainer == nil { | |
| 357 | - | return errors.New("split target missing container") | |
| 358 | - | } | |
| 359 | - | ||
| 360 | - | orientation, existingFirst := mapDirection(direction) | |
| 361 | - | log.Printf("[workspace] inserting popup direction=%s orientation=%v existingFirst=%v splitTarget.parent=%p", direction, orientation, existingFirst, splitTarget.parent) | |
| 362 | - | ||
| 363 | - | paned := gtk.NewPaned(orientation) | |
| 364 | - | if paned == nil { | |
| 365 | - | return errors.New("failed to create GtkPaned") | |
| 366 | - | } | |
| 367 | - | paned.SetHExpand(true) | |
| 368 | - | paned.SetVExpand(true) | |
| 369 | - | // For popup panes: preserve existing pane size, let popup take remaining space | |
| 370 | - | paned.SetResizeStartChild(false) // existing pane keeps its size | |
| 371 | - | paned.SetResizeEndChild(true) // popup can resize | |
| 372 | - | log.Printf("[workspace] configured paned for popup: start=fixed, end=flexible") | |
| 373 | - | ||
| 374 | - | newLeaf := &paneNode{ | |
| 375 | - | pane: newPane, | |
| 376 | - | isLeaf: true, | |
| 377 | - | } | |
| 378 | - | wm.initializePaneWidgets(newLeaf, newContainer) | |
| 379 | - | ||
| 380 | - | split := &paneNode{ | |
| 381 | - | parent: splitTarget.parent, | |
| 382 | - | orientation: orientation, | |
| 383 | - | isLeaf: false, | |
| 384 | - | } | |
| 385 | - | wm.initializePaneWidgets(split, paned) | |
| 386 | - | ||
| 387 | - | parent := split.parent | |
| 388 | - | ||
| 389 | - | // CRITICAL: Capture parent paned's divider position BEFORE reparenting to preserve layout | |
| 390 | - | // Only attempt this if the parent container is actually a paned widget | |
| 391 | - | var parentDividerPos int | |
| 392 | - | if parent != nil && parent.container != nil { | |
| 393 | - | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |
| 394 | - | parentDividerPos = parentPaned.Position() | |
| 395 | - | log.Printf("[workspace] captured parent divider position: %d", parentDividerPos) | |
| 396 | - | } else { | |
| 397 | - | log.Printf("[workspace] parent container is not a paned widget (likely a stacked container)") | |
| 398 | - | } | |
| 399 | - | } | |
| 400 | - | ||
| 401 | - | // Clear CSS classes before reparenting to avoid GTK bloom filter corruption | |
| 402 | - | if splitTargetContainer != nil { | |
| 403 | - | // Remove active border class if present (prevents GTK bloom filter corruption during unparent) | |
| 404 | - | if webkit.WidgetHasCSSClass(splitTargetContainer, activePaneClass) { | |
| 405 | - | webkit.WidgetRemoveCSSClass(splitTargetContainer, activePaneClass) | |
| 406 | - | } | |
| 407 | - | ||
| 408 | - | // CRITICAL: Hide widget before reparenting to disconnect WebKitGTK rendering pipeline | |
| 409 | - | // This forces WebKit to detach its compositor, preventing rendering corruption | |
| 410 | - | webkit.WidgetHide(splitTargetContainer) | |
| 411 | - | log.Printf("[workspace] Hidden split target container before reparenting: %p", splitTargetContainer) | |
| 412 | - | } | |
| 413 | - | ||
| 414 | - | // Prevent GTK from auto-removing controllers while the widget is temporarily unparented. | |
| 415 | - | wm.safelyDetachControllersBeforeReparent(splitTarget) | |
| 416 | - | ||
| 417 | - | // Detach existing container from its current GTK parent before inserting into new paned. | |
| 418 | - | if parent == nil { | |
| 419 | - | // Split target is the root - remove it from the window | |
| 420 | - | log.Printf("[workspace] removing split target container=%p from window", splitTargetContainer) | |
| 421 | - | if wm.window != nil { | |
| 422 | - | wm.window.SetChild(nil) | |
| 423 | - | } | |
| 424 | - | // GTK4 will auto-unparent when we add to new paned - no manual unparent needed | |
| 425 | - | } else if parent.container != nil { | |
| 426 | - | // Split target has a parent paned - unparent it from there | |
| 427 | - | log.Printf("[workspace] unparenting split target container from parent paned") | |
| 428 | - | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |
| 429 | - | log.Printf("[workspace] executing unparent: parentPtr=%p", parent.container) | |
| 430 | - | if parent.left == splitTarget { | |
| 431 | - | parentPaned.SetStartChild(nil) | |
| 432 | - | } else if parent.right == splitTarget { | |
| 433 | - | parentPaned.SetEndChild(nil) | |
| 434 | - | } | |
| 435 | - | webkit.WidgetQueueAllocate(parent.container) | |
| 436 | - | } | |
| 437 | - | } | |
| 438 | - | ||
| 439 | - | // Add both containers to the new paned | |
| 440 | - | if existingFirst { | |
| 441 | - | split.left = splitTarget | |
| 442 | - | split.right = newLeaf | |
| 443 | - | paned.SetStartChild(splitTargetContainer) | |
| 444 | - | paned.SetEndChild(newContainer) | |
| 445 | - | log.Printf("[workspace] added splitTarget=%p as start child, new=%p as end child", splitTargetContainer, newContainer) | |
| 446 | - | } else { | |
| 447 | - | split.left = newLeaf | |
| 448 | - | split.right = splitTarget | |
| 449 | - | paned.SetStartChild(newContainer) | |
| 450 | - | paned.SetEndChild(splitTargetContainer) | |
| 451 | - | log.Printf("[workspace] added new=%p as start child, splitTarget=%p as end child", newContainer, splitTargetContainer) | |
| 452 | - | } | |
| 453 | - | ||
| 454 | - | splitTarget.parent = split | |
| 455 | - | newLeaf.parent = split | |
| 456 | - | ||
| 457 | - | if parent == nil { | |
| 458 | - | wm.root = split | |
| 459 | - | if wm.window != nil { | |
| 460 | - | wm.window.SetChild(paned) | |
| 461 | - | paned.QueueAllocate() | |
| 462 | - | log.Printf("[workspace] paned set as window child: paned=%p", paned) | |
| 463 | - | } | |
| 464 | - | } else { | |
| 465 | - | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |
| 466 | - | log.Printf("[workspace] executing parent reparent: parentPtr=%p, paned=%p", parent.container, paned) | |
| 467 | - | if parent.left == splitTarget { | |
| 468 | - | parent.left = split | |
| 469 | - | parentPaned.SetStartChild(paned) | |
| 470 | - | } else if parent.right == splitTarget { | |
| 471 | - | parent.right = split | |
| 472 | - | parentPaned.SetEndChild(paned) | |
| 473 | - | } | |
| 474 | - | webkit.WidgetQueueAllocate(parent.container) | |
| 475 | - | paned.QueueAllocate() | |
| 476 | - | log.Printf("[workspace] paned inserted into parent successfully") | |
| 477 | - | ||
| 478 | - | // CRITICAL: Restore parent paned's divider position to preserve existing layout | |
| 479 | - | // Only attempt this if we captured a position and the parent is actually a paned widget | |
| 480 | - | if parentDividerPos > 0 { | |
| 481 | - | parentPaned.SetPosition(parentDividerPos) | |
| 482 | - | log.Printf("[workspace] restored parent divider position: %d", parentDividerPos) | |
| 483 | - | } | |
| 484 | - | } | |
| 485 | - | } | |
| 486 | - | ||
| 487 | - | paned.Show() | |
| 488 | - | ||
| 489 | - | // Force GTK to re-layout the new GtkPaned container once both children are attached. | |
| 490 | - | wm.scheduleIdleGuarded(func() bool { | |
| 491 | - | if split == nil || !split.widgetValid { | |
| 492 | - | return false | |
| 493 | - | } | |
| 494 | - | if split.container != nil { | |
| 495 | - | webkit.WidgetQueueResize(split.container) | |
| 496 | - | webkit.WidgetQueueDraw(split.container) | |
| 497 | - | } | |
| 498 | - | return false | |
| 499 | - | }, split) | |
| 500 | - | ||
| 501 | - | // Synchronize the divider once the paned has a meaningful allocation. | |
| 502 | - | wm.syncPanedDivider(paned, orientation, split) | |
| 503 | - | ||
| 504 | - | // CRITICAL: Show the split target container and force GTK to recreate rendering surface after reparenting | |
| 505 | - | // This reconnects WebKitGTK's rendering pipeline and fixes compositor sync issues | |
| 506 | - | if splitTarget != nil && splitTarget.container != nil { | |
| 507 | - | wm.scheduleIdleGuarded(func() bool { | |
| 508 | - | if splitTarget == nil || !splitTarget.widgetValid || splitTarget.container == nil { | |
| 509 | - | return false | |
| 510 | - | } | |
| 511 | - | // Show widget to reconnect WebKit rendering pipeline | |
| 512 | - | webkit.WidgetShow(splitTarget.container) | |
| 513 | - | // Force GTK to recalculate size and recreate rendering surface | |
| 514 | - | webkit.WidgetQueueResize(splitTarget.container) | |
| 515 | - | webkit.WidgetQueueDraw(splitTarget.container) | |
| 516 | - | log.Printf("[workspace] Shown and queued resize+draw for split target container after reparenting") | |
| 517 | - | return false | |
| 518 | - | }, splitTarget) | |
| 519 | - | } | |
| 520 | - | ||
| 521 | - | wm.viewToNode[newPane.webView] = newLeaf | |
| 522 | - | wm.ensureHover(newLeaf) | |
| 523 | - | wm.ensureHover(splitTarget) | |
| 524 | - | wm.app.panes = append(wm.app.panes, newPane) | |
| 525 | - | if newPane.zoomController != nil { | |
| 526 | - | newPane.zoomController.ApplyInitialZoom() | |
| 527 | - | } | |
| 528 | - | ||
| 529 | - | // Update CSS classes for all panes now that we have multiple panes | |
| 530 | - | ||
| 531 | - | reattachTargets := []*paneNode{splitTarget} | |
| 532 | - | if splitTarget != nil && splitTarget.isStacked { | |
| 533 | - | reattachTargets = append(reattachTargets, splitTarget.stackedPanes...) | |
| 534 | - | } | |
| 535 | - | guardNodes := append([]*paneNode{newLeaf}, reattachTargets...) | |
| 536 | - | wm.scheduleIdleGuarded(func() bool { | |
| 537 | - | if newLeaf == nil || !newLeaf.widgetValid { | |
| 538 | - | return false | |
| 539 | - | } | |
| 540 | - | if newContainer != nil { | |
| 541 | - | webkit.WidgetShow(newContainer) | |
| 542 | - | } | |
| 543 | - | // Attach GTK focus controller to new pane | |
| 544 | - | if wm.focusStateMachine != nil { | |
| 545 | - | wm.focusStateMachine.attachGTKController(newLeaf) | |
| 546 | - | } | |
| 547 | - | ||
| 548 | - | for _, candidate := range reattachTargets { | |
| 549 | - | if candidate == nil || !candidate.widgetValid { | |
| 550 | - | continue | |
| 551 | - | } | |
| 552 | - | if candidate.pendingHoverReattach { | |
| 553 | - | wm.ensureHover(candidate) | |
| 554 | - | if candidate.hoverToken != nil { | |
| 555 | - | candidate.pendingHoverReattach = false | |
| 556 | - | } | |
| 557 | - | } | |
| 558 | - | if candidate.pendingFocusReattach && wm.focusStateMachine != nil { | |
| 559 | - | wm.focusStateMachine.attachGTKController(candidate) | |
| 560 | - | if candidate.focusControllerToken != 0 { | |
| 561 | - | candidate.pendingFocusReattach = false | |
| 562 | - | } | |
| 563 | - | } | |
| 564 | - | } | |
| 565 | - | ||
| 566 | - | wm.SetActivePane(newLeaf, SourceSplit) | |
| 567 | - | return false | |
| 568 | - | }, guardNodes...) | |
| 569 | - | ||
| 570 | - | return nil | |
| 571 | - | } | |
| 294 | + | // insertPopupPane has been REMOVED - all popup insertion now uses splitNode() with existing pane parameter. | |
| 295 | + | // This eliminates widget reparenting of existing WebViews, which was causing rendering corruption. | |
| 296 | + | // See splitNode() for the unified implementation. | |
| 572 | 297 | ||
| 573 | 298 | // splitNode splits a target pane into two panes in the specified direction | |
| 574 | - | func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*paneNode, error) { | |
| 299 | + | // splitNode splits a target pane into two panes in the specified direction. | |
| 300 | + | // If existingPane is provided, it will be used instead of creating a new WebView. | |
| 301 | + | // This allows reusing the function for both normal splits (nil) and popup insertion (popup pane). | |
| 302 | + | func (wm *WorkspaceManager) splitNode(target *paneNode, direction string, existingPane *BrowserPane) (*paneNode, error) { | |
| 575 | 303 | if target == nil || !target.isLeaf || target.pane == nil { | |
| 576 | 304 | return nil, errors.New("split target must be a leaf pane") | |
| 577 | 305 | } | |
| @@ -602,14 +330,25 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 602 | 330 | log.Printf("[workspace] normal split from pane: target=%p", splitTarget) | |
| 603 | 331 | } | |
| 604 | 332 | ||
| 605 | - | newView, err := wm.createWebView() | |
| 606 | - | if err != nil { | |
| 607 | - | return nil, err | |
| 608 | - | } | |
| 333 | + | // Use provided pane or create new one | |
| 334 | + | var newPane *BrowserPane | |
| 609 | 335 | ||
| 610 | - | newPane, err := wm.createPane(newView) | |
| 611 | - | if err != nil { | |
| 612 | - | return nil, err | |
| 336 | + | if existingPane != nil { | |
| 337 | + | // Reuse existing pane (for popup insertion) | |
| 338 | + | newPane = existingPane | |
| 339 | + | log.Printf("[workspace] Using existing pane for split: %p", newPane) | |
| 340 | + | } else { | |
| 341 | + | // Create new WebView and pane (for normal splits) | |
| 342 | + | newView, err := wm.createWebView() | |
| 343 | + | if err != nil { | |
| 344 | + | return nil, err | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | newPane, err = wm.createPane(newView) | |
| 348 | + | if err != nil { | |
| 349 | + | return nil, err | |
| 350 | + | } | |
| 351 | + | log.Printf("[workspace] Created new pane for split: %p", newPane) | |
| 613 | 352 | } | |
| 614 | 353 | ||
| 615 | 354 | if handler := newPane.MessageHandler(); handler != nil { | |
| @@ -645,8 +384,8 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 645 | 384 | } | |
| 646 | 385 | ||
| 647 | 386 | orientation, existingFirst := mapDirection(direction) | |
| 648 | - | log.Printf("[workspace] splitting direction=%s orientation=%v existingFirst=%v splitTarget.parent=%p splitTarget.isStacked=%v", | |
| 649 | - | direction, orientation, existingFirst, splitTarget.parent, splitTarget.isStacked) | |
| 387 | + | log.Printf("[workspace] splitting direction=%s orientation=%v existingFirst=%v splitTarget.parent=%p splitTarget.isStacked=%v isPopup=%v", | |
| 388 | + | direction, orientation, existingFirst, splitTarget.parent, splitTarget.isStacked, existingPane != nil) | |
| 650 | 389 | ||
| 651 | 390 | paned := gtk.NewPaned(orientation) | |
| 652 | 391 | if paned == nil { | |
| @@ -654,8 +393,18 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 654 | 393 | } | |
| 655 | 394 | paned.SetHExpand(true) | |
| 656 | 395 | paned.SetVExpand(true) | |
| 657 | - | paned.SetResizeStartChild(true) | |
| 658 | - | paned.SetResizeEndChild(true) | |
| 396 | + | ||
| 397 | + | // Configure resize behavior based on whether this is a popup | |
| 398 | + | if existingPane != nil { | |
| 399 | + | // For popup panes: preserve existing pane size, let popup take remaining space | |
| 400 | + | paned.SetResizeStartChild(false) // existing pane keeps its size | |
| 401 | + | paned.SetResizeEndChild(true) // popup can resize | |
| 402 | + | log.Printf("[workspace] configured paned for popup: start=fixed, end=flexible") | |
| 403 | + | } else { | |
| 404 | + | // For normal splits: both panes can resize | |
| 405 | + | paned.SetResizeStartChild(true) | |
| 406 | + | paned.SetResizeEndChild(true) | |
| 407 | + | } | |
| 659 | 408 | ||
| 660 | 409 | newLeaf := &paneNode{ | |
| 661 | 410 | pane: newPane, | |
| @@ -799,7 +548,14 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||
| 799 | 548 | } | |
| 800 | 549 | } | |
| 801 | 550 | ||
| 802 | - | wm.app.panes = append(wm.app.panes, newPane) | |
| 551 | + | // Only add to app.panes if this is a new pane (not an existing popup pane) | |
| 552 | + | if existingPane == nil { | |
| 553 | + | wm.app.panes = append(wm.app.panes, newPane) | |
| 554 | + | } else { | |
| 555 | + | // For existing popup panes, they're already in app.panes from createPane() in handlePopupReadyToShow | |
| 556 | + | log.Printf("[workspace] Skipping app.panes append for existing popup pane (already added)") | |
| 557 | + | } | |
| 558 | + | ||
| 803 | 559 | if newPane.zoomController != nil { | |
| 804 | 560 | newPane.zoomController.ApplyInitialZoom() | |
| 805 | 561 | } | |
| @@ -291,287 +291,15 @@ func (wm *WorkspaceManager) removeFromAppPanes(pane *BrowserPane) { | |||||
| 291 | } | 291 | } | ||
| 292 | } | 292 | } | ||
| 293 | 293 | ||||
| 294 | - | // insertPopupPane inserts a pre-created popup pane into the workspace | 294 | + | // insertPopupPane has been REMOVED - all popup insertion now uses splitNode() with existing pane parameter. |
| 295 | - | func (wm *WorkspaceManager) insertPopupPane(target *paneNode, newPane *BrowserPane, direction string) error { | 295 | + | // This eliminates widget reparenting of existing WebViews, which was causing rendering corruption. |
| 296 | - | if target == nil { | 296 | + | // See splitNode() for the unified implementation. |
| 297 | - | return errors.New("insert target cannot be nil") | |||
| 298 | - | } | |||
| 299 | - | ||||
| 300 | - | // Accept both leaf panes and stacked panes as valid targets | |||
| 301 | - | if !target.isLeaf && !target.isStacked { | |||
| 302 | - | return errors.New("insert target must be a leaf pane or stacked pane") | |||
| 303 | - | } | |||
| 304 | - | ||||
| 305 | - | // For leaf panes, require a pane | |||
| 306 | - | if target.isLeaf && target.pane == nil { | |||
| 307 | - | return errors.New("leaf target missing pane") | |||
| 308 | - | } | |||
| 309 | - | ||||
| 310 | - | // For stacked panes, we trust the caller - production stacks have varied structures | |||
| 311 | - | // Do not validate internal stack structure here | |||
| 312 | - | ||||
| 313 | - | if newPane == nil || newPane.webView == nil { | |||
| 314 | - | return errors.New("new pane missing webview") | |||
| 315 | - | } | |||
| 316 | - | ||||
| 317 | - | if handler := newPane.MessageHandler(); handler != nil { | |||
| 318 | - | handler.SetWorkspaceObserver(wm) | |||
| 319 | - | } | |||
| 320 | - | ||||
| 321 | - | newContainer := newPane.webView.RootWidget() | |||
| 322 | - | if newContainer == nil { | |||
| 323 | - | return errors.New("new pane missing container") | |||
| 324 | - | } | |||
| 325 | - | ||||
| 326 | - | webkit.WidgetSetHExpand(newContainer, true) | |||
| 327 | - | webkit.WidgetSetVExpand(newContainer, true) | |||
| 328 | - | ||||
| 329 | - | // CRITICAL: Handle stacked panes correctly - same logic as splitNode | |||
| 330 | - | // When the target is inside a stacked pane, we need to split around the entire stack, | |||
| 331 | - | // not the individual pane within the stack | |||
| 332 | - | originalTarget := target | |||
| 333 | - | var stackContainer *paneNode | |||
| 334 | - | if target.parent != nil && target.parent.isStacked { | |||
| 335 | - | // Target is in a stack - we'll create the split around the entire stack | |||
| 336 | - | stackContainer = target.parent | |||
| 337 | - | log.Printf("[workspace] target is in stack, will split around the stack: originalTarget=%p stackContainer=%p", originalTarget, stackContainer) | |||
| 338 | - | } | |||
| 339 | - | ||||
| 340 | - | // Determine what we're actually splitting | |||
| 341 | - | var splitTarget *paneNode | |||
| 342 | - | var splitTargetContainer gtk.Widgetter | |||
| 343 | - | ||||
| 344 | - | if stackContainer != nil { | |||
| 345 | - | // Case: splitting from inside a stack - we split around the entire stack | |||
| 346 | - | splitTarget = stackContainer | |||
| 347 | - | splitTargetContainer = stackContainer.container | |||
| 348 | - | log.Printf("[workspace] splitting around stack: stackContainer=%p", splitTarget) | |||
| 349 | - | } else { | |||
| 350 | - | // Case: normal split from a simple pane or direct stack target | |||
| 351 | - | splitTarget = target | |||
| 352 | - | splitTargetContainer = target.container | |||
| 353 | - | log.Printf("[workspace] normal split from pane: target=%p", splitTarget) | |||
| 354 | - | } | |||
| 355 | - | ||||
| 356 | - | if splitTargetContainer == nil { | |||
| 357 | - | return errors.New("split target missing container") | |||
| 358 | - | } | |||
| 359 | - | ||||
| 360 | - | orientation, existingFirst := mapDirection(direction) | |||
| 361 | - | log.Printf("[workspace] inserting popup direction=%s orientation=%v existingFirst=%v splitTarget.parent=%p", direction, orientation, existingFirst, splitTarget.parent) | |||
| 362 | - | ||||
| 363 | - | paned := gtk.NewPaned(orientation) | |||
| 364 | - | if paned == nil { | |||
| 365 | - | return errors.New("failed to create GtkPaned") | |||
| 366 | - | } | |||
| 367 | - | paned.SetHExpand(true) | |||
| 368 | - | paned.SetVExpand(true) | |||
| 369 | - | // For popup panes: preserve existing pane size, let popup take remaining space | |||
| 370 | - | paned.SetResizeStartChild(false) // existing pane keeps its size | |||
| 371 | - | paned.SetResizeEndChild(true) // popup can resize | |||
| 372 | - | log.Printf("[workspace] configured paned for popup: start=fixed, end=flexible") | |||
| 373 | - | ||||
| 374 | - | newLeaf := &paneNode{ | |||
| 375 | - | pane: newPane, | |||
| 376 | - | isLeaf: true, | |||
| 377 | - | } | |||
| 378 | - | wm.initializePaneWidgets(newLeaf, newContainer) | |||
| 379 | - | ||||
| 380 | - | split := &paneNode{ | |||
| 381 | - | parent: splitTarget.parent, | |||
| 382 | - | orientation: orientation, | |||
| 383 | - | isLeaf: false, | |||
| 384 | - | } | |||
| 385 | - | wm.initializePaneWidgets(split, paned) | |||
| 386 | - | ||||
| 387 | - | parent := split.parent | |||
| 388 | - | ||||
| 389 | - | // CRITICAL: Capture parent paned's divider position BEFORE reparenting to preserve layout | |||
| 390 | - | // Only attempt this if the parent container is actually a paned widget | |||
| 391 | - | var parentDividerPos int | |||
| 392 | - | if parent != nil && parent.container != nil { | |||
| 393 | - | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |||
| 394 | - | parentDividerPos = parentPaned.Position() | |||
| 395 | - | log.Printf("[workspace] captured parent divider position: %d", parentDividerPos) | |||
| 396 | - | } else { | |||
| 397 | - | log.Printf("[workspace] parent container is not a paned widget (likely a stacked container)") | |||
| 398 | - | } | |||
| 399 | - | } | |||
| 400 | - | ||||
| 401 | - | // Clear CSS classes before reparenting to avoid GTK bloom filter corruption | |||
| 402 | - | if splitTargetContainer != nil { | |||
| 403 | - | // Remove active border class if present (prevents GTK bloom filter corruption during unparent) | |||
| 404 | - | if webkit.WidgetHasCSSClass(splitTargetContainer, activePaneClass) { | |||
| 405 | - | webkit.WidgetRemoveCSSClass(splitTargetContainer, activePaneClass) | |||
| 406 | - | } | |||
| 407 | - | ||||
| 408 | - | // CRITICAL: Hide widget before reparenting to disconnect WebKitGTK rendering pipeline | |||
| 409 | - | // This forces WebKit to detach its compositor, preventing rendering corruption | |||
| 410 | - | webkit.WidgetHide(splitTargetContainer) | |||
| 411 | - | log.Printf("[workspace] Hidden split target container before reparenting: %p", splitTargetContainer) | |||
| 412 | - | } | |||
| 413 | - | ||||
| 414 | - | // Prevent GTK from auto-removing controllers while the widget is temporarily unparented. | |||
| 415 | - | wm.safelyDetachControllersBeforeReparent(splitTarget) | |||
| 416 | - | ||||
| 417 | - | // Detach existing container from its current GTK parent before inserting into new paned. | |||
| 418 | - | if parent == nil { | |||
| 419 | - | // Split target is the root - remove it from the window | |||
| 420 | - | log.Printf("[workspace] removing split target container=%p from window", splitTargetContainer) | |||
| 421 | - | if wm.window != nil { | |||
| 422 | - | wm.window.SetChild(nil) | |||
| 423 | - | } | |||
| 424 | - | // GTK4 will auto-unparent when we add to new paned - no manual unparent needed | |||
| 425 | - | } else if parent.container != nil { | |||
| 426 | - | // Split target has a parent paned - unparent it from there | |||
| 427 | - | log.Printf("[workspace] unparenting split target container from parent paned") | |||
| 428 | - | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |||
| 429 | - | log.Printf("[workspace] executing unparent: parentPtr=%p", parent.container) | |||
| 430 | - | if parent.left == splitTarget { | |||
| 431 | - | parentPaned.SetStartChild(nil) | |||
| 432 | - | } else if parent.right == splitTarget { | |||
| 433 | - | parentPaned.SetEndChild(nil) | |||
| 434 | - | } | |||
| 435 | - | webkit.WidgetQueueAllocate(parent.container) | |||
| 436 | - | } | |||
| 437 | - | } | |||
| 438 | - | ||||
| 439 | - | // Add both containers to the new paned | |||
| 440 | - | if existingFirst { | |||
| 441 | - | split.left = splitTarget | |||
| 442 | - | split.right = newLeaf | |||
| 443 | - | paned.SetStartChild(splitTargetContainer) | |||
| 444 | - | paned.SetEndChild(newContainer) | |||
| 445 | - | log.Printf("[workspace] added splitTarget=%p as start child, new=%p as end child", splitTargetContainer, newContainer) | |||
| 446 | - | } else { | |||
| 447 | - | split.left = newLeaf | |||
| 448 | - | split.right = splitTarget | |||
| 449 | - | paned.SetStartChild(newContainer) | |||
| 450 | - | paned.SetEndChild(splitTargetContainer) | |||
| 451 | - | log.Printf("[workspace] added new=%p as start child, splitTarget=%p as end child", newContainer, splitTargetContainer) | |||
| 452 | - | } | |||
| 453 | - | ||||
| 454 | - | splitTarget.parent = split | |||
| 455 | - | newLeaf.parent = split | |||
| 456 | - | ||||
| 457 | - | if parent == nil { | |||
| 458 | - | wm.root = split | |||
| 459 | - | if wm.window != nil { | |||
| 460 | - | wm.window.SetChild(paned) | |||
| 461 | - | paned.QueueAllocate() | |||
| 462 | - | log.Printf("[workspace] paned set as window child: paned=%p", paned) | |||
| 463 | - | } | |||
| 464 | - | } else { | |||
| 465 | - | if parentPaned, ok := parent.container.(*gtk.Paned); ok && parentPaned != nil { | |||
| 466 | - | log.Printf("[workspace] executing parent reparent: parentPtr=%p, paned=%p", parent.container, paned) | |||
| 467 | - | if parent.left == splitTarget { | |||
| 468 | - | parent.left = split | |||
| 469 | - | parentPaned.SetStartChild(paned) | |||
| 470 | - | } else if parent.right == splitTarget { | |||
| 471 | - | parent.right = split | |||
| 472 | - | parentPaned.SetEndChild(paned) | |||
| 473 | - | } | |||
| 474 | - | webkit.WidgetQueueAllocate(parent.container) | |||
| 475 | - | paned.QueueAllocate() | |||
| 476 | - | log.Printf("[workspace] paned inserted into parent successfully") | |||
| 477 | - | ||||
| 478 | - | // CRITICAL: Restore parent paned's divider position to preserve existing layout | |||
| 479 | - | // Only attempt this if we captured a position and the parent is actually a paned widget | |||
| 480 | - | if parentDividerPos > 0 { | |||
| 481 | - | parentPaned.SetPosition(parentDividerPos) | |||
| 482 | - | log.Printf("[workspace] restored parent divider position: %d", parentDividerPos) | |||
| 483 | - | } | |||
| 484 | - | } | |||
| 485 | - | } | |||
| 486 | - | ||||
| 487 | - | paned.Show() | |||
| 488 | - | ||||
| 489 | - | // Force GTK to re-layout the new GtkPaned container once both children are attached. | |||
| 490 | - | wm.scheduleIdleGuarded(func() bool { | |||
| 491 | - | if split == nil || !split.widgetValid { | |||
| 492 | - | return false | |||
| 493 | - | } | |||
| 494 | - | if split.container != nil { | |||
| 495 | - | webkit.WidgetQueueResize(split.container) | |||
| 496 | - | webkit.WidgetQueueDraw(split.container) | |||
| 497 | - | } | |||
| 498 | - | return false | |||
| 499 | - | }, split) | |||
| 500 | - | ||||
| 501 | - | // Synchronize the divider once the paned has a meaningful allocation. | |||
| 502 | - | wm.syncPanedDivider(paned, orientation, split) | |||
| 503 | - | ||||
| 504 | - | // CRITICAL: Show the split target container and force GTK to recreate rendering surface after reparenting | |||
| 505 | - | // This reconnects WebKitGTK's rendering pipeline and fixes compositor sync issues | |||
| 506 | - | if splitTarget != nil && splitTarget.container != nil { | |||
| 507 | - | wm.scheduleIdleGuarded(func() bool { | |||
| 508 | - | if splitTarget == nil || !splitTarget.widgetValid || splitTarget.container == nil { | |||
| 509 | - | return false | |||
| 510 | - | } | |||
| 511 | - | // Show widget to reconnect WebKit rendering pipeline | |||
| 512 | - | webkit.WidgetShow(splitTarget.container) | |||
| 513 | - | // Force GTK to recalculate size and recreate rendering surface | |||
| 514 | - | webkit.WidgetQueueResize(splitTarget.container) | |||
| 515 | - | webkit.WidgetQueueDraw(splitTarget.container) | |||
| 516 | - | log.Printf("[workspace] Shown and queued resize+draw for split target container after reparenting") | |||
| 517 | - | return false | |||
| 518 | - | }, splitTarget) | |||
| 519 | - | } | |||
| 520 | - | ||||
| 521 | - | wm.viewToNode[newPane.webView] = newLeaf | |||
| 522 | - | wm.ensureHover(newLeaf) | |||
| 523 | - | wm.ensureHover(splitTarget) | |||
| 524 | - | wm.app.panes = append(wm.app.panes, newPane) | |||
| 525 | - | if newPane.zoomController != nil { | |||
| 526 | - | newPane.zoomController.ApplyInitialZoom() | |||
| 527 | - | } | |||
| 528 | - | ||||
| 529 | - | // Update CSS classes for all panes now that we have multiple panes | |||
| 530 | - | ||||
| 531 | - | reattachTargets := []*paneNode{splitTarget} | |||
| 532 | - | if splitTarget != nil && splitTarget.isStacked { | |||
| 533 | - | reattachTargets = append(reattachTargets, splitTarget.stackedPanes...) | |||
| 534 | - | } | |||
| 535 | - | guardNodes := append([]*paneNode{newLeaf}, reattachTargets...) | |||
| 536 | - | wm.scheduleIdleGuarded(func() bool { | |||
| 537 | - | if newLeaf == nil || !newLeaf.widgetValid { | |||
| 538 | - | return false | |||
| 539 | - | } | |||
| 540 | - | if newContainer != nil { | |||
| 541 | - | webkit.WidgetShow(newContainer) | |||
| 542 | - | } | |||
| 543 | - | // Attach GTK focus controller to new pane | |||
| 544 | - | if wm.focusStateMachine != nil { | |||
| 545 | - | wm.focusStateMachine.attachGTKController(newLeaf) | |||
| 546 | - | } | |||
| 547 | - | ||||
| 548 | - | for _, candidate := range reattachTargets { | |||
| 549 | - | if candidate == nil || !candidate.widgetValid { | |||
| 550 | - | continue | |||
| 551 | - | } | |||
| 552 | - | if candidate.pendingHoverReattach { | |||
| 553 | - | wm.ensureHover(candidate) | |||
| 554 | - | if candidate.hoverToken != nil { | |||
| 555 | - | candidate.pendingHoverReattach = false | |||
| 556 | - | } | |||
| 557 | - | } | |||
| 558 | - | if candidate.pendingFocusReattach && wm.focusStateMachine != nil { | |||
| 559 | - | wm.focusStateMachine.attachGTKController(candidate) | |||
| 560 | - | if candidate.focusControllerToken != 0 { | |||
| 561 | - | candidate.pendingFocusReattach = false | |||
| 562 | - | } | |||
| 563 | - | } | |||
| 564 | - | } | |||
| 565 | - | ||||
| 566 | - | wm.SetActivePane(newLeaf, SourceSplit) | |||
| 567 | - | return false | |||
| 568 | - | }, guardNodes...) | |||
| 569 | - | ||||
| 570 | - | return nil | |||
| 571 | - | } | |||
| 572 | 297 | ||||
| 573 | // splitNode splits a target pane into two panes in the specified direction | 298 | // splitNode splits a target pane into two panes in the specified direction | ||
| 574 | - | func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*paneNode, error) { | 299 | + | // splitNode splits a target pane into two panes in the specified direction. |
| 300 | + | // If existingPane is provided, it will be used instead of creating a new WebView. | |||
| 301 | + | // This allows reusing the function for both normal splits (nil) and popup insertion (popup pane). | |||
| 302 | + | func (wm *WorkspaceManager) splitNode(target *paneNode, direction string, existingPane *BrowserPane) (*paneNode, error) { | |||
| 575 | if target == nil || !target.isLeaf || target.pane == nil { | 303 | if target == nil || !target.isLeaf || target.pane == nil { | ||
| 576 | return nil, errors.New("split target must be a leaf pane") | 304 | return nil, errors.New("split target must be a leaf pane") | ||
| 577 | } | 305 | } | ||
| @@ -602,14 +330,25 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 602 | log.Printf("[workspace] normal split from pane: target=%p", splitTarget) | 330 | log.Printf("[workspace] normal split from pane: target=%p", splitTarget) | ||
| 603 | } | 331 | } | ||
| 604 | 332 | ||||
| 605 | - | newView, err := wm.createWebView() | 333 | + | // Use provided pane or create new one |
| 606 | - | if err != nil { | 334 | + | var newPane *BrowserPane |
| 607 | - | return nil, err | |||
| 608 | - | } | |||
| 609 | 335 | ||||
| 610 | - | newPane, err := wm.createPane(newView) | 336 | + | if existingPane != nil { |
| 611 | - | if err != nil { | 337 | + | // Reuse existing pane (for popup insertion) |
| 612 | - | return nil, err | 338 | + | newPane = existingPane |
| 339 | + | log.Printf("[workspace] Using existing pane for split: %p", newPane) | |||
| 340 | + | } else { | |||
| 341 | + | // Create new WebView and pane (for normal splits) | |||
| 342 | + | newView, err := wm.createWebView() | |||
| 343 | + | if err != nil { | |||
| 344 | + | return nil, err | |||
| 345 | + | } | |||
| 346 | + | ||||
| 347 | + | newPane, err = wm.createPane(newView) | |||
| 348 | + | if err != nil { | |||
| 349 | + | return nil, err | |||
| 350 | + | } | |||
| 351 | + | log.Printf("[workspace] Created new pane for split: %p", newPane) | |||
| 613 | } | 352 | } | ||
| 614 | 353 | ||||
| 615 | if handler := newPane.MessageHandler(); handler != nil { | 354 | if handler := newPane.MessageHandler(); handler != nil { | ||
| @@ -645,8 +384,8 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 645 | } | 384 | } | ||
| 646 | 385 | ||||
| 647 | orientation, existingFirst := mapDirection(direction) | 386 | orientation, existingFirst := mapDirection(direction) | ||
| 648 | - | log.Printf("[workspace] splitting direction=%s orientation=%v existingFirst=%v splitTarget.parent=%p splitTarget.isStacked=%v", | 387 | + | log.Printf("[workspace] splitting direction=%s orientation=%v existingFirst=%v splitTarget.parent=%p splitTarget.isStacked=%v isPopup=%v", |
| 649 | - | direction, orientation, existingFirst, splitTarget.parent, splitTarget.isStacked) | 388 | + | direction, orientation, existingFirst, splitTarget.parent, splitTarget.isStacked, existingPane != nil) |
| 650 | 389 | ||||
| 651 | paned := gtk.NewPaned(orientation) | 390 | paned := gtk.NewPaned(orientation) | ||
| 652 | if paned == nil { | 391 | if paned == nil { | ||
| @@ -654,8 +393,18 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 654 | } | 393 | } | ||
| 655 | paned.SetHExpand(true) | 394 | paned.SetHExpand(true) | ||
| 656 | paned.SetVExpand(true) | 395 | paned.SetVExpand(true) | ||
| 657 | - | paned.SetResizeStartChild(true) | 396 | + | |
| 658 | - | paned.SetResizeEndChild(true) | 397 | + | // Configure resize behavior based on whether this is a popup |
| 398 | + | if existingPane != nil { | |||
| 399 | + | // For popup panes: preserve existing pane size, let popup take remaining space | |||
| 400 | + | paned.SetResizeStartChild(false) // existing pane keeps its size | |||
| 401 | + | paned.SetResizeEndChild(true) // popup can resize | |||
| 402 | + | log.Printf("[workspace] configured paned for popup: start=fixed, end=flexible") | |||
| 403 | + | } else { | |||
| 404 | + | // For normal splits: both panes can resize | |||
| 405 | + | paned.SetResizeStartChild(true) | |||
| 406 | + | paned.SetResizeEndChild(true) | |||
| 407 | + | } | |||
| 659 | 408 | ||||
| 660 | newLeaf := &paneNode{ | 409 | newLeaf := &paneNode{ | ||
| 661 | pane: newPane, | 410 | pane: newPane, | ||
| @@ -799,7 +548,14 @@ func (wm *WorkspaceManager) splitNode(target *paneNode, direction string) (*pane | |||||
| 799 | } | 548 | } | ||
| 800 | } | 549 | } | ||
| 801 | 550 | ||||
| 802 | - | wm.app.panes = append(wm.app.panes, newPane) | 551 | + | // Only add to app.panes if this is a new pane (not an existing popup pane) |
| 552 | + | if existingPane == nil { | |||
| 553 | + | wm.app.panes = append(wm.app.panes, newPane) | |||
| 554 | + | } else { | |||
| 555 | + | // For existing popup panes, they're already in app.panes from createPane() in handlePopupReadyToShow | |||
| 556 | + | log.Printf("[workspace] Skipping app.panes append for existing popup pane (already added)") | |||
| 557 | + | } | |||
| 558 | + | ||||
| 803 | if newPane.zoomController != nil { | 559 | if newPane.zoomController != nil { | ||
| 804 | newPane.zoomController.ApplyInitialZoom() | 560 | newPane.zoomController.ApplyInitialZoom() | ||
| 805 | } | 561 | } | ||
M internal/app/browser/workspace_popup.go +11 -9
| @@ -39,7 +39,8 @@ func (wm *WorkspaceManager) insertIndependentPane(sourceNode *paneNode, webView | |||
| 39 | 39 | if direction == "" { | |
| 40 | 40 | direction = "right" | |
| 41 | 41 | } | |
| 42 | - | if err := wm.insertPopupPane(sourceNode, newPane, direction); err != nil { // reuse insertion primitive | |
| 42 | + | // Use splitNode with existing pane instead of insertPopupPane | |
| 43 | + | if _, err := wm.splitNode(sourceNode, direction, newPane); err != nil { | |
| 43 | 44 | return err | |
| 44 | 45 | } | |
| 45 | 46 | node := wm.viewToNode[webView] | |
| @@ -66,7 +67,8 @@ func (wm *WorkspaceManager) configureRelatedPopup(sourceNode *paneNode, webView | |||
| 66 | 67 | if direction == "" { | |
| 67 | 68 | direction = "right" | |
| 68 | 69 | } | |
| 69 | - | if err := wm.insertPopupPane(sourceNode, newPane, direction); err != nil { | |
| 70 | + | // Use splitNode with existing pane instead of insertPopupPane | |
| 71 | + | if _, err := wm.splitNode(sourceNode, direction, newPane); err != nil { | |
| 70 | 72 | log.Printf("[workspace] failed to insert related popup pane: %v", err) | |
| 71 | 73 | return | |
| 72 | 74 | } | |
| @@ -422,12 +424,12 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||
| 422 | 424 | ||
| 423 | 425 | switch behavior { | |
| 424 | 426 | case config.PopupBehaviorSplit: | |
| 425 | - | // Split pane behavior (default) | |
| 427 | + | // Split pane behavior (default) - use splitNode with existing popup pane | |
| 426 | 428 | direction := strings.ToLower(wm.app.config.Workspace.Popups.Placement) | |
| 427 | 429 | if direction == "" { | |
| 428 | 430 | direction = "right" | |
| 429 | 431 | } | |
| 430 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | |
| 432 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { | |
| 431 | 433 | log.Printf("[workspace] Failed to insert split popup pane: %v", err) | |
| 432 | 434 | return | |
| 433 | 435 | } | |
| @@ -439,12 +441,12 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||
| 439 | 441 | log.Printf("[workspace] Failed to stack existing pane: %v", err) | |
| 440 | 442 | return | |
| 441 | 443 | } | |
| 442 | - | // Insert the popup pane in the same container | |
| 444 | + | // Insert the popup pane in the same container - use splitNode with existing popup pane | |
| 443 | 445 | direction := strings.ToLower(wm.app.config.Workspace.Popups.Placement) | |
| 444 | 446 | if direction == "" { | |
| 445 | 447 | direction = "right" | |
| 446 | 448 | } | |
| 447 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | |
| 449 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { | |
| 448 | 450 | log.Printf("[workspace] Failed to insert stacked popup pane: %v", err) | |
| 449 | 451 | return | |
| 450 | 452 | } | |
| @@ -456,7 +458,7 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||
| 456 | 458 | if direction == "" { | |
| 457 | 459 | direction = "right" | |
| 458 | 460 | } | |
| 459 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | |
| 461 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { | |
| 460 | 462 | log.Printf("[workspace] Failed to insert tabbed popup pane: %v", err) | |
| 461 | 463 | return | |
| 462 | 464 | } | |
| @@ -469,7 +471,7 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||
| 469 | 471 | if direction == "" { | |
| 470 | 472 | direction = "right" | |
| 471 | 473 | } | |
| 472 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | |
| 474 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { | |
| 473 | 475 | log.Printf("[workspace] Failed to insert windowed popup pane: %v", err) | |
| 474 | 476 | return | |
| 475 | 477 | } | |
| @@ -481,7 +483,7 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||
| 481 | 483 | if direction == "" { | |
| 482 | 484 | direction = "right" | |
| 483 | 485 | } | |
| 484 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | |
| 486 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { | |
| 485 | 487 | log.Printf("[workspace] Failed to insert popup pane: %v", err) | |
| 486 | 488 | return | |
| 487 | 489 | } | |
| @@ -39,7 +39,8 @@ func (wm *WorkspaceManager) insertIndependentPane(sourceNode *paneNode, webView | |||||
| 39 | if direction == "" { | 39 | if direction == "" { | ||
| 40 | direction = "right" | 40 | direction = "right" | ||
| 41 | } | 41 | } | ||
| 42 | - | if err := wm.insertPopupPane(sourceNode, newPane, direction); err != nil { // reuse insertion primitive | 42 | + | // Use splitNode with existing pane instead of insertPopupPane |
| 43 | + | if _, err := wm.splitNode(sourceNode, direction, newPane); err != nil { | |||
| 43 | return err | 44 | return err | ||
| 44 | } | 45 | } | ||
| 45 | node := wm.viewToNode[webView] | 46 | node := wm.viewToNode[webView] | ||
| @@ -66,7 +67,8 @@ func (wm *WorkspaceManager) configureRelatedPopup(sourceNode *paneNode, webView | |||||
| 66 | if direction == "" { | 67 | if direction == "" { | ||
| 67 | direction = "right" | 68 | direction = "right" | ||
| 68 | } | 69 | } | ||
| 69 | - | if err := wm.insertPopupPane(sourceNode, newPane, direction); err != nil { | 70 | + | // Use splitNode with existing pane instead of insertPopupPane |
| 71 | + | if _, err := wm.splitNode(sourceNode, direction, newPane); err != nil { | |||
| 70 | log.Printf("[workspace] failed to insert related popup pane: %v", err) | 72 | log.Printf("[workspace] failed to insert related popup pane: %v", err) | ||
| 71 | return | 73 | return | ||
| 72 | } | 74 | } | ||
| @@ -422,12 +424,12 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||||
| 422 | 424 | ||||
| 423 | switch behavior { | 425 | switch behavior { | ||
| 424 | case config.PopupBehaviorSplit: | 426 | case config.PopupBehaviorSplit: | ||
| 425 | - | // Split pane behavior (default) | 427 | + | // Split pane behavior (default) - use splitNode with existing popup pane |
| 426 | direction := strings.ToLower(wm.app.config.Workspace.Popups.Placement) | 428 | direction := strings.ToLower(wm.app.config.Workspace.Popups.Placement) | ||
| 427 | if direction == "" { | 429 | if direction == "" { | ||
| 428 | direction = "right" | 430 | direction = "right" | ||
| 429 | } | 431 | } | ||
| 430 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | 432 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { |
| 431 | log.Printf("[workspace] Failed to insert split popup pane: %v", err) | 433 | log.Printf("[workspace] Failed to insert split popup pane: %v", err) | ||
| 432 | return | 434 | return | ||
| 433 | } | 435 | } | ||
| @@ -439,12 +441,12 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||||
| 439 | log.Printf("[workspace] Failed to stack existing pane: %v", err) | 441 | log.Printf("[workspace] Failed to stack existing pane: %v", err) | ||
| 440 | return | 442 | return | ||
| 441 | } | 443 | } | ||
| 442 | - | // Insert the popup pane in the same container | 444 | + | // Insert the popup pane in the same container - use splitNode with existing popup pane |
| 443 | direction := strings.ToLower(wm.app.config.Workspace.Popups.Placement) | 445 | direction := strings.ToLower(wm.app.config.Workspace.Popups.Placement) | ||
| 444 | if direction == "" { | 446 | if direction == "" { | ||
| 445 | direction = "right" | 447 | direction = "right" | ||
| 446 | } | 448 | } | ||
| 447 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | 449 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { |
| 448 | log.Printf("[workspace] Failed to insert stacked popup pane: %v", err) | 450 | log.Printf("[workspace] Failed to insert stacked popup pane: %v", err) | ||
| 449 | return | 451 | return | ||
| 450 | } | 452 | } | ||
| @@ -456,7 +458,7 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||||
| 456 | if direction == "" { | 458 | if direction == "" { | ||
| 457 | direction = "right" | 459 | direction = "right" | ||
| 458 | } | 460 | } | ||
| 459 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | 461 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { |
| 460 | log.Printf("[workspace] Failed to insert tabbed popup pane: %v", err) | 462 | log.Printf("[workspace] Failed to insert tabbed popup pane: %v", err) | ||
| 461 | return | 463 | return | ||
| 462 | } | 464 | } | ||
| @@ -469,7 +471,7 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||||
| 469 | if direction == "" { | 471 | if direction == "" { | ||
| 470 | direction = "right" | 472 | direction = "right" | ||
| 471 | } | 473 | } | ||
| 472 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | 474 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { |
| 473 | log.Printf("[workspace] Failed to insert windowed popup pane: %v", err) | 475 | log.Printf("[workspace] Failed to insert windowed popup pane: %v", err) | ||
| 474 | return | 476 | return | ||
| 475 | } | 477 | } | ||
| @@ -481,7 +483,7 @@ func (wm *WorkspaceManager) handlePopupReadyToShow(popupID uint64) { | |||||
| 481 | if direction == "" { | 483 | if direction == "" { | ||
| 482 | direction = "right" | 484 | direction = "right" | ||
| 483 | } | 485 | } | ||
| 484 | - | if err := wm.insertPopupPane(info.parentNode, popupPane, direction); err != nil { | 486 | + | if _, err := wm.splitNode(info.parentNode, direction, popupPane); err != nil { |
| 485 | log.Printf("[workspace] Failed to insert popup pane: %v", err) | 487 | log.Printf("[workspace] Failed to insert popup pane: %v", err) | ||
| 486 | return | 488 | return | ||
| 487 | } | 489 | } | ||