UIA objects and app modules: convert obj.UIAElement.cachedAutomationId to obj.UIAAutomationId#13125
Merged
Merged
Conversation
…-> obj.UIAAutomationId. Re nvaccess#11483. Replace current AutomationID with obj.UIAAutomationId. The biggest benefit is that code surrounding obj.UIAAutomationId call does not have to check for COM error exception (the actual definition found in UIA NVDA objects does catch COM error exception) which simplifies the code. Also, perform general lint on surrouding code inside the function where obj.UIAAutomationId is called (most notably, adding whitespace around operators).
…1483. Change obj.UIAElement.cachedAutomationID to obj.UIAAutomationId, along iwth linting emoji panel app module due to line length changes.
…nvaccess#11483. Convert obj/self.UIAElement.cachedAutomationID to obj.UIAAutomationId, accompanied bh linting Word document edits.
…(dead code). Re nvaccess#11483. Planned for a while: since obj.UIAAutomationId catches COM error, there is no need for UIA object's overlay class finder to handle this exception for search fields and suggestion lists. Therefore remove COM error handler for search fields.
Contributor
Author
|
Perhaps postponing this to 2022.2 or 2023.1. Thanks. |
Contributor
|
@josephsl Sorry for the delay. Is there a technical reason to defer this? If so, please mark as a draft. |
feerrenrut
approved these changes
Feb 15, 2022
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
The changes here are what is described, I don't see any issues. I'd like @michaelDCurran to confirm this approach is expected.
Contributor
Author
|
Hi, I was under the impression that, with the exchange between master and beta branches, 2022.1 milestone might be coming to a close in a few weeks a.k.a. NV Access preparing to tag the beta release in a few weeks. I can work on this branch – I need to check if I need to edit some few files, and then the PR should be good to go. I’ll give this a priority boost from my part within a day or so. Thanks for the reminder.
|
Contributor
Author
|
Hi, thanks for getting back to this. Can we make sure we won’t come across cachedAutomationId before we merge it? Thanks.
|
michaelDCurran
approved these changes
Feb 15, 2022
michaelDCurran
left a comment
Member
There was a problem hiding this comment.
This approach seems perfectly fine to me. @feerrenrut I am approving on this basis. Though I have not read through every specific change myself.
Contributor
|
Thanks @michaelDCurran, I'll wait for @josephsl to confirm this is ready to merge. Todo:
|
Contributor
Author
|
Hi, There are two instances of cachedAutomationId that should be kept:
Therefore, these will not be changed, and apart from that, things should be good to go (conflicts resolved and file headers updated). |
Contributor
|
@josephsl could you double check the changes file entry for accuracy please? |
Contributor
Author
|
Hi, for now the entry looks good. I wish we can come up with some kind of a process to document UIA properties by linking it to Microsoft Docs pages, something I think should be done at some point in the future when folks work on docs improvements. Thanks.
|
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
4 participants
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:
Closes #11483
Summary of the issue:
As part of #11445 discussion, a UIAAutomationId getter was introduced to UIA NVDA object class in NVDA 2020.3. subsequent UIA work uses UIAAutomationId property but it wasn't expanded to cover all NVDA source code.
Description of how this pull request fixes the issue:
Converts obj.UIAElement.cachedAutomationId call to use obj.UIAAutomationId, which fetches (I believe) current Automation Id string. Most source code fragments that call cachedAutomationId were converted except for:
In addition, general lint was done for modified files, along with recovering COM error handler for UIA search fields, suggestions lists, and parts of Explorer and Open With app modules since obj.UIAAutomationId will catch COM error itself and return an empty string.
Testing strategy:
Manual, system, and unit tests:
Known issues with pull request:
None
Change log entries:
Unnecessary unless a note on encouraging people to use UIAAutomationId property for UIA objects should be documented in changes for developers section.
Code Review Checklist:
Additional considerations:
PR #13107 comes with lint and changes for emoji panel app module, but it doesn't matter which PR gets tested, reviewed, and integrated first.
Thanks.