From 89576e090d87f195f0b7da6c11955a8134f184f9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 20 Jul 2026 22:30:22 -0700 Subject: [PATCH] refactor(dialog): consolidate accessibility traversal --- .../UI/DialogService+ButtonActions.swift | 7 +- .../UI/DialogService+Classification.swift | 11 --- .../Services/UI/DialogService+Elements.swift | 52 ++-------- .../DialogService+FileDialogNavigation.swift | 4 +- .../DialogService+FileDialogResolution.swift | 4 +- .../UI/DialogService+Resolution.swift | 7 +- .../Services/UI/DialogService+Traversal.swift | 94 +++++++++++++++++++ .../DialogButtonResolutionTests.swift | 53 +++++++++++ .../DialogElementTraversalTests.swift | 35 ++++++- 9 files changed, 194 insertions(+), 73 deletions(-) create mode 100644 Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Traversal.swift create mode 100644 Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogButtonResolutionTests.swift diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+ButtonActions.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+ButtonActions.swift index 52f34703f..e17d57513 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+ButtonActions.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+ButtonActions.swift @@ -27,7 +27,7 @@ extension DialogService { self.logger.debug("Found \(buttons.count) buttons in dialog") guard let targetButton = self.resolveButton( - in: dialog, + in: buttons, requestedTitle: buttonText, allowFallbackToDefaultAction: allowFallbackToDefaultAction) else { @@ -71,12 +71,11 @@ extension DialogService { return result } - private func resolveButton( - in dialog: Element, + func resolveButton( + in buttons: [Element], requestedTitle: String, allowFallbackToDefaultAction: Bool) -> Element? { - let buttons = self.collectButtons(from: dialog) let identifierAttribute = Attribute("AXIdentifier") let normalizedRequested = self.normalizedDialogButtonTitle(requestedTitle) diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Classification.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Classification.swift index 7df01afbd..f14fd8fd3 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Classification.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Classification.swift @@ -3,17 +3,6 @@ import Foundation @MainActor extension DialogService { - func sheetElements(for element: Element) -> [Element] { - var sheets: [Element] = [] - if let children = element.children() { - sheets.append(contentsOf: children.filter { $0.role() == "AXSheet" }) - } - if let attachedSheets = element.sheets() { - sheets.append(contentsOf: attachedSheets) - } - return sheets - } - func isDialogElement(_ element: Element, matching title: String?) -> Bool { let role = element.role() ?? "" let subrole = element.subrole() ?? "" diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Elements.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Elements.swift index f1cc7d3da..a42ddc226 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Elements.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Elements.swift @@ -3,55 +3,15 @@ import AXorcist import Foundation import PeekabooFoundation -func collectUniqueDepthFirst( - from root: Node, - matching predicate: (Node) -> Bool, - children: (Node) -> [Node]) -> [Node] -{ - var matches: [Node] = [] - var visited: Set = [] - var stack = [root] - - while let node = stack.popLast() { - guard visited.insert(node).inserted else { continue } - - if predicate(node) { - matches.append(node) - } - - stack.append(contentsOf: children(node).reversed()) - } - - return matches -} - -func firstUniqueDepthFirst( - from root: Node, - matching predicate: (Node) -> Bool, - children: (Node) -> [Node]) -> Node? -{ - var visited: Set = [] - var stack = [root] - - while let node = stack.popLast() { - guard visited.insert(node).inserted else { continue } - - if predicate(node) { - return node - } - - stack.append(contentsOf: children(node).reversed()) - } - - return nil -} - @MainActor extension DialogService { func collectTextFields(from element: Element) -> [Element] { - collectUniqueDepthFirst( + DialogTraversal.collectUniqueDepthFirst( from: element, - matching: { $0.role() == "AXTextField" || $0.role() == "AXTextArea" }, + matching: { + let role = $0.role() + return role == "AXTextField" || role == "AXTextArea" + }, children: { $0.children() ?? [] }) } @@ -143,7 +103,7 @@ extension DialogService { } func collectButtons(from element: Element) -> [Element] { - collectUniqueDepthFirst( + DialogTraversal.collectUniqueDepthFirst( from: element, matching: { $0.role() == "AXButton" }, children: { $0.children() ?? [] }) diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogNavigation.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogNavigation.swift index c3287d4b6..10986318f 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogNavigation.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogNavigation.swift @@ -49,7 +49,7 @@ extension DialogService { let identifierAttribute = Attribute("AXIdentifier") func findDisclosureCandidate(in element: Element) -> Element? { - firstUniqueDepthFirst( + DialogTraversal.firstUniqueDepthFirst( from: element, matching: { current in if current.role() == "AXDisclosureTriangle" { @@ -73,7 +73,7 @@ extension DialogService { let description = (current.attribute(Attribute("AXDescription")) ?? "").lowercased() return description.contains("show details") || description.contains("hide details") }, - children: { self.sheetElements(for: $0) + ($0.children() ?? []) }) + children: { self.sheetFirstTraversalChildren(for: $0) }) } guard let disclosure = findDisclosureCandidate(in: dialog) else { return } diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogResolution.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogResolution.swift index 539810c7d..87a349ace 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogResolution.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+FileDialogResolution.swift @@ -17,9 +17,9 @@ extension DialogService { } private func findActiveFileDialogCandidate(in element: Element) -> Element? { - firstUniqueDepthFirst( + DialogTraversal.firstUniqueDepthFirst( from: element, matching: self.isFileDialogElement, - children: { self.sheetElements(for: $0) + ($0.children() ?? []) }) + children: { self.sheetFirstTraversalChildren(for: $0) }) } } diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Resolution.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Resolution.swift index 59e2ba966..78dd1e1ac 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Resolution.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Resolution.swift @@ -190,13 +190,12 @@ extension DialogService { } func resolveDialogCandidate(in element: Element, matching title: String?) -> Element? { - firstUniqueDepthFirst( + DialogTraversal.firstUniqueDepthFirst( from: element, matching: { self.isDialogElement($0, matching: title) }, children: { current in - let sheets = title == nil ? (current.sheets() ?? []) : self.sheetElements(for: current) - guard title != nil else { return sheets } - return sheets + (current.children() ?? []) + guard title != nil else { return current.sheets() ?? [] } + return self.sheetFirstTraversalChildren(for: current) }) } } diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Traversal.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Traversal.swift new file mode 100644 index 000000000..3ae91aa9a --- /dev/null +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/DialogService+Traversal.swift @@ -0,0 +1,94 @@ +import AXorcist + +enum DialogTraversal { + static func collectUniqueDepthFirst( + from root: Node, + matching predicate: (Node) -> Bool, + children: (Node) -> [Node]) -> [Node] + { + var matches: [Node] = [] + _ = self.visitUniqueDepthFirst( + from: root, + children: children, + stopWhen: { node in + if predicate(node) { + matches.append(node) + } + return false + }) + return matches + } + + static func firstUniqueDepthFirst( + from root: Node, + matching predicate: (Node) -> Bool, + children: (Node) -> [Node]) -> Node? + { + self.visitUniqueDepthFirst( + from: root, + children: children, + stopWhen: predicate) + } + + private static func visitUniqueDepthFirst( + from root: Node, + children: (Node) -> [Node], + stopWhen predicate: (Node) -> Bool) -> Node? + { + var visited: Set = [] + var stack = [root] + + while let node = stack.popLast() { + guard visited.insert(node).inserted else { continue } + + if predicate(node) { + return node + } + + stack.append(contentsOf: children(node).reversed()) + } + + return nil + } + + static func sheetFirstChildren( + children: [Node], + attachedSheets: [Node], + isSheet: (Node) -> Bool) -> [Node] + { + var childSheets: [Node] = [] + var ordinaryChildren: [Node] = [] + + for child in children { + if isSheet(child) { + childSheets.append(child) + } else { + ordinaryChildren.append(child) + } + } + + var ordered: [Node] = [] + ordered.reserveCapacity(children.count + attachedSheets.count) + var visited: Set = [] + + for group in [childSheets, attachedSheets, ordinaryChildren] { + for child in group where visited.insert(child).inserted { + ordered.append(child) + } + } + + return ordered + } +} + +@MainActor +extension DialogService { + func sheetFirstTraversalChildren(for element: Element) -> [Element] { + let children = element.children() ?? [] + let attachedSheets = element.sheets() ?? [] + return DialogTraversal.sheetFirstChildren( + children: children, + attachedSheets: attachedSheets, + isSheet: { $0.role() == "AXSheet" }) + } +} diff --git a/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogButtonResolutionTests.swift b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogButtonResolutionTests.swift new file mode 100644 index 000000000..ceb06db70 --- /dev/null +++ b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogButtonResolutionTests.swift @@ -0,0 +1,53 @@ +import ApplicationServices +@preconcurrency import AXorcist +import Testing +@testable import PeekabooAutomationKit + +@MainActor +struct DialogButtonResolutionTests { + @Test + func `button resolution uses the supplied snapshot`() { + let service = DialogService() + let cancelButton = makeButton(offset: 1, title: "Cancel", identifier: "CancelButton") + let saveButton = makeButton(offset: 2, title: "Save…", identifier: "SaveButton") + + let resolved = service.resolveButton( + in: [cancelButton, saveButton], + requestedTitle: "Save", + allowFallbackToDefaultAction: false) + + #expect(resolved == saveButton) + } + + @Test + func `button resolution preserves identifier fallbacks`() { + let service = DialogService() + let cancelButton = makeButton(offset: 3, title: "Abort", identifier: "CancelButton") + let okButton = makeButton(offset: 4, title: "Continue", identifier: "OKButton") + let buttons = [cancelButton, okButton] + + let defaultResolved = service.resolveButton( + in: buttons, + requestedTitle: "default", + allowFallbackToDefaultAction: false) + let cancelResolved = service.resolveButton( + in: buttons, + requestedTitle: "Dismiss", + allowFallbackToDefaultAction: false) + + #expect(defaultResolved == okButton) + #expect(cancelResolved == cancelButton) + } +} + +@MainActor +private func makeButton(offset: pid_t, title: String, identifier: String) -> Element { + Element( + AXUIElementCreateApplication(getpid() + offset), + attributes: [ + "AXTitle": .string(title), + "AXIdentifier": .string(identifier), + ], + children: nil, + actions: nil) +} diff --git a/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogElementTraversalTests.swift b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogElementTraversalTests.swift index 482d874e8..094655ed5 100644 --- a/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogElementTraversalTests.swift +++ b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/DialogElementTraversalTests.swift @@ -16,7 +16,7 @@ struct DialogElementTraversalTests { primaryButton.children = [dialog] secondGroup.children = [sharedButton, secondaryButton] - let matches = collectUniqueDepthFirst( + let matches = DialogTraversal.collectUniqueDepthFirst( from: dialog, matching: { $0.isMatch }, children: { $0.children }) @@ -37,7 +37,7 @@ struct DialogElementTraversalTests { firstGroup.children = [dialog, primaryButton] secondGroup.children = [secondaryButton] - let match = firstUniqueDepthFirst( + let match = DialogTraversal.firstUniqueDepthFirst( from: dialog, matching: { inspected.append($0.name) @@ -49,6 +49,31 @@ struct DialogElementTraversalTests { #expect(inspected == ["dialog", "first group", "primary button"]) } + @Test + func `sheet first children preserve order and deduplicate identities`() { + let firstOrdinary = TraversalNode(name: "first ordinary") + let firstChildSheet = TraversalNode(name: "first child sheet", isSheet: true) + let secondOrdinary = TraversalNode(name: "second ordinary") + let secondChildSheet = TraversalNode(name: "second child sheet", isSheet: true) + let attachedOnlySheet = TraversalNode(name: "attached only sheet", isSheet: true) + + let children = [firstOrdinary, firstChildSheet, secondOrdinary, secondChildSheet] + let attachedSheets = [secondChildSheet, attachedOnlySheet, firstChildSheet] + + let ordered = DialogTraversal.sheetFirstChildren( + children: children, + attachedSheets: attachedSheets, + isSheet: { $0.isSheet }) + + #expect(ordered.map(\.name) == [ + "first child sheet", + "second child sheet", + "attached only sheet", + "first ordinary", + "second ordinary", + ]) + } + @Test func `deep hierarchies do not consume the call stack`() { let root = TraversalNode(name: "0") @@ -62,7 +87,7 @@ struct DialogElementTraversalTests { current = child } - let matches = collectUniqueDepthFirst( + let matches = DialogTraversal.collectUniqueDepthFirst( from: root, matching: { $0.isMatch }, children: { $0.children }) @@ -79,11 +104,13 @@ struct DialogElementTraversalTests { private final class TraversalNode: Hashable { let name: String let isMatch: Bool + let isSheet: Bool var children: [TraversalNode] = [] - init(name: String, isMatch: Bool = false) { + init(name: String, isMatch: Bool = false, isSheet: Bool = false) { self.name = name self.isMatch = isMatch + self.isSheet = isSheet } static func == (lhs: TraversalNode, rhs: TraversalNode) -> Bool {