fix(navigation): resolve coordinate space mixing in workspace navigation
- commit
- 6bf432a50aa7c49878e28ced40ab9cfd911b4ece
- author
- bnema · · 2025-11-13 22:53 UTC
- committer
- bnema · · 2025-11-13 17:38 UTC
- parent
- adbefcc
- signature
- unsigned
3 files · +159 -11
M internal/app/browser/workspace_navigation.go +66 -11
| @@ -208,13 +208,15 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||
| 208 | 208 | } | |
| 209 | 209 | ||
| 210 | 210 | if neighbor := wm.structuralNeighbor(currentFocused, direction); neighbor != nil { | |
| 211 | + | log.Printf("[workspace] focusAdjacent: using structural neighbor for direction=%s", direction) | |
| 211 | 212 | wm.SetActivePane(neighbor, SourceKeyboard) | |
| 212 | 213 | return true | |
| 213 | 214 | } | |
| 214 | 215 | ||
| 215 | - | curX, curY, curWidth, curHeight := webkit.WidgetGetAllocation(currentFocused.container) | |
| 216 | + | curX, curY, curWidth, curHeight := wm.getNavigationAllocation(currentFocused) | |
| 216 | 217 | cx := float64(curX) + float64(curWidth)/2.0 | |
| 217 | 218 | cy := float64(curY) + float64(curHeight)/2.0 | |
| 219 | + | log.Printf("[workspace] focusAdjacent: current pane center=(%.0f, %.0f) direction=%s", cx, cy, direction) | |
| 218 | 220 | ||
| 219 | 221 | leaves := wm.collectLeaves() | |
| 220 | 222 | bestScore := math.MaxFloat64 | |
| @@ -226,7 +228,7 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||
| 226 | 228 | continue | |
| 227 | 229 | } | |
| 228 | 230 | ||
| 229 | - | x, y, width, height := webkit.WidgetGetAllocation(candidate.container) | |
| 231 | + | x, y, width, height := wm.getNavigationAllocation(candidate) | |
| 230 | 232 | tx := float64(x) + float64(width)/2.0 | |
| 231 | 233 | ty := float64(y) + float64(height)/2.0 | |
| 232 | 234 | ||
| @@ -247,18 +249,18 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||
| 247 | 249 | score = math.Abs(dx)*1000 + math.Abs(dy) | |
| 248 | 250 | case DirectionUp: | |
| 249 | 251 | if dy >= -focusEpsilon { | |
| 252 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("SKIPPED cand=%p center=(%.0f,%.0f) dy=%.0f (not above)", candidate.container, tx, ty, dy)) | |
| 250 | 253 | continue | |
| 251 | 254 | } | |
| 252 | 255 | score = math.Abs(dy)*1000 + math.Abs(dx) | |
| 256 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("cand=%p center=(%.0f,%.0f) dy=%.0f score=%.0f", candidate.container, tx, ty, dy, score)) | |
| 253 | 257 | case DirectionDown: | |
| 254 | 258 | if dy <= focusEpsilon { | |
| 259 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("SKIPPED cand=%p center=(%.0f,%.0f) dy=%.0f (not below)", candidate.container, tx, ty, dy)) | |
| 255 | 260 | continue | |
| 256 | 261 | } | |
| 257 | 262 | score = math.Abs(dy)*1000 + math.Abs(dx) | |
| 258 | - | } | |
| 259 | - | ||
| 260 | - | if direction == DirectionUp || direction == DirectionDown { | |
| 261 | - | debugCandidates = append(debugCandidates, fmt.Sprintf("cand=%p dx=%.2f dy=%.2f score=%.2f", candidate.container, dx, dy, score)) | |
| 263 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("cand=%p center=(%.0f,%.0f) dy=%.0f score=%.0f", candidate.container, tx, ty, dy, score)) | |
| 262 | 264 | } | |
| 263 | 265 | ||
| 264 | 266 | if score < bestScore { | |
| @@ -268,39 +270,83 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||
| 268 | 270 | } | |
| 269 | 271 | ||
| 270 | 272 | if best != nil { | |
| 273 | + | bx, by, _, _ := webkit.WidgetGetAllocation(best.container) | |
| 274 | + | log.Printf("[workspace] focusAdjacent: found best candidate at pos=(%d,%d) for direction=%s", bx, by, direction) | |
| 271 | 275 | wm.SetActivePane(best, SourceKeyboard) | |
| 272 | 276 | return true | |
| 273 | 277 | } | |
| 274 | 278 | ||
| 275 | 279 | if len(debugCandidates) > 0 { | |
| 276 | - | log.Printf("[workspace] focusAdjacent no candidate direction=%s current=%p candidates=%s", direction, currentFocused.container, strings.Join(debugCandidates, "; ")) | |
| 280 | + | log.Printf("[workspace] focusAdjacent: NO candidate found for direction=%s current=%p center=(%.0f,%.0f) candidates=[%s]", direction, currentFocused.container, cx, cy, strings.Join(debugCandidates, "; ")) | |
| 277 | 281 | } | |
| 278 | 282 | return false | |
| 279 | 283 | } | |
| 280 | 284 | ||
| 285 | + | // getNavigationAllocation returns window-absolute coordinates for navigation geometric checks. | |
| 286 | + | // Uses ComputeBounds relative to the window root to get actual screen positions. | |
| 287 | + | // For panes inside a stack, returns the stack wrapper's bounds. | |
| 288 | + | // For regular panes, returns the pane's own bounds. | |
| 289 | + | func (wm *WorkspaceManager) getNavigationAllocation(node *paneNode) (x, y, width, height int) { | |
| 290 | + | if node == nil || node.container == nil { | |
| 291 | + | return 0, 0, 0, 0 | |
| 292 | + | } | |
| 293 | + | ||
| 294 | + | // If this pane is inside a stack, use the stack wrapper's window bounds | |
| 295 | + | if node.parent != nil && node.parent.isStacked && node.parent.stackWrapper != nil { | |
| 296 | + | return webkit.WidgetGetWindowBounds(node.parent.stackWrapper) | |
| 297 | + | } | |
| 298 | + | ||
| 299 | + | // For regular panes or stack containers themselves, use their own window bounds | |
| 300 | + | return webkit.WidgetGetWindowBounds(node.container) | |
| 301 | + | } | |
| 302 | + | ||
| 281 | 303 | // structuralNeighbor finds neighbors based on the tree structure rather than geometry | |
| 282 | 304 | func (wm *WorkspaceManager) structuralNeighbor(node *paneNode, direction string) *paneNode { | |
| 283 | 305 | if node == nil || node.container == nil { | |
| 284 | 306 | return nil | |
| 285 | 307 | } | |
| 286 | 308 | ||
| 287 | - | refX, refY, refWidth, refHeight := webkit.WidgetGetAllocation(node.container) | |
| 309 | + | refX, refY, refWidth, refHeight := wm.getNavigationAllocation(node) | |
| 288 | 310 | cx := float64(refX) + float64(refWidth)/2.0 | |
| 289 | 311 | cy := float64(refY) + float64(refHeight)/2.0 | |
| 290 | 312 | axisVertical := direction == DirectionUp || direction == DirectionDown | |
| 291 | 313 | ||
| 292 | - | for parent := node.parent; parent != nil; parent = parent.parent { | |
| 314 | + | log.Printf("[workspace] structuralNeighbor: node=%p pos=(%d,%d) direction=%s", node.container, refX, refY, direction) | |
| 315 | + | ||
| 316 | + | // Start from the node's parent, but skip stack containers for vertical navigation | |
| 317 | + | // Stack containers are transparent to vertical navigation - we want to navigate | |
| 318 | + | // from stack to external panes, not within the stack (that's handled by NavigateStack) | |
| 319 | + | startParent := node.parent | |
| 320 | + | if startParent != nil && startParent.isStacked && axisVertical { | |
| 321 | + | log.Printf("[workspace] structuralNeighbor: skipping stack container parent, using stack's parent instead") | |
| 322 | + | node = startParent // Treat the stack container as the navigation node | |
| 323 | + | startParent = startParent.parent | |
| 324 | + | } | |
| 325 | + | ||
| 326 | + | for parent := startParent; parent != nil; parent = parent.parent { | |
| 327 | + | isLeft := parent.left == node | |
| 328 | + | isRight := parent.right == node | |
| 329 | + | log.Printf("[workspace] structuralNeighbor: checking parent orientation=%v isLeft=%v isRight=%v", parent.orientation, isLeft, isRight) | |
| 330 | + | ||
| 293 | 331 | switch direction { | |
| 294 | 332 | case DirectionUp: | |
| 295 | 333 | if axisVertical && parent.orientation == gtk.OrientationVertical && parent.right == node { | |
| 334 | + | log.Printf("[workspace] structuralNeighbor: DirectionUp - we are RIGHT child, looking in LEFT subtree") | |
| 296 | 335 | if leaf := wm.closestLeafFromSubtree(parent.left, cx, cy, direction); leaf != nil { | |
| 336 | + | lx, ly, _, _ := webkit.WidgetGetAllocation(leaf.container) | |
| 337 | + | log.Printf("[workspace] structuralNeighbor: found leaf at pos=(%d,%d)", lx, ly) | |
| 297 | 338 | return leaf | |
| 298 | 339 | } | |
| 299 | 340 | } | |
| 300 | 341 | case DirectionDown: | |
| 301 | 342 | if axisVertical && parent.orientation == gtk.OrientationVertical && parent.left == node { | |
| 343 | + | log.Printf("[workspace] structuralNeighbor: DirectionDown - we are LEFT child, looking in RIGHT subtree") | |
| 302 | 344 | if leaf := wm.closestLeafFromSubtree(parent.right, cx, cy, direction); leaf != nil { | |
| 345 | + | lx, ly, _, _ := wm.getNavigationAllocation(leaf) | |
| 346 | + | log.Printf("[workspace] structuralNeighbor: found leaf at pos=(%d,%d)", lx, ly) | |
| 303 | 347 | return leaf | |
| 348 | + | } else { | |
| 349 | + | log.Printf("[workspace] structuralNeighbor: closestLeafFromSubtree returned nil for DirectionDown") | |
| 304 | 350 | } | |
| 305 | 351 | } | |
| 306 | 352 | case DirectionLeft: | |
| @@ -323,7 +369,8 @@ func (wm *WorkspaceManager) structuralNeighbor(node *paneNode, direction string) | |||
| 323 | 369 | ||
| 324 | 370 | // closestLeafFromSubtree finds the closest leaf node in a subtree based on direction | |
| 325 | 371 | func (wm *WorkspaceManager) closestLeafFromSubtree(node *paneNode, cx, cy float64, direction string) *paneNode { | |
| 326 | - | leaves := wm.collectLeavesFrom(node) | |
| 372 | + | leaves := wm.collectLeavesFromWithDirection(node, direction) | |
| 373 | + | log.Printf("[workspace] closestLeafFromSubtree: found %d leaves for direction=%s from cx=%.0f cy=%.0f", len(leaves), direction, cx, cy) | |
| 327 | 374 | bestScore := math.MaxFloat64 | |
| 328 | 375 | var best *paneNode | |
| 329 | 376 | for _, leaf := range leaves { | |
| @@ -331,44 +378,52 @@ func (wm *WorkspaceManager) closestLeafFromSubtree(node *paneNode, cx, cy float6 | |||
| 331 | 378 | continue | |
| 332 | 379 | } | |
| 333 | 380 | ||
| 334 | - | x, y, width, height := webkit.WidgetGetAllocation(leaf.container) | |
| 381 | + | x, y, width, height := wm.getNavigationAllocation(leaf) | |
| 335 | 382 | tx := float64(x) + float64(width)/2.0 | |
| 336 | 383 | ty := float64(y) + float64(height)/2.0 | |
| 337 | 384 | dx := tx - cx | |
| 338 | 385 | dy := ty - cy | |
| 386 | + | log.Printf("[workspace] closestLeafFromSubtree: leaf=%p pos=(%d,%d) center=(%.0f,%.0f) dx=%.0f dy=%.0f", leaf.container, x, y, tx, ty, dx, dy) | |
| 339 | 387 | var score float64 | |
| 340 | 388 | switch direction { | |
| 341 | 389 | case DirectionLeft: | |
| 342 | 390 | if dx >= -focusEpsilon { | |
| 391 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dx=%.0f not left)", dx) | |
| 343 | 392 | continue | |
| 344 | 393 | } | |
| 345 | 394 | score = math.Abs(dx)*1000 + math.Abs(dy) | |
| 346 | 395 | case DirectionRight: | |
| 347 | 396 | if dx <= focusEpsilon { | |
| 397 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dx=%.0f not right)", dx) | |
| 348 | 398 | continue | |
| 349 | 399 | } | |
| 350 | 400 | score = math.Abs(dx)*1000 + math.Abs(dy) | |
| 351 | 401 | case DirectionUp: | |
| 352 | 402 | if dy >= -focusEpsilon { | |
| 403 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dy=%.0f not above)", dy) | |
| 353 | 404 | continue | |
| 354 | 405 | } | |
| 355 | 406 | score = math.Abs(dy)*1000 + math.Abs(dx) | |
| 356 | 407 | case DirectionDown: | |
| 357 | 408 | if dy <= focusEpsilon { | |
| 409 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dy=%.0f not below)", dy) | |
| 358 | 410 | continue | |
| 359 | 411 | } | |
| 360 | 412 | score = math.Abs(dy)*1000 + math.Abs(dx) | |
| 361 | 413 | default: | |
| 362 | 414 | continue | |
| 363 | 415 | } | |
| 416 | + | log.Printf("[workspace] closestLeafFromSubtree: ACCEPTED with score=%.0f", score) | |
| 364 | 417 | if score < bestScore { | |
| 365 | 418 | bestScore = score | |
| 366 | 419 | best = leaf | |
| 367 | 420 | } | |
| 368 | 421 | } | |
| 369 | 422 | if best == nil { | |
| 423 | + | log.Printf("[workspace] closestLeafFromSubtree: no match found, trying boundaryFallback") | |
| 370 | 424 | return wm.boundaryFallback(node, direction) | |
| 371 | 425 | } | |
| 426 | + | log.Printf("[workspace] closestLeafFromSubtree: returning best=%p", best.container) | |
| 372 | 427 | return best | |
| 373 | 428 | } | |
| 374 | 429 | ||
| @@ -208,13 +208,15 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||||
| 208 | } | 208 | } | ||
| 209 | 209 | ||||
| 210 | if neighbor := wm.structuralNeighbor(currentFocused, direction); neighbor != nil { | 210 | if neighbor := wm.structuralNeighbor(currentFocused, direction); neighbor != nil { | ||
| 211 | + | log.Printf("[workspace] focusAdjacent: using structural neighbor for direction=%s", direction) | |||
| 211 | wm.SetActivePane(neighbor, SourceKeyboard) | 212 | wm.SetActivePane(neighbor, SourceKeyboard) | ||
| 212 | return true | 213 | return true | ||
| 213 | } | 214 | } | ||
| 214 | 215 | ||||
| 215 | - | curX, curY, curWidth, curHeight := webkit.WidgetGetAllocation(currentFocused.container) | 216 | + | curX, curY, curWidth, curHeight := wm.getNavigationAllocation(currentFocused) |
| 216 | cx := float64(curX) + float64(curWidth)/2.0 | 217 | cx := float64(curX) + float64(curWidth)/2.0 | ||
| 217 | cy := float64(curY) + float64(curHeight)/2.0 | 218 | cy := float64(curY) + float64(curHeight)/2.0 | ||
| 219 | + | log.Printf("[workspace] focusAdjacent: current pane center=(%.0f, %.0f) direction=%s", cx, cy, direction) | |||
| 218 | 220 | ||||
| 219 | leaves := wm.collectLeaves() | 221 | leaves := wm.collectLeaves() | ||
| 220 | bestScore := math.MaxFloat64 | 222 | bestScore := math.MaxFloat64 | ||
| @@ -226,7 +228,7 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||||
| 226 | continue | 228 | continue | ||
| 227 | } | 229 | } | ||
| 228 | 230 | ||||
| 229 | - | x, y, width, height := webkit.WidgetGetAllocation(candidate.container) | 231 | + | x, y, width, height := wm.getNavigationAllocation(candidate) |
| 230 | tx := float64(x) + float64(width)/2.0 | 232 | tx := float64(x) + float64(width)/2.0 | ||
| 231 | ty := float64(y) + float64(height)/2.0 | 233 | ty := float64(y) + float64(height)/2.0 | ||
| 232 | 234 | ||||
| @@ -247,18 +249,18 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||||
| 247 | score = math.Abs(dx)*1000 + math.Abs(dy) | 249 | score = math.Abs(dx)*1000 + math.Abs(dy) | ||
| 248 | case DirectionUp: | 250 | case DirectionUp: | ||
| 249 | if dy >= -focusEpsilon { | 251 | if dy >= -focusEpsilon { | ||
| 252 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("SKIPPED cand=%p center=(%.0f,%.0f) dy=%.0f (not above)", candidate.container, tx, ty, dy)) | |||
| 250 | continue | 253 | continue | ||
| 251 | } | 254 | } | ||
| 252 | score = math.Abs(dy)*1000 + math.Abs(dx) | 255 | score = math.Abs(dy)*1000 + math.Abs(dx) | ||
| 256 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("cand=%p center=(%.0f,%.0f) dy=%.0f score=%.0f", candidate.container, tx, ty, dy, score)) | |||
| 253 | case DirectionDown: | 257 | case DirectionDown: | ||
| 254 | if dy <= focusEpsilon { | 258 | if dy <= focusEpsilon { | ||
| 259 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("SKIPPED cand=%p center=(%.0f,%.0f) dy=%.0f (not below)", candidate.container, tx, ty, dy)) | |||
| 255 | continue | 260 | continue | ||
| 256 | } | 261 | } | ||
| 257 | score = math.Abs(dy)*1000 + math.Abs(dx) | 262 | score = math.Abs(dy)*1000 + math.Abs(dx) | ||
| 258 | - | } | 263 | + | debugCandidates = append(debugCandidates, fmt.Sprintf("cand=%p center=(%.0f,%.0f) dy=%.0f score=%.0f", candidate.container, tx, ty, dy, score)) |
| 259 | - | ||||
| 260 | - | if direction == DirectionUp || direction == DirectionDown { | |||
| 261 | - | debugCandidates = append(debugCandidates, fmt.Sprintf("cand=%p dx=%.2f dy=%.2f score=%.2f", candidate.container, dx, dy, score)) | |||
| 262 | } | 264 | } | ||
| 263 | 265 | ||||
| 264 | if score < bestScore { | 266 | if score < bestScore { | ||
| @@ -268,39 +270,83 @@ func (wm *WorkspaceManager) focusAdjacent(direction string) bool { | |||||
| 268 | } | 270 | } | ||
| 269 | 271 | ||||
| 270 | if best != nil { | 272 | if best != nil { | ||
| 273 | + | bx, by, _, _ := webkit.WidgetGetAllocation(best.container) | |||
| 274 | + | log.Printf("[workspace] focusAdjacent: found best candidate at pos=(%d,%d) for direction=%s", bx, by, direction) | |||
| 271 | wm.SetActivePane(best, SourceKeyboard) | 275 | wm.SetActivePane(best, SourceKeyboard) | ||
| 272 | return true | 276 | return true | ||
| 273 | } | 277 | } | ||
| 274 | 278 | ||||
| 275 | if len(debugCandidates) > 0 { | 279 | if len(debugCandidates) > 0 { | ||
| 276 | - | log.Printf("[workspace] focusAdjacent no candidate direction=%s current=%p candidates=%s", direction, currentFocused.container, strings.Join(debugCandidates, "; ")) | 280 | + | log.Printf("[workspace] focusAdjacent: NO candidate found for direction=%s current=%p center=(%.0f,%.0f) candidates=[%s]", direction, currentFocused.container, cx, cy, strings.Join(debugCandidates, "; ")) |
| 277 | } | 281 | } | ||
| 278 | return false | 282 | return false | ||
| 279 | } | 283 | } | ||
| 280 | 284 | ||||
| 285 | + | // getNavigationAllocation returns window-absolute coordinates for navigation geometric checks. | |||
| 286 | + | // Uses ComputeBounds relative to the window root to get actual screen positions. | |||
| 287 | + | // For panes inside a stack, returns the stack wrapper's bounds. | |||
| 288 | + | // For regular panes, returns the pane's own bounds. | |||
| 289 | + | func (wm *WorkspaceManager) getNavigationAllocation(node *paneNode) (x, y, width, height int) { | |||
| 290 | + | if node == nil || node.container == nil { | |||
| 291 | + | return 0, 0, 0, 0 | |||
| 292 | + | } | |||
| 293 | + | ||||
| 294 | + | // If this pane is inside a stack, use the stack wrapper's window bounds | |||
| 295 | + | if node.parent != nil && node.parent.isStacked && node.parent.stackWrapper != nil { | |||
| 296 | + | return webkit.WidgetGetWindowBounds(node.parent.stackWrapper) | |||
| 297 | + | } | |||
| 298 | + | ||||
| 299 | + | // For regular panes or stack containers themselves, use their own window bounds | |||
| 300 | + | return webkit.WidgetGetWindowBounds(node.container) | |||
| 301 | + | } | |||
| 302 | + | ||||
| 281 | // structuralNeighbor finds neighbors based on the tree structure rather than geometry | 303 | // structuralNeighbor finds neighbors based on the tree structure rather than geometry | ||
| 282 | func (wm *WorkspaceManager) structuralNeighbor(node *paneNode, direction string) *paneNode { | 304 | func (wm *WorkspaceManager) structuralNeighbor(node *paneNode, direction string) *paneNode { | ||
| 283 | if node == nil || node.container == nil { | 305 | if node == nil || node.container == nil { | ||
| 284 | return nil | 306 | return nil | ||
| 285 | } | 307 | } | ||
| 286 | 308 | ||||
| 287 | - | refX, refY, refWidth, refHeight := webkit.WidgetGetAllocation(node.container) | 309 | + | refX, refY, refWidth, refHeight := wm.getNavigationAllocation(node) |
| 288 | cx := float64(refX) + float64(refWidth)/2.0 | 310 | cx := float64(refX) + float64(refWidth)/2.0 | ||
| 289 | cy := float64(refY) + float64(refHeight)/2.0 | 311 | cy := float64(refY) + float64(refHeight)/2.0 | ||
| 290 | axisVertical := direction == DirectionUp || direction == DirectionDown | 312 | axisVertical := direction == DirectionUp || direction == DirectionDown | ||
| 291 | 313 | ||||
| 292 | - | for parent := node.parent; parent != nil; parent = parent.parent { | 314 | + | log.Printf("[workspace] structuralNeighbor: node=%p pos=(%d,%d) direction=%s", node.container, refX, refY, direction) |
| 315 | + | ||||
| 316 | + | // Start from the node's parent, but skip stack containers for vertical navigation | |||
| 317 | + | // Stack containers are transparent to vertical navigation - we want to navigate | |||
| 318 | + | // from stack to external panes, not within the stack (that's handled by NavigateStack) | |||
| 319 | + | startParent := node.parent | |||
| 320 | + | if startParent != nil && startParent.isStacked && axisVertical { | |||
| 321 | + | log.Printf("[workspace] structuralNeighbor: skipping stack container parent, using stack's parent instead") | |||
| 322 | + | node = startParent // Treat the stack container as the navigation node | |||
| 323 | + | startParent = startParent.parent | |||
| 324 | + | } | |||
| 325 | + | ||||
| 326 | + | for parent := startParent; parent != nil; parent = parent.parent { | |||
| 327 | + | isLeft := parent.left == node | |||
| 328 | + | isRight := parent.right == node | |||
| 329 | + | log.Printf("[workspace] structuralNeighbor: checking parent orientation=%v isLeft=%v isRight=%v", parent.orientation, isLeft, isRight) | |||
| 330 | + | ||||
| 293 | switch direction { | 331 | switch direction { | ||
| 294 | case DirectionUp: | 332 | case DirectionUp: | ||
| 295 | if axisVertical && parent.orientation == gtk.OrientationVertical && parent.right == node { | 333 | if axisVertical && parent.orientation == gtk.OrientationVertical && parent.right == node { | ||
| 334 | + | log.Printf("[workspace] structuralNeighbor: DirectionUp - we are RIGHT child, looking in LEFT subtree") | |||
| 296 | if leaf := wm.closestLeafFromSubtree(parent.left, cx, cy, direction); leaf != nil { | 335 | if leaf := wm.closestLeafFromSubtree(parent.left, cx, cy, direction); leaf != nil { | ||
| 336 | + | lx, ly, _, _ := webkit.WidgetGetAllocation(leaf.container) | |||
| 337 | + | log.Printf("[workspace] structuralNeighbor: found leaf at pos=(%d,%d)", lx, ly) | |||
| 297 | return leaf | 338 | return leaf | ||
| 298 | } | 339 | } | ||
| 299 | } | 340 | } | ||
| 300 | case DirectionDown: | 341 | case DirectionDown: | ||
| 301 | if axisVertical && parent.orientation == gtk.OrientationVertical && parent.left == node { | 342 | if axisVertical && parent.orientation == gtk.OrientationVertical && parent.left == node { | ||
| 343 | + | log.Printf("[workspace] structuralNeighbor: DirectionDown - we are LEFT child, looking in RIGHT subtree") | |||
| 302 | if leaf := wm.closestLeafFromSubtree(parent.right, cx, cy, direction); leaf != nil { | 344 | if leaf := wm.closestLeafFromSubtree(parent.right, cx, cy, direction); leaf != nil { | ||
| 345 | + | lx, ly, _, _ := wm.getNavigationAllocation(leaf) | |||
| 346 | + | log.Printf("[workspace] structuralNeighbor: found leaf at pos=(%d,%d)", lx, ly) | |||
| 303 | return leaf | 347 | return leaf | ||
| 348 | + | } else { | |||
| 349 | + | log.Printf("[workspace] structuralNeighbor: closestLeafFromSubtree returned nil for DirectionDown") | |||
| 304 | } | 350 | } | ||
| 305 | } | 351 | } | ||
| 306 | case DirectionLeft: | 352 | case DirectionLeft: | ||
| @@ -323,7 +369,8 @@ func (wm *WorkspaceManager) structuralNeighbor(node *paneNode, direction string) | |||||
| 323 | 369 | ||||
| 324 | // closestLeafFromSubtree finds the closest leaf node in a subtree based on direction | 370 | // closestLeafFromSubtree finds the closest leaf node in a subtree based on direction | ||
| 325 | func (wm *WorkspaceManager) closestLeafFromSubtree(node *paneNode, cx, cy float64, direction string) *paneNode { | 371 | func (wm *WorkspaceManager) closestLeafFromSubtree(node *paneNode, cx, cy float64, direction string) *paneNode { | ||
| 326 | - | leaves := wm.collectLeavesFrom(node) | 372 | + | leaves := wm.collectLeavesFromWithDirection(node, direction) |
| 373 | + | log.Printf("[workspace] closestLeafFromSubtree: found %d leaves for direction=%s from cx=%.0f cy=%.0f", len(leaves), direction, cx, cy) | |||
| 327 | bestScore := math.MaxFloat64 | 374 | bestScore := math.MaxFloat64 | ||
| 328 | var best *paneNode | 375 | var best *paneNode | ||
| 329 | for _, leaf := range leaves { | 376 | for _, leaf := range leaves { | ||
| @@ -331,44 +378,52 @@ func (wm *WorkspaceManager) closestLeafFromSubtree(node *paneNode, cx, cy float6 | |||||
| 331 | continue | 378 | continue | ||
| 332 | } | 379 | } | ||
| 333 | 380 | ||||
| 334 | - | x, y, width, height := webkit.WidgetGetAllocation(leaf.container) | 381 | + | x, y, width, height := wm.getNavigationAllocation(leaf) |
| 335 | tx := float64(x) + float64(width)/2.0 | 382 | tx := float64(x) + float64(width)/2.0 | ||
| 336 | ty := float64(y) + float64(height)/2.0 | 383 | ty := float64(y) + float64(height)/2.0 | ||
| 337 | dx := tx - cx | 384 | dx := tx - cx | ||
| 338 | dy := ty - cy | 385 | dy := ty - cy | ||
| 386 | + | log.Printf("[workspace] closestLeafFromSubtree: leaf=%p pos=(%d,%d) center=(%.0f,%.0f) dx=%.0f dy=%.0f", leaf.container, x, y, tx, ty, dx, dy) | |||
| 339 | var score float64 | 387 | var score float64 | ||
| 340 | switch direction { | 388 | switch direction { | ||
| 341 | case DirectionLeft: | 389 | case DirectionLeft: | ||
| 342 | if dx >= -focusEpsilon { | 390 | if dx >= -focusEpsilon { | ||
| 391 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dx=%.0f not left)", dx) | |||
| 343 | continue | 392 | continue | ||
| 344 | } | 393 | } | ||
| 345 | score = math.Abs(dx)*1000 + math.Abs(dy) | 394 | score = math.Abs(dx)*1000 + math.Abs(dy) | ||
| 346 | case DirectionRight: | 395 | case DirectionRight: | ||
| 347 | if dx <= focusEpsilon { | 396 | if dx <= focusEpsilon { | ||
| 397 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dx=%.0f not right)", dx) | |||
| 348 | continue | 398 | continue | ||
| 349 | } | 399 | } | ||
| 350 | score = math.Abs(dx)*1000 + math.Abs(dy) | 400 | score = math.Abs(dx)*1000 + math.Abs(dy) | ||
| 351 | case DirectionUp: | 401 | case DirectionUp: | ||
| 352 | if dy >= -focusEpsilon { | 402 | if dy >= -focusEpsilon { | ||
| 403 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dy=%.0f not above)", dy) | |||
| 353 | continue | 404 | continue | ||
| 354 | } | 405 | } | ||
| 355 | score = math.Abs(dy)*1000 + math.Abs(dx) | 406 | score = math.Abs(dy)*1000 + math.Abs(dx) | ||
| 356 | case DirectionDown: | 407 | case DirectionDown: | ||
| 357 | if dy <= focusEpsilon { | 408 | if dy <= focusEpsilon { | ||
| 409 | + | log.Printf("[workspace] closestLeafFromSubtree: SKIPPED (dy=%.0f not below)", dy) | |||
| 358 | continue | 410 | continue | ||
| 359 | } | 411 | } | ||
| 360 | score = math.Abs(dy)*1000 + math.Abs(dx) | 412 | score = math.Abs(dy)*1000 + math.Abs(dx) | ||
| 361 | default: | 413 | default: | ||
| 362 | continue | 414 | continue | ||
| 363 | } | 415 | } | ||
| 416 | + | log.Printf("[workspace] closestLeafFromSubtree: ACCEPTED with score=%.0f", score) | |||
| 364 | if score < bestScore { | 417 | if score < bestScore { | ||
| 365 | bestScore = score | 418 | bestScore = score | ||
| 366 | best = leaf | 419 | best = leaf | ||
| 367 | } | 420 | } | ||
| 368 | } | 421 | } | ||
| 369 | if best == nil { | 422 | if best == nil { | ||
| 423 | + | log.Printf("[workspace] closestLeafFromSubtree: no match found, trying boundaryFallback") | |||
| 370 | return wm.boundaryFallback(node, direction) | 424 | return wm.boundaryFallback(node, direction) | ||
| 371 | } | 425 | } | ||
| 426 | + | log.Printf("[workspace] closestLeafFromSubtree: returning best=%p", best.container) | |||
| 372 | return best | 427 | return best | ||
| 373 | } | 428 | } | ||
| 374 | 429 | ||||
M internal/app/browser/workspace_navigation_test.go +76 -0
| @@ -144,3 +144,79 @@ func TestCaseSensitiveDirections(t *testing.T) { | |||
| 144 | 144 | }) | |
| 145 | 145 | } | |
| 146 | 146 | } | |
| 147 | + | ||
| 148 | + | // TestNavigationCoordinateBug documents the coordinate space bug in navigation | |
| 149 | + | // | |
| 150 | + | // Bug Description: | |
| 151 | + | // When comparing positions for geometric navigation, the code mixes two coordinate spaces: | |
| 152 | + | // - Normal panes: return window-absolute coordinates from WidgetGetAllocation() | |
| 153 | + | // - Stacked panes: return stack-box-relative coordinates from WidgetGetAllocation() | |
| 154 | + | // | |
| 155 | + | // This causes navigation to fail because we're comparing coordinates in different spaces. | |
| 156 | + | // | |
| 157 | + | // Real-world example from logs: | |
| 158 | + | // Normal pane (top half): center=(957, 532) - window coords | |
| 159 | + | // Stack pane (bottom): center=(955, 511) - relative to stack box! | |
| 160 | + | // Result: dy=-21 → SKIPPED as "not below" | |
| 161 | + | // | |
| 162 | + | // The fix: Always use stack wrapper allocation for panes inside stacks. | |
| 163 | + | func TestNavigationCoordinateBug(t *testing.T) { | |
| 164 | + | // Simulate the coordinate math from real user scenario | |
| 165 | + | // Window: 1914x1064, vertical split: normal pane (top) + stack (bottom) | |
| 166 | + | ||
| 167 | + | t.Run("BugValidation_CoordinateMixing", func(t *testing.T) { | |
| 168 | + | // Normal pane occupies top half - window-absolute coords | |
| 169 | + | normalCX, normalCY := 957.0, 266.0 | |
| 170 | + | ||
| 171 | + | // Stack pane child returns RELATIVE coordinates (bug!) | |
| 172 | + | // Its allocation is relative to stack box at y=532 | |
| 173 | + | stackChildCX, stackChildCY := 957.0, 133.0 // Relative to parent! | |
| 174 | + | ||
| 175 | + | dy := stackChildCY - normalCY | |
| 176 | + | t.Logf("Normal pane: center=(%.0f, %.0f)", normalCX, normalCY) | |
| 177 | + | t.Logf("Stack child (buggy relative): center=(%.0f, %.0f)", stackChildCX, stackChildCY) | |
| 178 | + | t.Logf("dy = %.0f (negative = appears above)", dy) | |
| 179 | + | ||
| 180 | + | // Navigation DOWN requires dy > 0, but we get negative! | |
| 181 | + | const focusEpsilon = 0.5 | |
| 182 | + | if dy <= focusEpsilon { | |
| 183 | + | t.Logf("✓ BUG: dy=%.0f, navigation DOWN would FAIL", dy) | |
| 184 | + | } | |
| 185 | + | }) | |
| 186 | + | ||
| 187 | + | t.Run("CorrectBehavior_UseStackWrapper", func(t *testing.T) { | |
| 188 | + | // Normal pane (same as before) | |
| 189 | + | normalCX, normalCY := 957.0, 266.0 | |
| 190 | + | ||
| 191 | + | // Stack wrapper has correct window-absolute coordinates | |
| 192 | + | stackWrapperCX, stackWrapperCY := 957.0, 798.0 | |
| 193 | + | ||
| 194 | + | dy := stackWrapperCY - normalCY | |
| 195 | + | t.Logf("Normal pane: center=(%.0f, %.0f)", normalCX, normalCY) | |
| 196 | + | t.Logf("Stack wrapper (correct): center=(%.0f, %.0f)", stackWrapperCX, stackWrapperCY) | |
| 197 | + | t.Logf("dy = %.0f (positive = correctly below)", dy) | |
| 198 | + | ||
| 199 | + | const focusEpsilon = 0.5 | |
| 200 | + | if dy > focusEpsilon { | |
| 201 | + | t.Logf("✓ CORRECT: Navigation DOWN would SUCCEED") | |
| 202 | + | } | |
| 203 | + | ||
| 204 | + | // Also verify UP navigation | |
| 205 | + | dyUp := normalCY - stackWrapperCY | |
| 206 | + | if dyUp < -focusEpsilon { | |
| 207 | + | t.Logf("✓ CORRECT: Navigation UP would SUCCEED") | |
| 208 | + | } | |
| 209 | + | }) | |
| 210 | + | } | |
| 211 | + | ||
| 212 | + | // TestGetNavigationAllocation tests the proposed fix helper function | |
| 213 | + | // This will be uncommented and updated after implementing the helper | |
| 214 | + | func TestGetNavigationAllocation(t *testing.T) { | |
| 215 | + | t.Skip("Enable this test after implementing getNavigationAllocation() helper") | |
| 216 | + | ||
| 217 | + | // TODO: After implementing getNavigationAllocation(), test: | |
| 218 | + | // 1. Regular leaf pane → returns pane.container allocation | |
| 219 | + | // 2. Pane inside stack → returns stack.stackWrapper allocation | |
| 220 | + | // 3. Stack container itself → returns stack.stackWrapper allocation | |
| 221 | + | // 4. Nil safety: node=nil, container=nil, parent=nil, stackWrapper=nil | |
| 222 | + | } | |
| @@ -144,3 +144,79 @@ func TestCaseSensitiveDirections(t *testing.T) { | |||||
| 144 | }) | 144 | }) | ||
| 145 | } | 145 | } | ||
| 146 | } | 146 | } | ||
| 147 | + | ||||
| 148 | + | // TestNavigationCoordinateBug documents the coordinate space bug in navigation | |||
| 149 | + | // | |||
| 150 | + | // Bug Description: | |||
| 151 | + | // When comparing positions for geometric navigation, the code mixes two coordinate spaces: | |||
| 152 | + | // - Normal panes: return window-absolute coordinates from WidgetGetAllocation() | |||
| 153 | + | // - Stacked panes: return stack-box-relative coordinates from WidgetGetAllocation() | |||
| 154 | + | // | |||
| 155 | + | // This causes navigation to fail because we're comparing coordinates in different spaces. | |||
| 156 | + | // | |||
| 157 | + | // Real-world example from logs: | |||
| 158 | + | // Normal pane (top half): center=(957, 532) - window coords | |||
| 159 | + | // Stack pane (bottom): center=(955, 511) - relative to stack box! | |||
| 160 | + | // Result: dy=-21 → SKIPPED as "not below" | |||
| 161 | + | // | |||
| 162 | + | // The fix: Always use stack wrapper allocation for panes inside stacks. | |||
| 163 | + | func TestNavigationCoordinateBug(t *testing.T) { | |||
| 164 | + | // Simulate the coordinate math from real user scenario | |||
| 165 | + | // Window: 1914x1064, vertical split: normal pane (top) + stack (bottom) | |||
| 166 | + | ||||
| 167 | + | t.Run("BugValidation_CoordinateMixing", func(t *testing.T) { | |||
| 168 | + | // Normal pane occupies top half - window-absolute coords | |||
| 169 | + | normalCX, normalCY := 957.0, 266.0 | |||
| 170 | + | ||||
| 171 | + | // Stack pane child returns RELATIVE coordinates (bug!) | |||
| 172 | + | // Its allocation is relative to stack box at y=532 | |||
| 173 | + | stackChildCX, stackChildCY := 957.0, 133.0 // Relative to parent! | |||
| 174 | + | ||||
| 175 | + | dy := stackChildCY - normalCY | |||
| 176 | + | t.Logf("Normal pane: center=(%.0f, %.0f)", normalCX, normalCY) | |||
| 177 | + | t.Logf("Stack child (buggy relative): center=(%.0f, %.0f)", stackChildCX, stackChildCY) | |||
| 178 | + | t.Logf("dy = %.0f (negative = appears above)", dy) | |||
| 179 | + | ||||
| 180 | + | // Navigation DOWN requires dy > 0, but we get negative! | |||
| 181 | + | const focusEpsilon = 0.5 | |||
| 182 | + | if dy <= focusEpsilon { | |||
| 183 | + | t.Logf("✓ BUG: dy=%.0f, navigation DOWN would FAIL", dy) | |||
| 184 | + | } | |||
| 185 | + | }) | |||
| 186 | + | ||||
| 187 | + | t.Run("CorrectBehavior_UseStackWrapper", func(t *testing.T) { | |||
| 188 | + | // Normal pane (same as before) | |||
| 189 | + | normalCX, normalCY := 957.0, 266.0 | |||
| 190 | + | ||||
| 191 | + | // Stack wrapper has correct window-absolute coordinates | |||
| 192 | + | stackWrapperCX, stackWrapperCY := 957.0, 798.0 | |||
| 193 | + | ||||
| 194 | + | dy := stackWrapperCY - normalCY | |||
| 195 | + | t.Logf("Normal pane: center=(%.0f, %.0f)", normalCX, normalCY) | |||
| 196 | + | t.Logf("Stack wrapper (correct): center=(%.0f, %.0f)", stackWrapperCX, stackWrapperCY) | |||
| 197 | + | t.Logf("dy = %.0f (positive = correctly below)", dy) | |||
| 198 | + | ||||
| 199 | + | const focusEpsilon = 0.5 | |||
| 200 | + | if dy > focusEpsilon { | |||
| 201 | + | t.Logf("✓ CORRECT: Navigation DOWN would SUCCEED") | |||
| 202 | + | } | |||
| 203 | + | ||||
| 204 | + | // Also verify UP navigation | |||
| 205 | + | dyUp := normalCY - stackWrapperCY | |||
| 206 | + | if dyUp < -focusEpsilon { | |||
| 207 | + | t.Logf("✓ CORRECT: Navigation UP would SUCCEED") | |||
| 208 | + | } | |||
| 209 | + | }) | |||
| 210 | + | } | |||
| 211 | + | ||||
| 212 | + | // TestGetNavigationAllocation tests the proposed fix helper function | |||
| 213 | + | // This will be uncommented and updated after implementing the helper | |||
| 214 | + | func TestGetNavigationAllocation(t *testing.T) { | |||
| 215 | + | t.Skip("Enable this test after implementing getNavigationAllocation() helper") | |||
| 216 | + | ||||
| 217 | + | // TODO: After implementing getNavigationAllocation(), test: | |||
| 218 | + | // 1. Regular leaf pane → returns pane.container allocation | |||
| 219 | + | // 2. Pane inside stack → returns stack.stackWrapper allocation | |||
| 220 | + | // 3. Stack container itself → returns stack.stackWrapper allocation | |||
| 221 | + | // 4. Nil safety: node=nil, container=nil, parent=nil, stackWrapper=nil | |||
| 222 | + | } | |||
M pkg/webkit/helpers.go +17 -0
| @@ -171,6 +171,23 @@ func WidgetGetBounds(w gtk.Widgetter) (x, y, width, height float64) { | |||
| 171 | 171 | return 0, 0, 0, 0 | |
| 172 | 172 | } | |
| 173 | 173 | ||
| 174 | + | // WidgetGetWindowBounds returns the widget's bounds relative to the toplevel window | |
| 175 | + | // This provides window-absolute coordinates needed for navigation geometry calculations | |
| 176 | + | func WidgetGetWindowBounds(w gtk.Widgetter) (x, y, width, height int) { | |
| 177 | + | if widget := getWidget(w); widget != nil { | |
| 178 | + | // Get the toplevel window (GtkWindow) | |
| 179 | + | root := widget.Root() | |
| 180 | + | if root != nil { | |
| 181 | + | rect, ok := widget.ComputeBounds(root) | |
| 182 | + | if ok && rect != nil { | |
| 183 | + | return int(rect.X()), int(rect.Y()), | |
| 184 | + | int(rect.Width()), int(rect.Height()) | |
| 185 | + | } | |
| 186 | + | } | |
| 187 | + | } | |
| 188 | + | return 0, 0, 0, 0 | |
| 189 | + | } | |
| 190 | + | ||
| 174 | 191 | // WidgetSetFocusChild sets the focus child of a widget | |
| 175 | 192 | func WidgetSetFocusChild(w gtk.Widgetter, child gtk.Widgetter) { | |
| 176 | 193 | if widget := getWidget(w); widget != nil { | |
| @@ -171,6 +171,23 @@ func WidgetGetBounds(w gtk.Widgetter) (x, y, width, height float64) { | |||||
| 171 | return 0, 0, 0, 0 | 171 | return 0, 0, 0, 0 | ||
| 172 | } | 172 | } | ||
| 173 | 173 | ||||
| 174 | + | // WidgetGetWindowBounds returns the widget's bounds relative to the toplevel window | |||
| 175 | + | // This provides window-absolute coordinates needed for navigation geometry calculations | |||
| 176 | + | func WidgetGetWindowBounds(w gtk.Widgetter) (x, y, width, height int) { | |||
| 177 | + | if widget := getWidget(w); widget != nil { | |||
| 178 | + | // Get the toplevel window (GtkWindow) | |||
| 179 | + | root := widget.Root() | |||
| 180 | + | if root != nil { | |||
| 181 | + | rect, ok := widget.ComputeBounds(root) | |||
| 182 | + | if ok && rect != nil { | |||
| 183 | + | return int(rect.X()), int(rect.Y()), | |||
| 184 | + | int(rect.Width()), int(rect.Height()) | |||
| 185 | + | } | |||
| 186 | + | } | |||
| 187 | + | } | |||
| 188 | + | return 0, 0, 0, 0 | |||
| 189 | + | } | |||
| 190 | + | ||||
| 174 | // WidgetSetFocusChild sets the focus child of a widget | 191 | // WidgetSetFocusChild sets the focus child of a widget | ||
| 175 | func WidgetSetFocusChild(w gtk.Widgetter, child gtk.Widgetter) { | 192 | func WidgetSetFocusChild(w gtk.Widgetter, child gtk.Widgetter) { | ||
| 176 | if widget := getWidget(w); widget != nil { | 193 | if widget := getWidget(w); widget != nil { | ||