Fix focus jumping when same-app floating windows closes#1914
Fix focus jumping when same-app floating windows closes#1914jperras wants to merge 1 commit intonikitabobko:mainfrom
Conversation
Skip focus recalculation when a floating window (dialog) is destroyed if it belongs to the same app as the currently focused window. This fixes Emacs child frames (posframes, completion popups) causing focus to jump to another application when they close. Fixes nikitabobko#776 Fixes nikitabobko#1220
|
This is effecting me to. I would love to see this PR merged! |
|
I just tested this PR on my machine, and it fixes the issue for me with Emacs. As others have said, it would be great to see this merged! :) |
|
@nikitabobko Can this PR get some attention please 🥺 |
|
I don't like the proposed logic as it sounds wrong to me. I don't like the existing logic in general, but I don't know how to do it better. This PR just increases the technical debt and the complexity of the code that I didn't like already. I'd like to notice that this "preserve the focused workspace" logic doesn't activate if the window is detected as a popup. So the alternative fix could be to improve AeroSpace popup heuristics to detect these Emacs windows as popups (according to your descriptions, they do sound like popups, not real windows) Pointers:
Closing the PR for now |
| if focus.windowOrNil?.app.pid == app.pid && parent is Workspace { | ||
| // The destroyed window was a floating window (dialog) from the same app. | ||
| // Don't recalculate focus - let the app handle it internally. | ||
| break |
There was a problem hiding this comment.
Was the guard logic was added to avoid setFocus(to: deadWindowFocus) or deadWindowFocus.windowOrNil?.nativeFocus() or both?
There was a problem hiding this comment.
Emacs (at least the flavour I have) doesn't end up calling nativeFocus() in the branching logic after this because, well, all the windows/frames that emacs creates belong to emacs.
The setFocus(to: deadWindowFocus), from what I remember, is what messes things up, though it might be a red herring. When I was bisecting, it was a bit tough to pinpoint things exactly because of gc changes in 405dec8, where you moved to a polling-based implementation.
|
@nikitabobko yeah, that's fair - this isn't the cleanest fix. Emacs child frames should probably just be treated as popups, even though they're a bit weird (because emacs). FWIW I've been running this fix for a while now without any complications, but I'm not the most adventurous when it comes to macOS app usage, so it's definitely possible this diff would cause additional problems for other apps that might do some more esoteric/weird stuff with child windows. I'll see if I can improve the popup detection heuristics instead. |
|
If I'm not mistaken, I think some applications create new windows for popups like completion similar to what we see with Emacs. I remember someone having an issue with MATLAB (which I think uses Swing but not 100% certain of this) on a tiling manager because it would try to tile the completion popups... I don't know if that error is the same with Aerospace (I try to avoid MATLAB like the plague :P ), but it could be. |
|
For anyone following along on this PR that uses emacs, try out #2036 - I believe it's a more sane fix. |
Summary
Skip focus recalculation when a floating window (dialog) is destroyed if it belongs to the same app as the currently focused window.
This fixes Emacs child frames (posframes, completion popups, etc.) causing focus to jump to another application when they close.
References
Fixes #776
Fixes #1220
PR checklist
./run-tests.shexits with non-zero exit code.Failure to follow the checklist with no apparent reasons will result in silent PR rejection.