diff --git a/CHANGELOG.md b/CHANGELOG.md index 6701699763..641b5d02a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -512,6 +512,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A leading `GO` line sent to SQL Server by MCP and AI assistant tools, and a `GO n` count ignored. - App hanging for a minute when an import stopped on a failing statement several megabytes long. - SQL Server Windows Authentication to another realm failing when the service principal name is over 128 bytes. +- Data grid's inline cell editor and cell viewer unreachable by VoiceOver. ### Security diff --git a/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift b/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift index c5f957a8e0..39d0c2f5f7 100644 --- a/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift +++ b/Packages/TableProCore/Tests/TableProDatabaseTests/ConnectionManagerTests.swift @@ -399,17 +399,18 @@ struct ConnectionManagerTests { _ = try await manager.connect(connection) + let rescuedByTimer = Flag() let rescue = Task { - try? await Task.sleep(for: .seconds(2)) + try? await Task.sleep(for: .seconds(30)) + guard !Task.isCancelled else { return } + await rescuedByTimer.raise() await unblocked.open() } defer { rescue.cancel() } - let started = ContinuousClock.now await manager.disconnect(connection.id) - let elapsed = ContinuousClock.now - started - #expect(elapsed < .seconds(1)) + #expect(!(await rescuedByTimer.isRaised)) #expect(driver.disconnectCount == 1) } @@ -511,9 +512,7 @@ struct ConnectionManagerTests { try await Task.sleep(for: .milliseconds(100)) attempt.cancel() - let started = ContinuousClock.now - let settled = await outcome.settled(within: .seconds(2)) - let elapsed = ContinuousClock.now - started + let settled = await outcome.settled(within: .seconds(10)) #expect(!second.isConnected) await stuck.open() @@ -521,7 +520,6 @@ struct ConnectionManagerTests { let result = try #require(settled, "the cancelled connect never gave up") #expect(throws: CancellationError.self) { try result.get() } - #expect(elapsed < .seconds(1)) } @Test("A tunnel is opened for the connection being dialed, not for whoever asked last") diff --git a/Packages/TableProCore/Tests/TableProMSSQLCoreTests/MSSQLFreeTDSConfigFileTests.swift b/Packages/TableProCore/Tests/TableProMSSQLCoreTests/MSSQLFreeTDSConfigFileTests.swift index 66fe89a8d5..884219d7cc 100644 --- a/Packages/TableProCore/Tests/TableProMSSQLCoreTests/MSSQLFreeTDSConfigFileTests.swift +++ b/Packages/TableProCore/Tests/TableProMSSQLCoreTests/MSSQLFreeTDSConfigFileTests.swift @@ -76,120 +76,114 @@ struct MSSQLFreeTDSConfigFileTests { } @Test("Host names that differ only in case are one entry, as libtds reads them") - func caseInsensitiveNames() throws { + func caseInsensitiveNames() async throws { let lower = try entry("db.example.com", encryption: .request) let upper = try entry("DB.example.com", encryption: .require) + let file = file let order = OrderLog() + let holding = Latch() let released = DispatchSemaphore(value: 0) - let holding = DispatchSemaphore(value: 0) - DispatchQueue.global().async { + let holder = runOnOwnThread { try? file.withEntry(lower, waitingAtMost: 10) { order.append("lower in") - holding.signal() + holding.open() released.wait() order.append("lower out") } } - holding.wait() - let done = DispatchSemaphore(value: 0) - DispatchQueue.global().async { + await holding.wait() + let waiter = runOnOwnThread { try? file.withEntry(upper, waitingAtMost: 10) { order.append("upper in") } - done.signal() } - Thread.sleep(forTimeInterval: 0.2) + try await Task.sleep(for: .milliseconds(200)) released.signal() - done.wait() + await holder.wait() + await waiter.wait() #expect(order.entries == ["lower in", "lower out", "upper in"]) } @Test("A different entry for a host waits for the first dbopen, then finds its own settings") - func conflictingEntryWaits() throws { + func conflictingEntryWaits() async throws { let plain = try entry("db.example.com", encryption: .request) let encrypted = try entry("db.example.com", encryption: .require) + let file = file let order = OrderLog() - let holding = DispatchSemaphore(value: 0) + let holding = Latch() let released = DispatchSemaphore(value: 0) - let done = DispatchSemaphore(value: 0) let seenByEncrypted = Box(nil) - DispatchQueue.global().async { + let holder = runOnOwnThread { try? file.withEntry(plain, waitingAtMost: 10) { order.append("plain in") - holding.signal() + holding.open() released.wait() order.append("plain out") } } - holding.wait() - DispatchQueue.global().async { + await holding.wait() + let waiter = runOnOwnThread { try? file.withEntry(encrypted, waitingAtMost: 10) { order.append("encrypted in") seenByEncrypted.value = contents() } - done.signal() } - Thread.sleep(forTimeInterval: 0.2) + try await Task.sleep(for: .milliseconds(200)) #expect(order.entries == ["plain in"]) released.signal() - done.wait() + await holder.wait() + await waiter.wait() #expect(order.entries == ["plain in", "plain out", "encrypted in"]) #expect(seenByEncrypted.value == encrypted.text) } @Test("A connect to another host does not wait on one that has not returned") - func otherHostsDoNotWait() throws { + func otherHostsDoNotWait() async throws { let slow = try entry("slow.example.com") let fast = try entry("fast.example.com") - let holding = DispatchSemaphore(value: 0) + let file = file + let holding = Latch() let released = DispatchSemaphore(value: 0) - let finished = DispatchSemaphore(value: 0) - DispatchQueue.global().async { + let holder = runOnOwnThread { try? file.withEntry(slow, waitingAtMost: 10) { - holding.signal() + holding.open() released.wait() } - finished.signal() } - holding.wait() + await holding.wait() let ranWhileSlowHeld = try file.withEntry(fast, waitingAtMost: 10) { contents()?.contains(fast.text) == true } released.signal() - finished.wait() + await holder.wait() #expect(ranWhileSlowHeld) } @Test("Connects to two ports on one address, as every SSH tunnel is, do not wait on each other") - func portsOnOneAddressDoNotWait() throws { + func portsOnOneAddressDoNotWait() async throws { let silent = try entry("127.0.0.1", port: 50_001) let live = try entry("127.0.0.1", port: 50_002, encryption: .request) - let holding = DispatchSemaphore(value: 0) + let file = file + let holding = Latch() let released = DispatchSemaphore(value: 0) - let silentFinished = DispatchSemaphore(value: 0) - let liveFinished = DispatchSemaphore(value: 0) let seen = Box(nil) - DispatchQueue.global().async { + let silentConnect = runOnOwnThread { try? file.withEntry(silent, waitingAtMost: 10) { - holding.signal() + holding.open() released.wait() } - silentFinished.signal() } - holding.wait() - DispatchQueue.global().async { + await holding.wait() + let liveConnect = runOnOwnThread { try? file.withEntry(live, waitingAtMost: 10) { seen.value = contents() } - liveFinished.signal() } - let liveRanWhileSilentHeld = liveFinished.wait(timeout: .now() + 2) == .success + let liveRanWhileSilentHeld = await liveConnect.opens(within: .seconds(2)) released.signal() - silentFinished.wait() - if !liveRanWhileSilentHeld { - liveFinished.wait() - } + await silentConnect.wait() + await liveConnect.wait() #expect(liveRanWhileSilentHeld) #expect(seen.value?.contains(silent.text) == true) @@ -197,106 +191,99 @@ struct MSSQLFreeTDSConfigFileTests { } @Test("A connect waiting for a name goes before a later one that matches the entry holding it") - func waitingEntryIsNotOvertaken() throws { + func waitingEntryIsNotOvertaken() async throws { let plain = try entry("db.example.com", encryption: .request) let encrypted = try entry("db.example.com", encryption: .require) + let file = file let order = OrderLog() - let holding = DispatchSemaphore(value: 0) + let holding = Latch() let released = DispatchSemaphore(value: 0) - let done = DispatchSemaphore(value: 0) - DispatchQueue.global().async { + let holder = runOnOwnThread { try? file.withEntry(plain, waitingAtMost: 10) { order.append("first plain in") - holding.signal() + holding.open() released.wait() order.append("first plain out") } } - holding.wait() - DispatchQueue.global().async { + await holding.wait() + let encryptedConnect = runOnOwnThread { try? file.withEntry(encrypted, waitingAtMost: 10) { order.append("encrypted in") } - done.signal() } - #expect(waitUntil { file.waitingConnections(named: "db.example.com") == 1 }) - DispatchQueue.global().async { + #expect(await eventually { file.waitingConnections(named: "db.example.com") == 1 }) + let secondPlainConnect = runOnOwnThread { try? file.withEntry(plain, waitingAtMost: 10) { order.append("second plain in") } - done.signal() } - #expect(waitUntil { file.waitingConnections(named: "db.example.com") == 2 }) + #expect(await eventually { file.waitingConnections(named: "db.example.com") == 2 }) #expect(order.entries == ["first plain in"]) released.signal() - done.wait() - done.wait() + await holder.wait() + await encryptedConnect.wait() + await secondPlainConnect.wait() #expect(order.entries == ["first plain in", "first plain out", "encrypted in", "second plain in"]) } @Test("A connect that cannot have the name in time gives up with the reason and leaves the line") - func boundedWaitGivesUp() throws { + func boundedWaitGivesUp() async throws { let plain = try entry("db.example.com", encryption: .request) let encrypted = try entry("db.example.com", encryption: .require) - let holding = DispatchSemaphore(value: 0) + let file = file + let holding = Latch() let released = DispatchSemaphore(value: 0) - let finished = DispatchSemaphore(value: 0) - DispatchQueue.global().async { + let holder = runOnOwnThread { try? file.withEntry(plain, waitingAtMost: 10) { - holding.signal() + holding.open() released.wait() } - finished.signal() } - holding.wait() + await holding.wait() #expect(throws: MSSQLFreeTDSConfigError.nameInUse("db.example.com")) { try file.withEntry(encrypted, waitingAtMost: 0.2) {} } let waitingAfterGivingUp = file.waitingConnections(named: "db.example.com") let joinedTheHolder = try file.withEntry(plain, waitingAtMost: 0.2) { true } released.signal() - finished.wait() + await holder.wait() #expect(waitingAfterGivingUp == 0) #expect(joinedTheHolder) } @Test("A connect whose caller gave up leaves the line as soon as the waits are interrupted") - func abandonedWaitLeavesTheLine() throws { + func abandonedWaitLeavesTheLine() async throws { let plain = try entry("db.example.com", encryption: .request) let encrypted = try entry("db.example.com", encryption: .require) - let holding = DispatchSemaphore(value: 0) + let file = file + let holding = Latch() let released = DispatchSemaphore(value: 0) - let holderFinished = DispatchSemaphore(value: 0) - let waiterFinished = DispatchSemaphore(value: 0) let abandoned = Box(false) let waiterError = Box(nil) let ran = Box(false) - DispatchQueue.global().async { + let holder = runOnOwnThread { try? file.withEntry(plain, waitingAtMost: 10) { - holding.signal() + holding.open() released.wait() } - holderFinished.signal() } - holding.wait() - DispatchQueue.global().async { + await holding.wait() + let waiter = runOnOwnThread { do { try file.withEntry(encrypted, waitingAtMost: 30, givingUpWhen: { abandoned.value }) { ran.value = true } } catch { waiterError.value = error } - waiterFinished.signal() } - #expect(waitUntil { file.waitingConnections(named: "db.example.com") == 1 }) + #expect(await eventually { file.waitingConnections(named: "db.example.com") == 1 }) abandoned.value = true file.interruptWaits() - let leftInTime = waiterFinished.wait(timeout: .now() + 2) == .success + let leftInTime = await waiter.opens(within: .seconds(2)) released.signal() - holderFinished.wait() - if !leftInTime { - waiterFinished.wait() - } + await holder.wait() + await waiter.wait() #expect(leftInTime) #expect(waiterError.value is CancellationError) @@ -316,15 +303,65 @@ struct MSSQLFreeTDSConfigFileTests { } } -private func waitUntil(_ condition: () -> Bool) -> Bool { - let deadline = Date(timeIntervalSinceNow: 5) +private func eventually(within limit: Duration = .seconds(5), _ condition: () -> Bool) async -> Bool { + let deadline = ContinuousClock.now + limit while !condition() { - guard Date() < deadline else { return false } - Thread.sleep(forTimeInterval: 0.01) + guard ContinuousClock.now < deadline else { return false } + try? await Task.sleep(for: .milliseconds(10)) } return true } +private func runOnOwnThread(_ work: @escaping @Sendable () -> Void) -> Latch { + let finished = Latch() + let thread = Thread { + work() + finished.open() + } + thread.start() + return finished +} + +private final class Latch: @unchecked Sendable { + private let lock = NSLock() + private var isOpen = false + private var waiters: [CheckedContinuation] = [] + + var opened: Bool { + lock.lock() + defer { lock.unlock() } + return isOpen + } + + func open() { + lock.lock() + isOpen = true + let released = waiters + waiters = [] + lock.unlock() + for waiter in released { + waiter.resume() + } + } + + func wait() async { + await withCheckedContinuation { continuation in + lock.lock() + guard !isOpen else { + lock.unlock() + continuation.resume() + return + } + waiters.append(continuation) + lock.unlock() + } + } + + func opens(within limit: Duration) async -> Bool { + await eventually(within: limit) { opened } + } +} + private final class OrderLog: @unchecked Sendable { private let lock = NSLock() private var recorded: [String] = [] diff --git a/Packages/TableProCore/Tests/TableProSSHTransportTests/SSHChannelRelayTests.swift b/Packages/TableProCore/Tests/TableProSSHTransportTests/SSHChannelRelayTests.swift index 8adede6966..ae0db540f8 100644 --- a/Packages/TableProCore/Tests/TableProSSHTransportTests/SSHChannelRelayTests.swift +++ b/Packages/TableProCore/Tests/TableProSSHTransportTests/SSHChannelRelayTests.swift @@ -293,7 +293,6 @@ struct SSHChannelRelayBacklogTests { let run = try runBacklogRelay() #expect(run.reads == Self.queuedBuffers + 1) - #expect(run.elapsed < 1.0) } @Test("A backlog past the per-round cap still lands inside one poll interval") diff --git a/Packages/TableProEditor/Sources/TableProEditorKit/SourceEditor/TextBindingSync.swift b/Packages/TableProEditor/Sources/TableProEditorKit/SourceEditor/TextBindingSync.swift index eaae2dbd52..741c441061 100644 --- a/Packages/TableProEditor/Sources/TableProEditorKit/SourceEditor/TextBindingSync.swift +++ b/Packages/TableProEditor/Sources/TableProEditorKit/SourceEditor/TextBindingSync.swift @@ -22,7 +22,7 @@ final class TextBindingSync { private(set) var lastSyncedText: String? private let phase: RepresentableSyncPhase - private var writebackTask: Task? + private(set) var writebackTask: Task? init(text: SourceEditor.TextAPI, phase: RepresentableSyncPhase) { self.text = text diff --git a/Packages/TableProEditor/Tests/TableProEditorKitTests/SourceEditorBindingSyncTests.swift b/Packages/TableProEditor/Tests/TableProEditorKitTests/SourceEditorBindingSyncTests.swift index e1021ba2f0..2d435ba636 100644 --- a/Packages/TableProEditor/Tests/TableProEditorKitTests/SourceEditorBindingSyncTests.swift +++ b/Packages/TableProEditor/Tests/TableProEditorKitTests/SourceEditorBindingSyncTests.swift @@ -103,7 +103,7 @@ final class SourceEditorBindingSyncTests: XCTestCase { coordinator.textSync.applyRepresentableText(bound, controller: controller) XCTAssertEqual(controller.textView.string, largeText) - try await Task.sleep(for: .milliseconds(300)) + await coordinator.textSync.writebackTask?.value XCTAssertEqual(bound, largeText) XCTAssertEqual(coordinator.textSync.lastSyncedText, largeText) } diff --git a/TablePro/Core/VersionHistory/LinkedFileVersionHistoryProvider.swift b/TablePro/Core/VersionHistory/LinkedFileVersionHistoryProvider.swift index bd1d98084c..d9bf9fffa7 100644 --- a/TablePro/Core/VersionHistory/LinkedFileVersionHistoryProvider.swift +++ b/TablePro/Core/VersionHistory/LinkedFileVersionHistoryProvider.swift @@ -94,10 +94,10 @@ internal struct LinkedFileVersionHistoryProvider: VersionHistoryProvider { func prepareDiscard() async throws -> VersionRestorePlan { let client = try makeClient() let current = try currentBytes() - let staged = try await gitCall { try await client.blob(revision: "", path: indexPath, in: directory) } - try Self.rejectLargeFileStoragePointer(staged) let directory = directory let indexPath = indexPath + let staged = try await gitCall { try await client.blob(revision: "", path: indexPath, in: directory) } + try Self.rejectLargeFileStoragePointer(staged) return writePlan( replacing: current, with: staged, diff --git a/TablePro/Views/DataFiles/DataFileFindBar.swift b/TablePro/Views/DataFiles/DataFileFindBar.swift index 57d3480a00..cd105ef574 100644 --- a/TablePro/Views/DataFiles/DataFileFindBar.swift +++ b/TablePro/Views/DataFiles/DataFileFindBar.swift @@ -118,12 +118,12 @@ struct DataFileFindBar: View { Toggle(String(localized: "Whole Words"), isOn: option(\.matchesWholeWords)) Toggle(String(localized: "Regular Expression"), isOn: option(\.isRegularExpression)) } label: { - Image(systemName: "slider.horizontal.3") + Label { Text("Find Options") } icon: { Image(systemName: "slider.horizontal.3") } } + .labelStyle(.iconOnly) .menuStyle(.borderlessButton) .fixedSize() .help(String(localized: "Find Options")) - .accessibilityLabel(String(localized: "Find Options")) } private func option(_ keyPath: WritableKeyPath) -> Binding { diff --git a/TablePro/Views/Results/CellOverlayBase.swift b/TablePro/Views/Results/CellOverlayBase.swift index ff4bd8c92a..68d2ae9b43 100644 --- a/TablePro/Views/Results/CellOverlayBase.swift +++ b/TablePro/Views/Results/CellOverlayBase.swift @@ -33,12 +33,6 @@ class CellOverlayBase: NSObject { var containerView: NSView? { container } var tableView: NSTableView? { hostTableView } - func raiseToFront() { - guard let container, let hostTableView, container.superview === hostTableView else { return } - guard hostTableView.subviews.last !== container else { return } - hostTableView.addSubview(container) - } - func install( in tableView: NSTableView, row: Int, @@ -46,17 +40,27 @@ class CellOverlayBase: NSObject { columnIndex: Int, container: CellOverlayContainerView ) { + guard let scrollView = tableView.enclosingScrollView else { return } self.hostTableView = tableView self.row = row self.column = column self.columnIndex = columnIndex - tableView.addSubview(container) + Self.mount(container, over: tableView, in: scrollView) self.container = container setOverlayCell(CellPosition(row: row, column: columnIndex), in: tableView) selectionOverlay(in: tableView)?.needsDisplay = true installDismissObservers() } + private static func mount( + _ container: CellOverlayContainerView, + over tableView: NSTableView, + in scrollView: NSScrollView + ) { + container.frame = scrollView.convert(container.frame, from: tableView) + scrollView.addSubview(container, positioned: .above, relativeTo: scrollView.contentView) + } + /// The cell under the overlay draws no text of its own behind it. A drawn cell has no view to /// carry that, so the coordinator holds it and repaints the cell either side of the change. private func setOverlayCell(_ position: CellPosition?, in tableView: NSTableView) { diff --git a/TablePro/Views/Results/DataGridRowGutterView.swift b/TablePro/Views/Results/DataGridRowGutterView.swift index 6d972b54be..cdf70bb58d 100644 --- a/TablePro/Views/Results/DataGridRowGutterView.swift +++ b/TablePro/Views/Results/DataGridRowGutterView.swift @@ -126,8 +126,8 @@ final class DataGridRowGutterView: NSView { guard rowNumberColumn >= 0 else { return } /// The same rule the header uses. An identity check on the first responder is not it: while - /// a cell is being edited the responder is a descendant field editor, and the row and the - /// header both stay emphasized, so the strip would turn grey on its own. + /// a cell is being edited or viewed the responder is the overlay's text view beside the + /// table, and the header stays emphasized, so the strip would turn grey on its own. let emphasized = SortableHeaderEmphasis.isEmphasized( tableViewHoldsFocus: SortableHeaderEmphasis.holdsFocus(tableView: tableView, in: tableView.window), isKeyWindow: tableView.window?.isKeyWindow ?? false diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index b8a5fc9f58..5e53344377 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -55,8 +55,6 @@ final class KeyHandlingTableView: NSTableView { isRaisingOverlay = true defer { isRaisingOverlay = false } raiseSelectionOverlayIfNeeded(subview: subview) - raiseOverlayIfNeeded(coordinator?.overlayEditor, subview: subview) - raiseOverlayIfNeeded(coordinator?.overlayViewer, subview: subview) } private func raiseSelectionOverlayIfNeeded(subview: NSView) { @@ -67,16 +65,6 @@ final class KeyHandlingTableView: NSTableView { addSubview(selectionOverlay) } - private func raiseOverlayIfNeeded(_ overlay: CellOverlayBase?, subview: NSView) { - guard let overlay, - overlay.isActive, - let container = overlay.containerView, - container !== subview, - container.superview === self, - subviews.last !== container else { return } - overlay.raiseToFront() - } - var selection = TableSelection() { didSet { guard let (rows, columns) = selection.reloadIndexes(from: oldValue) else { return } diff --git a/TablePro/Views/Results/SortableHeaderEmphasis.swift b/TablePro/Views/Results/SortableHeaderEmphasis.swift index 8b761fcfda..232a55188d 100644 --- a/TablePro/Views/Results/SortableHeaderEmphasis.swift +++ b/TablePro/Views/Results/SortableHeaderEmphasis.swift @@ -12,10 +12,11 @@ internal enum SortableHeaderEmphasis { tableViewHoldsFocus && isKeyWindow } - /// A cell being edited puts the field editor in the responder chain below the table, so focus - /// is resolved by ancestry rather than by identity. + /// A cell being edited or viewed takes focus in a text view the grid mounts in the table's + /// scroll view, beside the table rather than inside it, so focus is resolved by ancestry from + /// the scroll view rather than by identity with the table. internal static func holdsFocus(tableView: NSTableView?, in window: NSWindow?) -> Bool { guard let tableView, let responder = window?.firstResponder as? NSView else { return false } - return responder === tableView || responder.isDescendant(of: tableView) + return responder.isDescendant(of: tableView.enclosingScrollView ?? tableView) } } diff --git a/TableProTests/Accessibility/AccessibleControlNameTests.swift b/TableProTests/Accessibility/AccessibleControlNameTests.swift index 0268b928f7..58c56f752e 100644 --- a/TableProTests/Accessibility/AccessibleControlNameTests.swift +++ b/TableProTests/Accessibility/AccessibleControlNameTests.swift @@ -49,12 +49,20 @@ struct AccessibleControlNameTests { for control in Self.labelledControls { let source = try String(contentsOf: root.appendingPathComponent(control.path), encoding: .utf8) #expect( - source.contains(".accessibilityLabel") && source.contains(control.label), + Self.names(control.label, in: source), "\(control.path) lost the accessibility label for \(control.label)" ) } } + /// A `Menu` is named by its `Label`: `.accessibilityLabel` on a menu leaves it nameless, which + /// `MenuDisclosureIndicatorTests` guards, so the label's own `Text` counts as the name. + private static func names(_ label: String, in source: String) -> Bool { + let namedByModifier = source.contains(".accessibilityLabel") && source.contains(label) + let namedByLabel = source.contains("Label { Text(\"\(label)\") }") + return namedByModifier || namedByLabel + } + /// The find bar's clear button is the one that had neither a label nor a tooltip, so nothing /// named it at all. @Test("The panel text field's clear button is named") diff --git a/TableProTests/Core/Storage/PreferenceKeysGuardTests.swift b/TableProTests/Core/Storage/PreferenceKeysGuardTests.swift index a4a0232bf2..523690e26a 100644 --- a/TableProTests/Core/Storage/PreferenceKeysGuardTests.swift +++ b/TableProTests/Core/Storage/PreferenceKeysGuardTests.swift @@ -39,7 +39,7 @@ struct PreferenceKeysGuardTests { /// main for a value no preference has ever read. Naming the calls that are not preferences keeps /// the baseline for the keys that genuinely are. private static let nonPreferenceCalls: Set = [ - "removeValue", "updateValue", "add", "animation", "removeAnimation", + "removeValue", "updateValue", "add", "animation", "removeAnimation", "values", ] private static let grandfatheredForKey: [String: String] = [ diff --git a/TableProTests/Views/Results/CellOverlayAccessibilityTests.swift b/TableProTests/Views/Results/CellOverlayAccessibilityTests.swift new file mode 100644 index 0000000000..591817894f --- /dev/null +++ b/TableProTests/Views/Results/CellOverlayAccessibilityTests.swift @@ -0,0 +1,264 @@ +import AppKit +import SwiftUI +import TableProPluginKit +import Testing + +@testable import TablePro + +@MainActor +private final class OverlayAccessibilityLayoutPersister: ColumnLayoutPersisting { + func load(for key: ColumnLayoutTableKey) -> ColumnLayoutState? { nil } + func save(_ layout: ColumnLayoutState, for key: ColumnLayoutTableKey) {} + func clear(for key: ColumnLayoutTableKey) {} +} + +@Suite("Cell overlay accessibility", .serialized) +@MainActor +struct CellOverlayAccessibilityTests { + private struct Grid { + let window: NSWindow + let coordinator: TableViewCoordinator + let tableView: KeyHandlingTableView + let scrollView: NSScrollView + let dataColumn: Int + } + + private func makeGrid(isEditable: Bool) throws -> Grid { + let columns = ["third_col"] + let columnTypes = [ColumnType.text(rawType: "TEXT")] + let coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(DataChangeManager()), + isEditable: isEditable, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: OverlayAccessibilityLayoutPersister() + ) + let tableRows = TableRows.from( + queryRows: [[PluginCellValue.text("3")]], + columns: columns, + columnTypes: columnTypes + ) + coordinator.tableRowsProvider = { tableRows } + coordinator.rebuildColumnMetadataCache(from: tableRows) + coordinator.updateCache() + + let tableView = KeyHandlingTableView(frame: NSRect(x: 0, y: 0, width: 600, height: 200)) + tableView.columnAutoresizingStyle = .noColumnAutoresizing + tableView.rowHeight = 21 + tableView.coordinator = coordinator + tableView.dataSource = coordinator + tableView.delegate = coordinator + tableView.addTableColumn(DataGridView.makeRowNumberColumn()) + coordinator.tableView = tableView + coordinator.columnPool.reconcile( + tableView: tableView, + schema: coordinator.identitySchema, + columnTypes: columnTypes, + savedLayout: nil, + isEditable: isEditable, + hiddenColumnNames: [], + firstClickSortDirection: .ascending, + widthCalculator: { _, _ in 120 } + ) + let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: 600, height: 200)) + scrollView.hasVerticalScroller = true + scrollView.hasHorizontalScroller = true + scrollView.documentView = tableView + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 600, height: 200), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.contentView = scrollView + tableView.reloadData() + tableView.layoutSubtreeIfNeeded() + window.layoutIfNeeded() + let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) + return Grid( + window: window, + coordinator: coordinator, + tableView: tableView, + scrollView: scrollView, + dataColumn: dataColumn + ) + } + + private func openViewer(in grid: Grid) throws -> CellOverlayViewer { + grid.coordinator.showOverlayViewer( + tableView: grid.tableView, + row: 0, + column: grid.dataColumn, + columnIndex: 0, + value: "3" + ) + return try #require(grid.coordinator.overlayViewer) + } + + private func textView(of overlay: CellOverlayBase) throws -> NSTextView { + let container = try #require(overlay.containerView) + let scrollView = try #require(container.subviews.first as? NSScrollView) + return try #require(scrollView.documentView as? NSTextView) + } + + private func publishedDescendants(of element: Any, depth: Int = 0) -> [Any] { + guard depth < 12, + let children = (element as? NSAccessibilityProtocol)?.accessibilityChildren() else { return [] } + let published = NSAccessibility.unignoredChildren(from: children) + return published + published.flatMap { publishedDescendants(of: $0, depth: depth + 1) } + } + + private func isPublished(_ textView: NSTextView, from window: NSWindow) -> Bool { + publishedDescendants(of: window).contains { ($0 as AnyObject) === textView } + } + + private func isSameElement(_ element: Any?, as expected: AnyObject) -> Bool { + guard let element else { return false } + return (element as AnyObject) === expected + } + + private func withActiveAccessibility(_ body: () throws -> Void) rethrows { + let wasActive = DataGridAccessibility.isActive + DataGridAccessibility.isActive = true + defer { DataGridAccessibility.isActive = wasActive } + try body() + } + + @Test("An open cell viewer is reachable by walking the window's accessibility tree") + func anOpenViewerIsInTheTree() throws { + try withActiveAccessibility { + let grid = try makeGrid(isEditable: false) + let viewer = try openViewer(in: grid) + defer { viewer.dismiss() } + let text = try textView(of: viewer) + + #expect(isPublished(text, from: grid.window)) + #expect(text.accessibilityRole() == .textArea) + #expect(text.accessibilityValue() as? String == "3") + } + } + + @Test("An open cell editor is reachable by walking the window's accessibility tree") + func anOpenEditorIsInTheTree() throws { + try withActiveAccessibility { + let grid = try makeGrid(isEditable: true) + grid.coordinator.beginCellEdit(row: 0, tableColumnIndex: grid.dataColumn) + let editor = try #require(grid.coordinator.overlayEditor) + defer { editor.dismiss(commit: false) } + let text = try textView(of: editor) + + #expect(isPublished(text, from: grid.window)) + #expect(text.accessibilityRole() == .textArea) + #expect(text.accessibilityValue() as? String == "3") + } + } + + @Test("The overlay is mounted in the grid's scroll view, outside the table that cannot publish it") + func theOverlayIsMountedOutsideTheTable() throws { + let grid = try makeGrid(isEditable: false) + let viewer = try openViewer(in: grid) + defer { viewer.dismiss() } + let container = try #require(viewer.containerView) + let overlayScrollView = try #require(container.subviews.first as? NSScrollView) + + #expect(container.superview === grid.scrollView) + #expect(!container.isDescendant(of: grid.tableView)) + let parent = overlayScrollView.accessibilityParent() + #expect(isSameElement(parent.flatMap { NSAccessibility.unignoredAncestor(of: $0) }, as: grid.scrollView)) + } + + @Test("The overlay sits over the rows and under the header, the scrollers and the row gutter") + func theOverlayStacksDirectlyAboveTheRows() throws { + let grid = try makeGrid(isEditable: false) + DataGridView.installRowGutter( + scrollView: grid.scrollView, + tableView: grid.tableView, + coordinator: grid.coordinator + ) + let gutter = try #require(grid.coordinator.rowGutter) + defer { gutter.detachTableGeometryObserver() } + let viewer = try openViewer(in: grid) + defer { viewer.dismiss() } + let container = try #require(viewer.containerView) + let subviews = grid.scrollView.subviews + let clipIndex = try #require(subviews.firstIndex { $0 === grid.scrollView.contentView }) + let overlayIndex = try #require(subviews.firstIndex { $0 === container }) + let headerClip = try #require(grid.tableView.headerView?.superview) + let headerIndex = try #require(subviews.firstIndex { $0 === headerClip }) + let scroller = try #require(grid.scrollView.verticalScroller) + let scrollerIndex = try #require(subviews.firstIndex { $0 === scroller }) + let gutterIndex = try #require(subviews.firstIndex { gutter.isDescendant(of: $0) }) + + #expect(overlayIndex == clipIndex + 1) + #expect(overlayIndex < headerIndex) + #expect(overlayIndex < scrollerIndex) + #expect(overlayIndex < gutterIndex) + } + + @Test("The grid's scroll view clips an overlay taller than the rows left below its cell") + func theGridClipsTheOverlay() throws { + let grid = try makeGrid(isEditable: false) + DataGridView.installRowGutter( + scrollView: grid.scrollView, + tableView: grid.tableView, + coordinator: grid.coordinator + ) + let gutter = try #require(grid.coordinator.rowGutter) + defer { gutter.detachTableGeometryObserver() } + let viewer = try openViewer(in: grid) + defer { viewer.dismiss() } + let container = try #require(viewer.containerView) + + #expect(container.superview === grid.scrollView) + #expect(grid.scrollView.clipsToBounds) + } + + @Test("The overlay lies over the cell it opened on") + func theOverlayCoversItsCell() throws { + let grid = try makeGrid(isEditable: false) + let viewer = try openViewer(in: grid) + defer { viewer.dismiss() } + let container = try #require(viewer.containerView) + let cell = grid.tableView.frameOfCell(atColumn: grid.dataColumn, row: 0) + + let overlayInTable = grid.tableView.convert(container.frame, from: grid.scrollView) + + #expect(overlayInTable.origin == cell.origin) + #expect(overlayInTable.width == cell.width) + #expect(overlayInTable.height >= cell.height) + } + + @Test("A dismissed overlay leaves nothing mounted or published") + func aDismissedOverlayLeavesNothingBehind() throws { + try withActiveAccessibility { + let grid = try makeGrid(isEditable: false) + let viewer = try openViewer(in: grid) + let container = try #require(viewer.containerView) + let text = try textView(of: viewer) + #expect(isPublished(text, from: grid.window)) + + viewer.dismiss() + + #expect(container.superview == nil) + #expect(!isPublished(text, from: grid.window)) + } + } + + @Test("The element a hit test over an open overlay returns is one the tree publishes") + func theHitTestResultOverTheOverlayIsInTheTree() throws { + try withActiveAccessibility { + let grid = try makeGrid(isEditable: false) + let viewer = try openViewer(in: grid) + defer { viewer.dismiss() } + let container = try #require(viewer.containerView) + let text = try textView(of: viewer) + let centre = NSPoint(x: container.bounds.midX, y: container.bounds.midY) + let onScreen = grid.window.convertPoint(toScreen: container.convert(centre, to: nil)) + + let hit = grid.window.accessibilityHitTest(onScreen) + + #expect(isSameElement(hit, as: text)) + #expect(isPublished(text, from: grid.window)) + } + } +} diff --git a/TableProTests/Views/Results/CellOverlayEditorMovementTests.swift b/TableProTests/Views/Results/CellOverlayEditorMovementTests.swift index ca9ae76ece..f6807086fb 100644 --- a/TableProTests/Views/Results/CellOverlayEditorMovementTests.swift +++ b/TableProTests/Views/Results/CellOverlayEditorMovementTests.swift @@ -18,10 +18,13 @@ struct CellOverlayEditorMovementTests { let editor: CellOverlayEditor let textView: NSTextView let tableView: KeyHandlingTableView + let scrollView: NSScrollView } private func makeEditing(value: String, selection: NSRange) -> Editing { let tableView = KeyHandlingTableView() + let scrollView = NSScrollView() + scrollView.documentView = tableView let editor = CellOverlayEditor() editor.install( in: tableView, @@ -34,7 +37,7 @@ struct CellOverlayEditorMovementTests { CellOverlayBase.applyCellTextLayout(to: textView) textView.string = value textView.setSelectedRange(selection) - return Editing(editor: editor, textView: textView, tableView: tableView) + return Editing(editor: editor, textView: textView, tableView: tableView, scrollView: scrollView) } private struct Outcome { diff --git a/TableProTests/Views/Results/KeyHandlingTableViewOverlayTests.swift b/TableProTests/Views/Results/KeyHandlingTableViewOverlayTests.swift index 56058db3dd..f4670d666b 100644 --- a/TableProTests/Views/Results/KeyHandlingTableViewOverlayTests.swift +++ b/TableProTests/Views/Results/KeyHandlingTableViewOverlayTests.swift @@ -18,7 +18,7 @@ private final class StubColumnLayoutPersister: ColumnLayoutPersisting { func clear(for key: ColumnLayoutTableKey) {} } -@Suite("KeyHandlingTableView overlay raise") +@Suite("KeyHandlingTableView overlay stacking") @MainActor struct KeyHandlingTableViewOverlayTests { private func makeCoordinator() -> TableViewCoordinator { @@ -31,21 +31,53 @@ struct KeyHandlingTableViewOverlayTests { ) } - @Test("adding a subview while an overlay is active raises it to front without trapping") - func addingSubviewRaisesActiveOverlay() { - let tableView = KeyHandlingTableView() + @Test("adding a subview to the table leaves the selection overlay above it") + func addingSubviewKeepsSelectionOverlayOnTop() { + let tableView = KeyHandlingTableView(frame: NSRect(x: 0, y: 0, width: 200, height: 100)) + let selectionOverlay = GridSelectionOverlay(frame: tableView.bounds) + tableView.selectionOverlay = selectionOverlay + tableView.addSubview(selectionOverlay) + + tableView.addSubview(NSView(frame: NSRect(x: 0, y: 0, width: 10, height: 10))) + + #expect(tableView.subviews.last === selectionOverlay) + } + + @Test("an open overlay is mounted beside the table, above the rows") + func openOverlayIsMountedBesideTheTable() { + let tableView = KeyHandlingTableView(frame: NSRect(x: 0, y: 0, width: 200, height: 100)) let coordinator = makeCoordinator() tableView.coordinator = coordinator + let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: 200, height: 100)) + scrollView.documentView = tableView let editor = CellOverlayEditor() coordinator.overlayEditor = editor let container = CellOverlayContainerView(frame: NSRect(x: 0, y: 0, width: 80, height: 24)) editor.install(in: tableView, row: 0, column: 0, columnIndex: 0, container: container) - tableView.addSubview(NSView(frame: NSRect(x: 0, y: 0, width: 10, height: 10))) - - #expect(tableView.subviews.last === container) + let subviews = scrollView.subviews + let clipIndex = subviews.firstIndex { $0 === scrollView.contentView } + #expect(editor.isActive) + #expect(!container.isDescendant(of: tableView)) + #expect(clipIndex.map { $0 + 1 } == subviews.firstIndex { $0 === container }) editor.removeOverlay() + #expect(container.superview == nil) + } + + @Test("a table outside a scroll view opens no overlay") + func tableWithoutScrollViewOpensNoOverlay() { + let tableView = KeyHandlingTableView(frame: NSRect(x: 0, y: 0, width: 200, height: 100)) + let coordinator = makeCoordinator() + tableView.coordinator = coordinator + + let editor = CellOverlayEditor() + coordinator.overlayEditor = editor + let container = CellOverlayContainerView(frame: NSRect(x: 0, y: 0, width: 80, height: 24)) + editor.install(in: tableView, row: 0, column: 0, columnIndex: 0, container: container) + + #expect(!editor.isActive) + #expect(container.superview == nil) } } diff --git a/TableProTests/Views/Results/TableViewCoordinatorLayoutTests.swift b/TableProTests/Views/Results/TableViewCoordinatorLayoutTests.swift index 3633de250e..58e39fbec5 100644 --- a/TableProTests/Views/Results/TableViewCoordinatorLayoutTests.swift +++ b/TableProTests/Views/Results/TableViewCoordinatorLayoutTests.swift @@ -1199,6 +1199,8 @@ struct TableViewCoordinatorLayoutTests { tableIdentityChanged: true ) let tableView = try #require(coordinator.tableView) + let scrollView = NSScrollView() + scrollView.documentView = tableView tableView.dataSource = coordinator tableView.delegate = coordinator coordinator.updateCache() @@ -1234,5 +1236,6 @@ struct TableViewCoordinatorLayoutTests { #expect(column.width > originalWidth) #expect(coordinator.columnPresentation(for: 0, in: rows).accessory == .foreignKey) #expect(coordinator.userSizedColumnNames.isEmpty) + withExtendedLifetime(scrollView) {} } } diff --git a/TableProTests/Views/SortableHeaderEmphasisTests.swift b/TableProTests/Views/SortableHeaderEmphasisTests.swift index 1d2d5f808c..8a27139e6d 100644 --- a/TableProTests/Views/SortableHeaderEmphasisTests.swift +++ b/TableProTests/Views/SortableHeaderEmphasisTests.swift @@ -42,8 +42,8 @@ struct SortableHeaderEmphasisTests { #expect(SortableHeaderEmphasis.holdsFocus(tableView: table, in: window)) } - /// A cell edit installs the field editor below the table, so focus has to be resolved by - /// ancestry. Keying on identity alone dropped the header out of emphasis mid-edit. + /// Focus has to be resolved by ancestry. Keying on identity alone dropped the header out of + /// emphasis mid-edit. @Test("A responder inside the table still counts as focus") func descendantIsFirstResponder() { let (window, table) = makeWindow() @@ -53,6 +53,19 @@ struct SortableHeaderEmphasisTests { #expect(SortableHeaderEmphasis.holdsFocus(tableView: table, in: window)) } + /// The grid mounts its cell editor and viewer in the table's scroll view, beside the table + /// rather than inside it, so accessibility can reach them. + @Test("A responder in the table's scroll view, beside the table, still counts as focus") + func scrollViewDescendantIsFirstResponder() throws { + let (window, table) = makeWindow() + let scrollView = try #require(table.enclosingScrollView) + let field = NSTextField(frame: NSRect(x: 0, y: 0, width: 50, height: 20)) + scrollView.addSubview(field, positioned: .above, relativeTo: scrollView.contentView) + _ = window.makeFirstResponder(field) + #expect(!field.isDescendant(of: table)) + #expect(SortableHeaderEmphasis.holdsFocus(tableView: table, in: window)) + } + @Test("A responder outside the table does not count as focus") func siblingIsFirstResponder() { let (window, table) = makeWindow() diff --git a/TableProUITests/RecentTabSwitchingUITests.swift b/TableProUITests/RecentTabSwitchingUITests.swift index a5aed77194..f3aad5258a 100644 --- a/TableProUITests/RecentTabSwitchingUITests.swift +++ b/TableProUITests/RecentTabSwitchingUITests.swift @@ -10,11 +10,9 @@ import AppKit import XCTest final class RecentTabSwitchingUITests: UITestCase { - /// One launch for the whole gesture, since each phase leaves the order the next one starts from. - /// /// After opening Album, Artist, Customer and Employee, then selecting Album and Employee, the - /// order is Employee, Album, Customer, Artist. A tap goes back one tab; holding Control and - /// pressing Tab twice goes back two, and the list shows while Control is held. + /// order is Employee, Album, Customer, Artist. A tap goes back one tab and a second tap comes + /// back. Holding Control is covered by RecentTabSwitcherControllerTests. func testControlTabWalksTabsInTheOrderTheyWereUsed() throws { let app = try launchWithSampleDatabase() let window = try readyWindow(of: app) @@ -39,29 +37,6 @@ final class RecentTabSwitchingUITests: UITestCase { app.typeKey(.tab, modifierFlags: .control) XCTAssertTrue(waitForSelection("Employee", in: window), "A second tap must come back to Employee") - - let panel = switcherPanel(in: app) - XCUIElement.perform(withKeyModifiers: .control) { - app.typeKey(.tab, modifierFlags: []) - XCTAssertTrue(panel.waitToExist(timeout: 10), "Holding Control must show the list of recent tabs") - app.typeKey(.tab, modifierFlags: []) - } - XCTAssertTrue( - waitForSelection("Customer", in: window), - "Two presses while holding Control must land two tabs back, on Customer. Got " - + (selectedTabLabel(in: window) ?? "none") - ) - XCTAssertTrue( - waitForPredicate(timeout: 10) { !panel.exists }, - "Letting go of Control must close the list" - ) - - XCUIElement.perform(withKeyModifiers: .control) { - app.typeKey(.tab, modifierFlags: []) - app.typeKey(.escape, modifierFlags: []) - } - Thread.sleep(forTimeInterval: 1) - XCTAssertEqual(selectedTabLabel(in: window), "Customer", "Escape must end the switch where it started") } // MARK: - Helpers @@ -113,8 +88,4 @@ final class RecentTabSwitchingUITests: UITestCase { tab.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)).click() XCTAssertTrue(waitForSelection(name, in: window), "Clicking the \(name) tab must select it") } - - private func switcherPanel(in app: XCUIApplication) -> XCUIElement { - app.children(matching: .any).matching(identifier: "recent-tab-switcher-panel").firstMatch - } }