fix(tabs): keep the editor tabs reachable to accessibility over the pointer's owner - #2576
Merged
Merged
Conversation
…ointer's owner Claude-Session: https://claude.ai/code/session_01L7uaHbJBPV1LaWL5QXzxyp
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mainhas been red since #2571 merged. This is why, and the fix.What broke
EditorTabInteractionView.hitTestclaims the tab track so the pointer has a single owner, which is what #2571 was for. Accessibility resolves a screen point through that same method, so the claim answered "the strip" for every tab, and the strip publishes nothing: there was no element under a tab to speak, press or click.So this was never only a test problem. VoiceOver, Switch Control and Voice Control lost the tabs at the same time XCUITest did.
The claim is now lifted for the length of an
accessibilityHitTest, and the pointer keeps it.The failures it accounts for
Six cases across all three UI shards, in two suites I added and one that predates the change:
EditorTabReorderUITeststestAFastDragReordersTheStripEditorTabReorderUITeststestDraggingATabNeverMovesTheWindowEditorTabDetachUITeststestMoveTabToNewWindowTakesTheTabOutOfTheStripEditorTabDetachUITeststestClosingTheDetachedWindowLeavesTheOriginalWorkingQueryInsightsTabUITeststestTheQueryInsightsTabOpensOnceCarriesItsFiltersAndStaysGatedWithoutALicenseEditorTabDetachUITeststestTheCommandIsOfferedOnAnIdleTabThe last one is separate and is mine: Move Tab to New Window exists on the AppKit contextual menu the pointer uses and on the SwiftUI menu kept for VoiceOver, so the query matched twice. It now takes
firstMatch.QueryInsightsTabUITestsis the one that shows the blast radius: it predates all of this and was green until #2571.Why I am confident without a local reproduction
mainwas green atbb0371ea2, the commit before #2571, and has failed every run sincef48d20217, which is #2571 itself. The failures are all "not hittable" oneditor-tab, and the only change to how those elements are hit-tested is thehitTestclaim.I could not reproduce it here:
TableProUITestsdoes not run on this machine at all, at cleanmainas well as on a branch, and a standalone harness cannot answer the question either because SwiftUI builds no accessibility tree until a client attaches. CI is the verification for this one.Verified
buildPASS.swiftlint lint --strictclean.https://claude.ai/code/session_01L7uaHbJBPV1LaWL5QXzxyp