-
-
Notifications
You must be signed in to change notification settings - Fork 643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support IAccessible2 labelled-by relation #17437
Merged
seanbudd
merged 10 commits into
nvaccess:master
from
michaelweghorn:michaelweghorn/iaccessible2_labelled_by
Dec 2, 2024
Merged
Support IAccessible2 labelled-by relation #17437
seanbudd
merged 10 commits into
nvaccess:master
from
michaelweghorn:michaelweghorn/iaccessible2_labelled_by
Dec 2, 2024
Conversation
This file contains 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
### Link to issue number: Fixes nvaccess#17436 ### Summary of the issue: IAccessible2 has an IA2_RELATION_LABELLED_BY relation type, see https://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/group__grp_relations.html#ga7bbace7ffb57476b75d621af2e27d1ff . However, that one was not taken into account by NVDA's "labeledBy" property for objects. This could could be seen for example with LibreOffice that implements handling of that IAccessible2 relation. Additionally, a `None` return value from IAccessibleHandler.accNavigate wasn't handled in `IAccessible._get_labeledBy`, triggering an error, e.g.: >>> focus.labeledBy Traceback (most recent call last): File "<console>", line 1, in <module> File "C:\tools\cygwin\home\user\development\git\nvda\source\baseObject.py", line 59, in __get__ return instance._getPropertyViaCache(self.fget) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\tools\cygwin\home\user\development\git\nvda\source\baseObject.py", line 167, in _getPropertyViaCache val = getterMethod(self) ^^^^^^^^^^^^^^^^^^ File "C:\tools\cygwin\home\user\development\git\nvda\source\NVDAObjects\IAccessible\__init__.py", line 1167, in _get_labeledBy (pacc, accChild) = IAccessibleHandler.accNavigate( ^^^^^^^^^^^^^^^^ TypeError: cannot unpack non-iterable NoneType object ### Description of user facing changes In NVDA's Python console, retrieving the "labeledBy" property now works for objects in applications implementing the "labelled-by" IAccessible2 relation and no longer triggers an error. ### Description of development approach * Extend `IAccessibleHandler.RelationType` with the `LABELLED_BY` relation as defined in the IAccessible2 spec. * Use this relation in `IAccessible._get_labeledBy` before falling back to trying a custom Mozilla/Gecko specific NAVRELATION. * Add handling for the case where `IAccessibleHandler.accNavigate` for the custom Mozilla/Geck approach returns `None` to fix the error triggered otherwise when no relation is set. ### Testing strategy: 1. start NVDA 2. Start LibreOffice Writer 3. open the options dialog: "Tools" -> "Options", go to the "User Data" page 4. move focus to the "Company" text edit 5. open NVDA's Python console (Ctrl+Insert+Z) 6. print the object that the currently focused edit is labelled by, and it's accessible name and role: >>> focus.labeledBy <NVDAObjects.IAccessible.IAccessible object at 0x0BC55A70> >>> focus.labeledBy.name 'Company:' >>> focus.labeledBy.role <Role.STATICTEXT: 7> ### Known issues with pull request: None ### Code Review Checklist: - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken. <!-- Please keep the following --> @coderabbitai summary
LeonarddeR
approved these changes
Nov 26, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
seanbudd
reviewed
Nov 27, 2024
Use qualified name. Otherwise the newly introduced from __future__ import annotations in commit feea4aa Author: Michael Weghorn <[email protected]> Date: Wed Nov 27 09:12:38 2024 +0000 Add type annotation causes source/NVDAObjects/IAccessible/__init__.py:2391:7: F811 Redefinition of unused `List` from line 13 | 2391 | class List(IAccessible): | ^^^^ F811 2392 | def _get_role(self): 2393 | return controlTypes.Role.LIST | = help: Remove definition: `List` Found 1 error.
seanbudd
reviewed
Nov 29, 2024
Without deferred evaluation (s. PEPs mentioned in the newly added comment): $ ./runlint.bat Ensuring NVDA Python virtual environment call ruff check --fix source\NVDAObjects\IAccessible\__init__.py:1165:30: F821 Undefined name `IAccessible` | 1163 | return True 1164 | 1165 | def _get_labeledBy(self) -> IAccessible | None: | ^^^^^^^^^^^ F821 1166 | label = self._getIA2RelationFirstTarget(IAccessibleHandler.RelationType.LABELLED_BY) 1167 | if label: | Found 1 error. Deactivating NVDA Python virtual environment
seanbudd
reviewed
Dec 1, 2024
seanbudd
approved these changes
Dec 1, 2024
seanbudd
reviewed
Dec 1, 2024
See test results for failed build of commit 263c3c810f |
seanbudd
reviewed
Dec 2, 2024
Thanks for the review and bringing this in line with the NVDA code conventions, @seanbudd ! |
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.
Link to issue number:
Fixes #17436
Summary of the issue:
IAccessible2 has an IA2_RELATION_LABELLED_BY relation type, see https://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/group__grp_relations.html#ga7bbace7ffb57476b75d621af2e27d1ff . However, that one was not taken into account by NVDA's "labeledBy" property for objects. This could could be seen for example with LibreOffice that implements handling of that IAccessible2 relation.
Additionally, a
None
return value fromIAccessibleHandler.accNavigate wasn't handled
in
IAccessible._get_labeledBy
, triggering an error, e.g.:Description of user facing changes
None, unless they're using the Python console or addons making use of the property.
Description of development approach
IAccessibleHandler.RelationType
with theLABELLED_BY
relation as defined in the IAccessible2 spec.IAccessible._get_labeledBy
before falling back to trying a custom Mozilla/Gecko specific NAVRELATION.IAccessibleHandler.accNavigate
for the custom Mozilla/Geck approach returnsNone
to fix the error triggered otherwise when no relation is set.Testing strategy:
start NVDA
Start LibreOffice Writer
open the options dialog: "Tools" -> "Options", go to the "User Data" page
move focus to the "Company" text edit
open NVDA's Python console (Ctrl+Insert+Z)
print the object that the currently focused edit is labelled by, and it's accessible name and role:
Known issues with pull request:
None
Code Review Checklist:
@coderabbitai summary