Fix TextField accessibility - contentDescription was ignored#2680
Fix TextField accessibility - contentDescription was ignored#2680m-sasha merged 1 commit intoJetBrains:jb-mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| if (setText != null) { | ||
| return semanticsConfig | ||
| .getOrNull(SemanticsProperties.ContentDescription) | ||
| ?.mergeText() | ||
| ?: text?.toString() | ||
| } |
There was a problem hiding this comment.
Is there a reason we should not just always return
(semanticsConfig.getOrNull(SemanticsProperties.Text) ?: semanticsConfig.getOrNull(SemanticsProperties.ContentDescription))
?.mergeText()
?
There was a problem hiding this comment.
The setText != null check is necessary because Text ?: ContentDescription doesn't work for the fallback case.
I tested your suggestion and it fails the textFieldAccessibleNameFallsBackToTextContent test. The issue is that for TextFields, SemanticsProperties.Text may exist in the config but doesn't provide the actual text content—that comes from the text field (via AccessibleText).
So when there's no contentDescription, we need text?.toString() as fallback, not SemanticsProperties.Text.
There was a problem hiding this comment.
Err, sorry, I meant the other way around (ContentDescription ?: Text).
My point was that it seems to me the content description should be used (if set) not just for text fields (or editable text components in general).
There was a problem hiding this comment.
I understand your point—using ContentDescription ?: Text for all components would be more consistent.
However, this fails the textFieldAccessibleNameFallsBackToTextContent test because SemanticsProperties.Text is null/empty for editable TextFields.
The fallback needs text?.toString() (from the Java AccessibleText API), not SemanticsProperties.Text (from Compose semantics). The setText != null check ensures we use the correct fallback source specifically for text fields.
There was a problem hiding this comment.
My question wasn't about semanticsConfig.getOrNull(SemanticsProperties.Text) vs. text (which is just a calculated property of ComposeAccessibleComponent, by the way; it doesn't come from the Java API), although that's a valid question too. It was about using ContentDescription even if setText is null. Is there a reason not to do that?
About the question of semanticsConfig.getOrNull(SemanticsProperties.Text) vs. text (which itself is EditableText ?: Text): I don't think it's correct to use EditableText in getAccessibleName (or getAccessibleDescription.
So my suggestion is:
- In
getAccessibleNamereturnContentDescription ?: Text. - In
getAccessibleDescriptionreturnContentDescription.
Do you know of any cases where this would result in undesirable behavior from the OS accessibility system?
There was a problem hiding this comment.
Hmm, getAccessibleDescription already returns ContentDescription.
So just the first suggestion then.
There was a problem hiding this comment.
Ah, sorry, I hadn’t understood it properly.
On my side, I also didn’t fully understand why EditableText was being used in getAccessibleName, but I intentionally kept it to avoid introducing overly large changes. My goal was really to fix this specific bug, without further altering the existing behavior.
To my knowledge, this should not cause any issues with screen readers: they should be able to read the content correctly through the AccessibleText interface.
Would you like me to apply the proposed changes?
There was a problem hiding this comment.
Would you like me to apply the proposed changes?
Yes, let's do that, and if/when anyone complains we'll fix it (and document exactly why).
There was a problem hiding this comment.
I've implemented your suggestion.
However, when running all AccessibilityTest tests together, 10 tests fail with assert(activeControllers.isEmpty()) in the cleanup. These same tests pass when run individually.
|
Please format the PR summary according to https://raw.githubusercontent.com/JetBrains/compose-multiplatform-core/refs/heads/jb-main/.github/PULL_REQUEST_TEMPLATE.md Also, if you haven't already, sign the Google Contributor's License Agreement at https://cla.developers.google.com to let us upstream your code to Google's AOSP repository |
|
@MSasha I've reformatted the PR according to the template. The description, testing details, and release notes are now properly structured. Regarding the Google CLA, I've signed it at https://cla.developers.google.com ✓ |
|
@kdroidFilter Please rebase on latest |
86c0a9e to
8c63496
Compare
|
@m-sasha I've rebased on the latest |
|
Thanks, running the CI tests now. |
- Changed getAccessibleName() to prioritize contentDescription over text - Added tests for TextField with and without contentDescription - Applied patch to initiallyFocusedElementNotifiesSystemOfFocus test
8c63496 to
eb896e9
Compare
|
@m-sasha I had an issue during the rebase - some deleted tests were reintroduced, which caused the CI to fail. I've fixed this now. Could you please restart the ci? |
|
Running |
|
@m-sasha |
Description
Fixes TextField accessibility issue on Desktop (macOS) where
contentDescriptionprovided viaModifier.semantics {}was completely ignored by VoiceOver screen readers. Before this fix, TextField components were unusable for screen reader users because the text content was used as both the accessible name (label) and value (content), making unlabeled fields indistinguishable.The fix updates
ComposeAccessible.getAccessibleName()to prioritizecontentDescriptionas the accessible name for text fields, aligning the behavior with iOS accessibility standards.Before
VoiceOver: "john@example.com" (no label)
After
VoiceOver: "Email", "john@example.com" (label + value)
Visual comparison:
Before fix:


After fix:


Testing
Unit Tests:
AccessibilityTest.ktcovering:contentDescription→ uses contentDescription as accessible namecontentDescription→ falls back to text contentManual Testing:
contentDescriptionRelease Notes
Fixes - Desktop
TextFieldaccessibility issue wherecontentDescriptionwas ignored by screen readers (VoiceOver).TextFieldnow properly usescontentDescriptionas the accessible name/label, making forms usable with assistive technologies.Google CLA
Note: I have signed the Google Contributor's License Agreement at https://cla.developers.google.com