fix: improve annotation control accessibility - #2639
Conversation
Expose translated names and caption relationships for AI and zoom controls. Report brightness and contrast values as percentages through the accessibility interface while retaining existing keyboard steps.
Record the annotation control accessibility fixes for the next release.
wkentaro
left a comment
There was a problem hiding this comment.
Verified on the PR head: the neutral rescale is consistent (dialog divides by the new constant, the app only keeps slider ints in an in-memory dict, nothing persists them), arrow/page steps stay at 2% / 20%, and the accessible value interface now reports 100 at neutral where main reports 50 with an empty name. Widget + related e2e tests, ruff, ty, and the translation check all pass locally. The fix is worth having.
Two things before merge:
Redundant setters. I probed Qt's name fallbacks directly (Qt 6.11.1). Several explicit setAccessibleName calls duplicate what Qt already derives, and each one costs 20 catalog entries:
- Run button: name comes from its text. The explicit call is dead.
- Score / IoU spinboxes: name comes from the new buddy labels. The explicit calls are dead.
- Zoom: name comes from the new buddy label in
_app.py. The explicit name and the newZoomWidgettranslation context are dead weight; the tooltip rewording is unrelated scope. - Brightness / Contrast sliders: buddy already yields
Brightness:. The colon-less explicit name is a taste choice costing 2 strings x 20 languages; fine to keep if you want it, but say so.
Only the model combos, the output-format combo, and the icon-only info button genuinely need explicit names (verified: an icon-only tool button with a tooltip gets an empty Name). The three setAccessibleDescription calls are optional polish.
Tests are mostly tautological. Nearly every assertion is widget.accessibleName() == "X", which only proves the setter ran. The one meaningful assertion is the valueInterface().currentValue() == 100 check, since that is what assistive technology actually reads. Suggest switching the name assertions to QAccessible.queryAccessibleInterface(w).text(QAccessible.Text.Name): that checks the real outcome, covers the buddy path, and lets the redundant setters be deleted without losing coverage. Keep the step-size asserts; they guard a real regression from the rescale.
Net: drop the four redundant setters and their catalog entries, assert through the accessible interface. Roughly -50 lines across code and catalogs.
Read names and descriptions through the accessible interface in tests. Use Qt text and buddy-label fallbacks, and retain combo descriptions because Qt exposes the selected option as the accessible name. Remove the unrelated zoom tooltip change and unused translations.
|
Addressed in 1103df1.
Regenerated all 20 catalogs, removing seven unused messages per language. Validation: 71 focused tests, |
wkentaro
left a comment
There was a problem hiding this comment.
Re-verified on 1103df1: the redundant setters, the ZoomWidget rename, and the extra catalog entries are gone (4 new strings remain, all needed), and every name assertion now goes through the accessible interface. Also confirmed your finding that QComboBox reports the current item as Name regardless of an explicit accessible name, so description is the right hook there. Widget + related e2e tests, ruff, ty, mdformat, and the translation check pass locally. LGTM.
wkentaro
left a comment
There was a problem hiding this comment.
CI failure on Windows is a real defect, not a flaky test, and it also corrects what I said in my last comment.
Both failing tests assert the combo box Name equals the current item, and on Windows it comes back empty. Qt 6.11 QAccessibleComboBox::text (complexwidgets.cpp:415-425) is platform-split: on Unix (macOS and Linux) Name falls through to Value and reports the current item, ignoring any explicit accessible name. On Windows Name goes through the generic widget path, which means explicit accessible name, then buddy label, else empty. The combos have no label, so on Windows a screen reader now gets no name at all for the three combos. My earlier "description is the right hook" claim was macOS-only; sorry for the misdirection.
Fix in two lines per combo plus a platform-tolerant assertion:
- Restore
setAccessibleName(self.tr("Model"))/setAccessibleName(self.tr("Output format"))on the three combos. Keep the descriptions. Windows uses the name; Unix ignores it and reports the current item, which is fine. - In the tests, accept either:
assert name in (widget.tr("Model"), "EfficientSam (speed)"), or branch onsys.platform == "win32". A one-line comment citing the Qt platform split will save the next person the same trip.
That brings back the two Model / Output format catalog strings, which are now justified.
Qt uses widget names on Windows and selected option text on Unix. Restore explicit model and output-format names for Windows, and check the native platform contract through the accessible interface.
|
Fixed in 623c033; all 21 CI checks now pass. My previous conclusion about combo-box names applied only to Unix. Qt 6.11.1 has a platform branch: Windows reads the explicit widget name, while Unix exposes the selected option. Restored the three model/output name setters and their translations. The tests now assert each platform's native name and also verify the selected value and description through The original two failures are green on Windows with Python 3.12, 3.13, and 3.14. The other redundant setters and unrelated zoom tooltip change remain removed. |
Make annotation controls identifiable through accessibility APIs and make brightness/contrast values agree with the percentages shown on screen. At neutral, assistive technology previously received 50 while the dialog displayed 100%; both now report 100.
Brightness and contrast values are stored only in the running window, so the new percentage scale needs no persistent-data migration. Existing 2% arrow-key and 20% page-key steps are retained.
Qt 6.11.1 uses explicit combo-box names on Windows and selected-option text on Unix (Qt source). Model and output-format controls retain explicit names for Windows and descriptions for their purpose. Other controls use native text or buddy-label names, with explicit names for the prompt and icon-only info buttons. Tests check each platform’s native names, descriptions, and values through
QAccessible, including zoom in the main window.Validation: all nine AI widget tests passed locally after the Windows fix, along with
make lintandmake check_translatefor all 20 languages. All 21 CI checks pass on the updated head, including the full Windows, macOS, and Linux test matrix across Python 3.12–3.14. Native macOS / Qt 6.11.1 inspection confirmed control names, percentage values, keyboard steps, and retained adjustments after reopening the dialog. Screen-reader speech and other operating systems were not exercised locally.Screenshots intentionally omitted: the visible layout is unchanged; the relevant evidence is the native accessibility inspection and regression tests.