macos: in new_window action, activate App (#8064)
> [!NOTE]
> The change might have been intentional, and so I lack context. I
mention two ways to fix below, this PR implements the first possible
fix.
This change makes sure that the new window is focused and visible.
When commit 33d128bcff removed the
TerminalManager class and moved its functionality into
TerminalController, it accidentally removed app activation for windows
triggered by global keybinds.
How the bug works:
1. Menu actions (like File → New Window) call AppDelegate.newWindow()
which: 2. Calls TerminalController.newWindow() 3. AND explicitly calls
NSApp.activate(ignoringOtherApps: true) in the AppDelegate
4. Global keybind actions trigger ghosttyNewWindow() notification
handler which:
5. Only calls TerminalController.newWindow()
6. Does NOT call NSApp.activate(ignoringOtherApps: true)
7. While TerminalController.newWindow() does call
NSApp.activate(ignoringOtherApps: true) internally, this call happens
before the async dispatch that shows the window, so the activation
occurs but the window isn't focused when it's actually shown.
8. In the old TerminalManager.newWindow(), the activation happened
immediately before the async dispatch, ensuring proper timing for window
focus.
To see the bug in action:
- run recent Ghostty `main`
- set up a global keybind for `new_window`
- focus some other window
- trigger keybind
- notice that Ghostty doesn't come to the foreground, but when manually
switching to Ghostty you will see that the new window _was_ created
The fix would be to either move the NSApp.activate() call back into
TerminalController.newWindow(), as it was for TerminalManager, or add
the activation call to the notification handlers in AppDelegate.
pull/8338/head
commit
3ef6de4ffa
|
|
@ -214,6 +214,10 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr
|
|||
}
|
||||
}
|
||||
|
||||
// All new_window actions force our app to be active, so that the new
|
||||
// window is focused and visible.
|
||||
NSApp.activate(ignoringOtherApps: true)
|
||||
|
||||
// We're dispatching this async because otherwise the lastCascadePoint doesn't
|
||||
// take effect. Our best theory is there is some next-event-loop-tick logic
|
||||
// that Cocoa is doing that we need to be after.
|
||||
|
|
|
|||
Loading…
Reference in New Issue