Skip to content

fix(datagrid): give assistive clients a data cell they can read (#2381) - #2392

Merged
datlechin merged 3 commits into
mainfrom
fix/green-main-ui-tests
Aug 23, 2026
Merged

fix(datagrid): give assistive clients a data cell they can read (#2381)#2392
datlechin merged 3 commits into
mainfrom
fix/green-main-ui-tests

Conversation

@datlechin

Copy link
Copy Markdown
Member

Main has been red for three runs. Eight UI tests failed in run 32631046978, in two unrelated groups.

The data grid published no accessibility at all

Since #2388 every data cell in every result was a blank AXCell. VoiceOver read the whole grid as empty, and the two grid UI tests were the symptom.

NSTableView builds its AXCell tree from cell views and from nothing else. The NSAccessibilityElements the row published never reached that tree, and AppKit put its own placeholder in each one's place. Measured four ways on macOS 27: the element's identifier, value, label and children never appear however they are set; overriding the accessibility methods instead of using the setters changes nothing; an element that declares .staticText still shows up as Cell; and the frames in the tree follow AppKit's own convention (column width less the intercell spacing) rather than the ones the element reports.

So a drawn cell can only speak through a view. DataGridCellAccessibilityView is one per data cell that draws nothing, takes no clicks, and reads its text through the coordinator so an edit is spoken with nothing to invalidate. It mounts only once DataGridAccessibility.isActive, which the first accessibility question asked of the grid sets: nothing reports an attached client, since NSWorkspace.isVoiceOverEnabled covers VoiceOver alone and nothing at all reports Switch Control, Voice Control, an inspector or XCUITest. A session with no assistive client keeps #2381's numbers exactly: 26 views for a 500-column table rather than 12,500.

Two smaller fixes ride along. The row now answers an accessibility hit test for the width it covers that no cell does, instead of falling through to the view that draws the cells, which is not in the tree. And that drawing view is out of the tree entirely.

testCommandDeleteDeletesTheEditorLineAfterSelectingAResultRow also had to stop clicking a row element. A table publishes its columns as siblings of its rows, each as tall as every row it spans and later in the tree, so XCUITest reads every row and every cell in the grid as obscured and refuses to click either. It clicks a point offset from the data-grid element now, the same shape #1837 needed.

Compare & Sync tested a window that never opens

All five CompareSyncUITests cases failed, and PR #1968 was merged with them failing. They could not have passed anywhere: the window is Starter-gated, the UI test sandbox carries no license, and choosing the menu item puts up the upgrade alert instead. The assertions were written against an earlier design too, looking for a popUpButtons value of "Choose a connection" that the rebuilt toolbar spells "Choose Source".

The suite now covers what an unlicensed run does, the way QueryInsightsTabUITests already does for its own gate: the menu item is reachable and stays enabled, because a feature the user can buy has to be discoverable, and choosing it explains the gate and opens no window. The window's own start-state contract moved to CompareSyncStartStateTests, which reaches it without a license: neither endpoint preselected, Compare and Swap refusing with a reason, the banner saying nothing has been written.

This leaves the window itself without UI coverage. Giving it any would mean a way to grant a license in the sandbox, which is a licensing bypass to design rather than a CI fix.

Verification

  • DrawnCellReachabilityTests rewritten: no view mounts while accessibility is idle, an active client gets a cell carrying its value, an edited cell reads back without invalidation, the mounted cell takes no clicks, a hit test past the last column lands on the row.
  • 182 data-grid unit tests pass, plus the 6 new Compare & Sync ones.
  • Locally: QueryTabDeleteLineUITests, QueryHistoryActionsUITests, CompareSyncUITests, ResultStatusBarUITests, SQLiteFirstTableLoadUITests and SidebarTableTabUITests all pass. testReturnInTheListLoadsTheSelectedEntry fails on this machine, and fails identically on unmodified main; it passes on CI.
  • No CHANGELOG entry: both Data grid breaks with many columns: flickering, columns stop rendering, horizontal scroll lags behind viewport #2381 and Data sync between two connections #721 are unreleased, so per CLAUDE.md the fixes fold into the entries already there.

https://claude.ai/code/session_01NAexgozD2h1j1EetZ6ArMH

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 42149e6 into main Aug 23, 2026
8 checks passed
@datlechin
datlechin deleted the fix/green-main-ui-tests branch August 23, 2026 15:49
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.

1 participant