Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node is not deleted from the tree when window is closed #653

Closed
mkutsevol opened this issue Oct 31, 2024 · 11 comments
Closed

Node is not deleted from the tree when window is closed #653

mkutsevol opened this issue Oct 31, 2024 · 11 comments
Labels
bug Something isn't working as expected duplicate The issue is a duplicate

Comments

@mkutsevol
Copy link

mkutsevol commented Oct 31, 2024

Describe the bug
When I close any window, its space is not reclaimed by siblings.
https://youtu.be/5JPp_Tts4tM how it happens for me.

$aerospace list-windows --workspace 5 --format "%{window-id} | %{window-title} | %{app-bundle-id} | %{app-name}"
gives me

7247 |   | com.googlecode.iterm2 | iTerm2
7250 |  mkutsevol@mkutsevol-mac:~ | com.googlecode.iterm2 | iTerm2
7152 |  mkutsevol@mkutsevol-mac:~ | com.googlecode.iterm2 | iTerm2

This happens for any app, not just the terminal.

But if I have 2 different apps on a workspace, and I close one, it behaves correctly.

I'd expect the ||| split to turn into || split after the middle one is closed.
It doesn't matter which one I close, if it's on the edge or inside. Type of layout doesn't matter also.
Configuration of aerospace is the default one from the docs.

Versions:
aerospace CLI client version: 0.15.2-Beta b6cf827
AeroSpace.app server version: 0.15.2-Beta b6cf827

Displays have separate spaces is disabled.

macOS is 15.0.1 (24A348) running on mac studio 2023 with 3 monitors.
Output of aerospace debug-windows I clicked that empty space in the middle. It was a different run of the same reporducer, not the one on the recording.

com.googlecode.iterm2.window.46 windowId: 46
com.googlecode.iterm2.window.46 workspace: 3
com.googlecode.iterm2.window.46 treeNodeParent: AppBundle.TilingContainer
com.googlecode.iterm2.window.46 recognizedAsDialog: false
com.googlecode.iterm2.window.46 AXTitle: Optional(aerospace debug-windows)
com.googlecode.iterm2.window.46 AXRole: Optional(AXWindow)
com.googlecode.iterm2.window.46 AXSubrole: Optional(AXStandardWindow)
com.googlecode.iterm2.window.46 AXFocused: Optional(0)
com.googlecode.iterm2.window.46 AXFullScreen: Optional(0)
com.googlecode.iterm2.window.46 AXFrame: Optional(<AXValue 0x600003480640> {value = x:-2560.000000 y:-1760.000000 w:2560.000000 h:2839.000000 type = kAXValueCGRectType})
com.googlecode.iterm2.window.46 AXPosition: Optional(<AXValue 0x600002fb1860> {value = x:-2560.000000 y:-1760.000000 type = kAXValueCGPointType})
com.googlecode.iterm2.window.46 AXGrowArea: nil
com.googlecode.iterm2.window.46 AXMinimizeButton: AXUIElement {
com.googlecode.iterm2.window.46      AXRole: Optional(AXButton)
com.googlecode.iterm2.window.46      AXTitle: nil
com.googlecode.iterm2.window.46      AXSubrole: Optional(AXMinimizeButton)
com.googlecode.iterm2.window.46      AXEnabled: Optional(1)
com.googlecode.iterm2.window.46      AXParent: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXTopLevelUIElement: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXWindow: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      Ignored: AXFrame, AXSize, AXFocused, AXHelp, AXPosition, AXRoleDescription
com.googlecode.iterm2.window.46 }
com.googlecode.iterm2.window.46 AXDocument: nil
com.googlecode.iterm2.window.46 AXSections: [
com.googlecode.iterm2.window.46         Optional({
com.googlecode.iterm2.window.46             SectionDescription = Content;
com.googlecode.iterm2.window.46             SectionObject = "<AXUIElement 0x600002ffa7f0> {pid=984}";
com.googlecode.iterm2.window.46             SectionUniqueID = AXContent;
com.googlecode.iterm2.window.46         }),
com.googlecode.iterm2.window.46         Optional({
com.googlecode.iterm2.window.46             SectionDescription = "Top Level Navigator";
com.googlecode.iterm2.window.46             SectionObject = "<AXUIElement 0x600002ffa280> {pid=984}";
com.googlecode.iterm2.window.46             SectionUniqueID = AXTopLevelNavigator;
com.googlecode.iterm2.window.46         })
com.googlecode.iterm2.window.46 ]
com.googlecode.iterm2.window.46 AXCloseButton: AXUIElement {
com.googlecode.iterm2.window.46      AXRole: Optional(AXButton)
com.googlecode.iterm2.window.46      AXTitle: nil
com.googlecode.iterm2.window.46      AXSubrole: Optional(AXCloseButton)
com.googlecode.iterm2.window.46      AXEnabled: Optional(1)
com.googlecode.iterm2.window.46      AXParent: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXTopLevelUIElement: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXWindow: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      Ignored: AXFrame, AXSize, AXFocused, AXHelp, AXPosition, AXRoleDescription, AXEdited
com.googlecode.iterm2.window.46 }
com.googlecode.iterm2.window.46 AXMain: Optional(1)
com.googlecode.iterm2.window.46 AXActivationPoint: Optional(<AXValue 0x600002ff1830> {value = x:-2490.000000 y:-1746.000000 type = kAXValueCGPointType})
com.googlecode.iterm2.window.46 AXFullScreenButton: AXUIElement {
com.googlecode.iterm2.window.46      AXRole: Optional(AXButton)
com.googlecode.iterm2.window.46      AXTitle: nil
com.googlecode.iterm2.window.46      AXSubrole: Optional(AXFullScreenButton)
com.googlecode.iterm2.window.46      AXEnabled: Optional(1)
com.googlecode.iterm2.window.46      AXParent: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXTopLevelUIElement: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXWindow: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      Ignored: AXFrame, AXSize, AXFocused, AXChildren, AXHelp, AXPosition, AXRoleDescription
com.googlecode.iterm2.window.46 }
com.googlecode.iterm2.window.46 AXProxy: nil
com.googlecode.iterm2.window.46 AXDefaultButton: nil
com.googlecode.iterm2.window.46 AXMinimized: Optional(0)
com.googlecode.iterm2.window.46 AXParent: Optional(<AXUIElement Application 0x600002f99b90> {pid=984})
com.googlecode.iterm2.window.46 AXTitleUIElement: AXUIElement(windowId=46, title=nil, role="AXStaticText", subrole=nil)
com.googlecode.iterm2.window.46 AXCancelButton: nil
com.googlecode.iterm2.window.46 AXModal: Optional(0)
com.googlecode.iterm2.window.46 AXZoomButton: AXUIElement {
com.googlecode.iterm2.window.46      AXRole: Optional(AXButton)
com.googlecode.iterm2.window.46      AXTitle: nil
com.googlecode.iterm2.window.46      AXSubrole: Optional(AXFullScreenButton)
com.googlecode.iterm2.window.46      AXEnabled: Optional(1)
com.googlecode.iterm2.window.46      AXParent: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXTopLevelUIElement: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      AXWindow: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2.window.46      Ignored: AXFrame, AXSize, AXFocused, AXChildren, AXHelp, AXPosition, AXRoleDescription
com.googlecode.iterm2.window.46 }
com.googlecode.iterm2.window.46 AXSize: Optional(<AXValue 0x600002f99b90> {value = w:2560.000000 h:2839.000000 type = kAXValueCGSizeType})
com.googlecode.iterm2.window.46 AXToolbarButton: nil
com.googlecode.iterm2.window.46 Ignored: AXChildrenInNavigationOrder, AXChildren, AXRoleDescription
com.googlecode.iterm2           AXRole: Optional(AXApplication)
com.googlecode.iterm2           AXTitle: Optional(iTerm2)
com.googlecode.iterm2           AXFunctionRowTopLevelElements: [
com.googlecode.iterm2           ]
com.googlecode.iterm2           AXFrame: nil
com.googlecode.iterm2           AXFocusedUIElement: AXUIElement(windowId=46, title=nil, role="AXTextArea", subrole=nil)
com.googlecode.iterm2           AXFrontmost: Optional(1)
com.googlecode.iterm2           AXExtrasMenuBar: nil
com.googlecode.iterm2           AXMainWindow: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2           AXFocusedWindow: AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2           AXMenuBar: Optional(<AXUIElement 0x600002f99b90> {pid=984})
com.googlecode.iterm2           AXWindows: [
com.googlecode.iterm2               AXUIElement(windowId=46, title="aerospace debug-windows", role="AXWindow", subrole="AXStandardWindow"),
com.googlecode.iterm2               AXUIElement(windowId=910, title="mkutsevol@mkutsevol-mac:~", role="AXWindow", subrole="AXStandardWindow"),
com.googlecode.iterm2               AXUIElement(windowId=904, title="mkutsevol@mkutsevol-mac:~", role="AXWindow", subrole="AXStandardWindow")
com.googlecode.iterm2           ]
com.googlecode.iterm2           AXSize: nil
com.googlecode.iterm2           AXPosition: nil
com.googlecode.iterm2           Ignored: AXChildren, AXChildrenInNavigationOrder, AXEnhancedUserInterface, AXPreferredLanguage, AXRoleDescription, AXHidden

Please help.

Thank you!

Related issues you could find
I failed to find anything. It makes me think that I'm missing something obvious. Please help :)

@mkutsevol mkutsevol added the bug Something isn't working as expected label Oct 31, 2024
@juandazapata
Copy link

juandazapata commented Oct 31, 2024

I'm experiencing the same issue on macOS Sequoia 15.0.1, however it works fine on Sonoma 14.7.1. I suspect this has something to do with the new Window Tiling feature in Sequoia, but I might be wrong (the tiling toggles are disabled in my preferences pane)

@mkutsevol
Copy link
Author

FWIW I have those tiling options in Desktop&Dock disabled.

@mkutsevol
Copy link
Author

I suspect that
if window.observe(destroyedObs, kAXUIElementDestroyedNotification) && in MacWindow.swift never gets notification of windows closing.
But I can't properly confirm, as I don't have access to a mac machine on which it works.
And this is the first time I look at apple apis. I either don't know where docs are, or they really don't have anything beyond "An accessibility object was disposed of." as description of this notification.

Another piece of observation: if I completely close iTerm, so that the app really quits, and then open it again, it will position itself correctly on all available space.

@mkutsevol
Copy link
Author

I got some progress, but I don't know what I'm doing :D
It's not a solution, it's narrowing down what is happening, but I'll run it locally, as I killed yabai and I don't want back. AeroSpace just works better :)
So, if I add

    private func garbageCollectWindows() {
        // This is hilariously bad compared to notification driven way.
        // I have no idea if the check for absence of attributes means that the window is actually closed,
        // that's what I gathered from running it in debugger.
        MacWindow.allWindows.lazy.filter { $0.app == self }.forEach {
            if $0.axWindow.attrs.isEmpty{
                $0.garbageCollect()
            }
        }
    }
    
    static func garbageCollectClosedWindows() {
        for app in Array(allAppsMap.values) {
            app.garbageCollectWindows()
        }
    }

to MacApp.swift, and then call it from gc() on every refresh after MacApp.garbageCollectTerminatedApps() then I get the behavior I expect. It reclaims space. I didn't notice any strange sideeffects, but a) test was really limited in time b) I don't have all the possible strange apps installed.

@nikitabobko
Copy link
Owner

Duplicate of #445

@nikitabobko nikitabobko marked this as a duplicate of #445 Nov 1, 2024
@nikitabobko nikitabobko added the duplicate The issue is a duplicate label Nov 1, 2024
@nikitabobko
Copy link
Owner

I didn't notice any strange sideeffects, but a) test was really limited in time

If you lock the screen, all windows attrs becomes empty. You unlock the screen and windows don't preserve their workspace assignment

@mkutsevol
Copy link
Author

I didn't notice any strange sideeffects, but a) test was really limited in time

If you lock the screen, all windows attrs becomes empty. You unlock the screen and windows don't preserve their workspace assignment

Thanks for looking into it @nikitabobko! And for writing this savior of macos that makes it useable! :D

So I went to sleep with a release build that had those changes running. It locked the screen because I was away for a long time. There are no logs in that piece of code so I can't tell if it executed it for every window or not, but I can't see any strange behavior from the machine. All windows were on correct monitors. I can manage windows with hotkeys. I tried moving focus on one workspace, jumping to different workspace on the same monitor and to different monitor, resizing windows, moving windows. And then I tried closing a window of iTerm. And it behaved as it should, I didn't notice anything at all. Then I tried locking the screen myself, and I also didn't experience any effects at all. It works.

Duplicate of #445

It says

(e.g. if user quickly closes several windows in a row)

For me it's every time, very consistently. I don't have to do it quickly. And I don't have to do multiple in a row. It just never fires the kAXUIElementDestroyedNotification. And I also had a breakpoint in the callback there, it was never hit.

@mkutsevol
Copy link
Author

release.tgz
that's a release build of the tip of master branch with my changes, if anyone wants to check if that works. It's an archive with the .release folder. https://github.com/nikitabobko/AeroSpace/blob/main/install-from-sources.sh#L17-L23 is the part that needs to be executed to install, boiling down to:

  • unpack, it will create .release folder
  • uninstall existing version, and -dev if you have it
  • brew install --cask ./.release/aerospace-dev.rb

I wonder if that will work for your case also, @juandazapata.

@nikitabobko
Copy link
Owner

@mkutsevol if you want to share code, please share links to commits in your fork instead of sharing compiled binaries.

@mkutsevol
Copy link
Author

git diff https://gist.github.com/mkutsevol/bf4567a7e5b7818245847e644965fc55

The meaningful part of the code is above in this thread.

I found it to be tedious to get it to compile, setup env. So a binary is useful if folks want to spend 10 minutes testing, not an hour setting up the build env for it.

@mkutsevol
Copy link
Author

After several days with my changes I definitely see that sleeping (logoff) does something to it, as @nikitabobko said earlier. But with my config, where workspaces are pinned to monitors and windows are pinned to workspaces in config, it doesn't matter much. I lose ordering of terminal windows after a break, but that I can live with, compared to the app being unusable for me. So the changes I had are just trading one problem for another, it's not a fix of any kind. I'll sit & wait for a proper solution :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected duplicate The issue is a duplicate
Projects
None yet
Development

No branches or pull requests

3 participants