Skip to content

Work around macOS issue in which elements lose their window property#75

Open
nriley wants to merge 4 commits intophillco:mainfrom
nriley:window-workaround
Open

Work around macOS issue in which elements lose their window property#75
nriley wants to merge 4 commits intophillco:mainfrom
nriley:window-workaround

Conversation

@nriley
Copy link
Collaborator

@nriley nriley commented Nov 17, 2024

Also use double quotes consistently.

@nriley nriley requested a review from phillco as a code owner November 17, 2024 21:38
Copy link
Owner

@phillco phillco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a Talon issue for this we can link?

Comment on lines +112 to +135
while True:
if hasattr(element, "AXWindow"):
try:
app = element.window.app
except ui.UIErr:
pass
else:
break
match element.AXRole:
case "AXWindow":
with suppress(AttributeError):
app = element.app
break
case "AXApplication":
apps = ui.apps(name=element.AXTitle)
if len(apps) == 1:
app = apps[0]
break
if hasattr(element, "AXParent"):
element = element.parent
continue
app = None
break

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think splitting it out into its own function can improve the readability a bit -- what do you think about this?

def extract_app(element) -> Optional[ui.App]:
    # Works around a macOS issue where elements lose their window property
    # https://github.com/talonvoice/talon/issues/XX
    while element:
        try:
            return element.window.app
        except (ui.UIErr, AttributeError):
            pass

        match element.AXRole:
            case "AXWindow":
                with suppress(AttributeError):
                    return element.app
            case "AXApplication":
                apps = ui.apps(name=element.AXTitle)
                if len(apps) == 1:
                    return apps[0]

        if not hasattr(element, "AXParent"):
            break

        element = element.parent
    return None

I normally would check for the property rather than catch the AttributeError, but since we're already doing this for ui.UIErr, I think it's cleaner and saves a level of indentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, hadn't filed an issue but have now (talonvoice/talon#689).

Your refactor looks good to me. Unfortunately sometimes checking for properties doesn't always work right with UI elements (see the ticket above), so I've defaulted to catching/suppressing exceptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants