fix(windows): Move to New Window never opens a window exactly on one already open

Merged
LA
6d ago
+71-6

Two Move to New Window actions from one window opened both new windows at the same frame, so the second one hid the first completely. The app then paused the hidden window's terminal, and after a restart the session showed one window where the person had two (v2.2.6-opt gate journey mw-restart-three-windows: both windows at [438, 242, 1100, 720]).

MovedWindowPlacement placed the new window from the source frame alone (beside it, else cascaded 28 pt), so every move from the same window got the same answer. frame(beside:on:avoiding:) now takes the frames of the windows already open. From a spot a window already takes (same top-left), it steps on by the cascade step, down and right, as AppKit's cascadeTopLeft(from:) does. A step that would leave the screen goes back to the screen's top-left only when that spot is free; otherwise it steps on, partly off screen. A zoomed or near-full source has no free spot on its screen, and a window partly off screen is better than one hidden under another (review finding on dc8712d2558). WindowManager.openMovedWindow passes every controller's window frame, including a window a move just opened that is not ordered in yet. Chrome offsets each new window from the last one in the same way (WindowSizer, the GetSavedWindowBounds cascade).

Ported from cp/fix-moved-window-cascade (perf train 321: c49bca966be, 27a3b5daf1b, d899ecc52c3), plus the zoomed-source fix found in review. On feat the bug is present: openMovedWindow called frame(beside:on:) with no other windows. The App change is net zero lines.

Testing

  • MoveToNewWindowTests.twoMovesFromOneWindowOpenTwoWindowsThatDoNotCoverEachOther runs the real action (moveWorkspaceToNewWindow) twice from one window on a same-tree daemon and checks that no two windows share a frame origin.
  • MoveToNewWindowTests.threeMovesFromOneWindowEachGetASpotOfTheirOwn calls the placement three times from a zoomed source, a near-full source and the gate's 1100x720 source, and checks that no two windows share a top-left.

Fleet jobs (package-test-lane.sh suite Packages/macOS/CmuxNext MoveToNewWindowTests):

  • Compile (Linux, CmuxNext + app + cmux-tui, recipe v2) at the head 107992259937: 22fd224f6c49ba07065294e7, ok. Its log shows the changed files compiled.
  • Red, test commit 191c26862e0 alone on feat: e5203dcf8f07b53eb1f87a3f on aws-m4pro-3. FAILED as expected: twoMovesFromOneWindowOpenTwoWindowsThatDoNotCoverEachOther at MoveToNewWindowTests.swift:162, (origins.count → 2) == (frames.count → 3); the other 7 tests passed.
  • Red, zoomed-source test at 7b9cf7e1f94: 0095c9ba236049b6ffc95432 on aws-m4pro-1. FAILED as expected at MoveToNewWindowTests.swift:190: zoomed source (spots.count → 2) == (open.count → 4), near-full source (3) == (4); the 1100x720 case and the other 8 tests passed.
  • Green, head 107992259937: ec225222ef8848ba784d7bac on aws-m4pro-6. PASSED: "Test run with 9 tests in 1 suite passed", both new tests included (the zoomed test with its 3 cases).

Observed / Not run

  • Observed: the compile, both reds and the green above. A Python model of the placement (review) puts every window at its own top-left for zoomed, near-full and 1100x720 sources, and in 20,000 random cases.
  • Not run: the live mw-restart-three-windows journey on a feat build.

Changelog

Fixed: Moving two workspaces to new windows from the same window no longer opens the second window exactly on top of the first

Proof

No capture: window placement only; the regression tests check the frames.