mirrors/ghostty
1
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2026-04-19 14:00:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

macOS: fix window frame when (re)opening new window (#11380)

Browse Source 
Claude wrote the fail path in the UI tests, or you can easily reproduce
this manually. This is kinda a regression after #11322, since we are not
delaying the frame update anymore, which exposes some of the "flaws" of
the previous implementation.

The following three commits fix this step by step:
- We shouldn't save intermediate frames when the window is loading,
which is triggered by `windowDidResize` and `windowDidMove` during the
process.
- We should set the initial position (from the config) after the window
is loaded.
- A small refactor on `LastWindowPosition` to support restoring the
window frame under certain conditions.


https://github.com/user-attachments/assets/6f90f9a5-653d-4146-95c6-8e5c69bda656



### AI Disclosure

Claude helped me write the UI tests.
This commit is contained in:
Mitchell Hashimoto
2026-03-11 10:08:14 -07:00
committed by GitHub
parent 7aff470ceb 596d502a75
commit 2296a82c13
4 changed files with 210 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

158
macos/GhosttyUITests/GhosttyWindowPositionUITests.swift Normal file
View File

@@ -0,0 +1,158 @@
//
// GhosttyWindowPositionUITests.swift
// GhosttyUITests
//
// Created by Claude on 2026-03-11.
//
import XCTest
final class GhosttyWindowPositionUITests: GhosttyCustomConfigCase {
override static var runsForEachTargetApplicationUIConfiguration: Bool { false }
// MARK: - Restore round-trip per titlebar style
@MainActor func testRestoredNative() throws { try runRestoreTest(titlebarStyle: "native") }
@MainActor func testRestoredHidden() throws { try runRestoreTest(titlebarStyle: "hidden") }
@MainActor func testRestoredTransparent() throws { try runRestoreTest(titlebarStyle: "transparent") }
@MainActor func testRestoredTabs() throws { try runRestoreTest(titlebarStyle: "tabs") }
// MARK: - Config overrides cached position/size
@MainActor
func testConfigOverridesCachedPositionAndSize() async throws {
// Launch maximized so the cached frame is fullscreen-sized.
try updateConfig(
"""
maximize = true
title = "GhosttyWindowPositionUITests"
"""
)
let app = try ghosttyApplication()
app.launch()
let window = app.windows.firstMatch
XCTAssertTrue(window.waitForExistence(timeout: 5), "Window should appear")
let maximizedFrame = window.frame
// Now update the config with a small explicit size and position,
// reload, and open a new window. It should respect the config, not the cache.
try updateConfig(
"""
window-position-x = 50
window-position-y = 50
window-width = 30
window-height = 30
title = "GhosttyWindowPositionUITests"
"""
)
app.typeKey(",", modifierFlags: [.command, .shift])
try await Task.sleep(for: .seconds(0.5))
app.typeKey("n", modifierFlags: [.command])
XCTAssertEqual(app.windows.count, 2, "Should have 2 windows")
let newWindow = app.windows.element(boundBy: 0)
let newFrame = newWindow.frame
// The new window should be smaller than the maximized one.
XCTAssertLessThan(newFrame.size.width, maximizedFrame.size.width,
"30 columns should be narrower than maximized")
XCTAssertLessThan(newFrame.size.height, maximizedFrame.size.height,
"30 rows should be shorter than maximized")
app.terminate()
}
// MARK: - Size-only config change preserves position
@MainActor
func testSizeOnlyConfigPreservesPosition() async throws {
// Launch maximized so the window has a known position (top-left of visible frame).
try updateConfig(
"""
maximize = true
title = "GhosttyWindowPositionUITests"
"""
)
let app = try ghosttyApplication()
app.launch()
let window = app.windows.firstMatch
XCTAssertTrue(window.waitForExistence(timeout: 5), "Window should appear")
let initialFrame = window.frame
// Reload with only size changed, close current window, open new one.
// Position should be restored from cache.
try updateConfig(
"""
window-width = 30
window-height = 30
title = "GhosttyWindowPositionUITests"
"""
)
app.typeKey(",", modifierFlags: [.command, .shift])
try await Task.sleep(for: .seconds(0.5))
app.typeKey("w", modifierFlags: [.command])
app.typeKey("n", modifierFlags: [.command])
let newWindow = app.windows.firstMatch
XCTAssertTrue(newWindow.waitForExistence(timeout: 5), "New window should appear")
let newFrame = newWindow.frame
// Position should be preserved from the cached value.
// Compare x and maxY since the window is anchored at the top-left
// but AppKit uses bottom-up coordinates (origin.y changes with height).
XCTAssertEqual(newFrame.origin.x, initialFrame.origin.x, accuracy: 2,
"x position should not change with size-only config")
XCTAssertEqual(newFrame.maxY, initialFrame.maxY, accuracy: 2,
"top edge (maxY) should not change with size-only config")
app.terminate()
}
// MARK: - Shared round-trip helper
/// Opens a new window, records its frame, closes it, opens another,
/// and verifies the frame is restored consistently.
private func runRestoreTest(titlebarStyle: String) throws {
try updateConfig(
"""
macos-titlebar-style = \(titlebarStyle)
title = "GhosttyWindowPositionUITests"
"""
)
let app = try ghosttyApplication()
app.launch()
let window = app.windows.firstMatch
XCTAssertTrue(window.waitForExistence(timeout: 5), "Window should appear")
let firstFrame = window.frame
// Close the window and open a new one — it should restore the same frame.
app.typeKey("w", modifierFlags: [.command])
app.typeKey("n", modifierFlags: [.command])
let window2 = app.windows.firstMatch
XCTAssertTrue(window2.waitForExistence(timeout: 5), "New window should appear")
let restoredFrame = window2.frame
XCTAssertEqual(restoredFrame.origin.x, firstFrame.origin.x, accuracy: 2,
"[\(titlebarStyle)] x position should be restored")
XCTAssertEqual(restoredFrame.origin.y, firstFrame.origin.y, accuracy: 2,
"[\(titlebarStyle)] y position should be restored")
XCTAssertEqual(restoredFrame.size.width, firstFrame.size.width, accuracy: 2,
"[\(titlebarStyle)] width should be restored")
XCTAssertEqual(restoredFrame.size.height, firstFrame.size.height, accuracy: 2,
"[\(titlebarStyle)] height should be restored")
app.terminate()
}
}

29
macos/Sources/Features/Terminal/TerminalController.swift
View File

@@ -1061,6 +1061,23 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr
}
}
// Set the initial window position. This must happen after the window
// is fully set up (content view, toolbar, default size) so that
// decorations added by subclass awakeFromNib (e.g. toolbar for tabs
// style) don't change the frame after the position is restored.
if let terminalWindow = window as? TerminalWindow {
terminalWindow.setInitialWindowPosition(
x: derivedConfig.windowPositionX,
y: derivedConfig.windowPositionY,
)
}
LastWindowPosition.shared.restore(
window,
origin: derivedConfig.windowPositionX == nil && derivedConfig.windowPositionY == nil,
size: defaultSize == nil,
)
// Store our initial frame so we can know our default later. This MUST
// be after the defaultSize call above so that we don't re-apply our frame.
// Note: we probably want to set this on the first frame change or something
@@ -1171,27 +1188,21 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr
self.fixTabBar()
// Whenever we move save our last position for the next start.
if let window {
LastWindowPosition.shared.save(window)
}
LastWindowPosition.shared.save(window)
}
override func windowDidResize(_ notification: Notification) {
super.windowDidResize(notification)
// Whenever we resize save our last position and size for the next start.
if let window {
LastWindowPosition.shared.save(window)
}
LastWindowPosition.shared.save(window)
}
func windowDidBecomeMain(_ notification: Notification) {
// Whenever we get focused, use that as our last window position for
// restart. This differs from Terminal.app but matches iTerm2 behavior
// and I think its sensible.
if let window {
LastWindowPosition.shared.save(window)
}
LastWindowPosition.shared.save(window)
// Remember our last main
Self.lastMain = self

16
macos/Sources/Features/Terminal/Window Styles/TerminalWindow.swift
View File

@@ -120,11 +120,10 @@ class TerminalWindow: NSWindow {
// If window decorations are disabled, remove our title
if !config.windowDecorations { styleMask.remove(.titled) }
// Set our window positioning to coordinates if config value exists, otherwise
// fallback to original centering behavior
setInitialWindowPosition(
x: config.windowPositionX,
y: config.windowPositionY)
// NOTE: setInitialWindowPosition is NOT called here because subclass
// awakeFromNib may add decorations (e.g. toolbar for tabs style) that
// change the frame. It is called from TerminalController.windowDidLoad
// after the window is fully set up.
// If our traffic buttons should be hidden, then hide them
if config.macosWindowButtons == .hidden {
@@ -537,13 +536,10 @@ class TerminalWindow: NSWindow {
terminalController?.updateColorSchemeForSurfaceTree()
}
private func setInitialWindowPosition(x: Int16?, y: Int16?) {
func setInitialWindowPosition(x: Int16?, y: Int16?) {
// If we don't have an X/Y then we try to use the previously saved window pos.
guard let x = x, let y = y else {
if !LastWindowPosition.shared.restore(self) {
center()
}
center()
return
}

31
macos/Sources/Helpers/LastWindowPosition.swift
View File

@@ -6,13 +6,32 @@ class LastWindowPosition {
private let positionKey = "NSWindowLastPosition"
func save(_ window: NSWindow) {
@discardableResult
func save(_ window: NSWindow?) -> Bool {
// We should only save the frame if the window is visible.
// This avoids overriding the previously saved one
// with the wrong one when window decorations change while creating,
// e.g. adding a toolbar affects the window's frame.
guard let window, window.isVisible else { return false }
let frame = window.frame
let rect = [frame.origin.x, frame.origin.y, frame.size.width, frame.size.height]
UserDefaults.standard.set(rect, forKey: positionKey)
return true
}
func restore(_ window: NSWindow) -> Bool {
/// Restores a previously saved window frame (or parts of it) onto the given window.
///
/// - Parameters:
/// - window: The window whose frame should be updated.
/// - restoreOrigin: Whether to restore the saved position. Pass `false` when the
/// config specifies an explicit `window-position-x`/`window-position-y`.
/// - restoreSize: Whether to restore the saved size. Pass `false` when the config
/// specifies an explicit `window-width`/`window-height`.
/// - Returns: `true` if the frame was modified, `false` if there was nothing to restore.
@discardableResult
func restore(_ window: NSWindow, origin restoreOrigin: Bool = true, size restoreSize: Bool = true) -> Bool {
guard restoreOrigin || restoreSize else { return false }
guard let values = UserDefaults.standard.array(forKey: positionKey) as? [Double],
values.count >= 2 else { return false }
@@ -22,14 +41,16 @@ class LastWindowPosition {
let visibleFrame = screen.visibleFrame
var newFrame = window.frame
newFrame.origin = lastPosition
if restoreOrigin {
newFrame.origin = lastPosition
}
if values.count >= 4 {
if restoreSize, values.count >= 4 {
newFrame.size.width = min(values[2], visibleFrame.width)
newFrame.size.height = min(values[3], visibleFrame.height)
}
if !visibleFrame.contains(newFrame.origin) {
if restoreOrigin, !visibleFrame.contains(newFrame.origin) {
newFrame.origin.x = max(visibleFrame.minX, min(visibleFrame.maxX - newFrame.width, newFrame.origin.x))
newFrame.origin.y = max(visibleFrame.minY, min(visibleFrame.maxY - newFrame.height, newFrame.origin.y))
}