Fix Issue 12147: new focus target is not always announced#12252
Merged
Conversation
If the previous focus object was removed, we might hit a false positive for overlap detection when the focus target is adjacent to the focus trigger. This tracks the previous focus target so that we can account for this scenario when detecting overlaps.
See test results for failed build of commit f280600b28 |
See test results for failed build of commit 1eea41e921 |
Contributor
Author
|
I'm feeling pretty confident about this solution, but this is my first PR and dive into NVDA, so please let me know if I can improve it in any way. |
michaelDCurran
approved these changes
Apr 20, 2021
michaelDCurran
left a comment
Member
There was a problem hiding this comment.
Thanks @mfairchild365 this is great.
See test results for failed build of commit e8b7de8ba9 |
8 tasks
michaelDCurran
added a commit
that referenced
this pull request
Jun 8, 2021
…ates when checking if an object is defunct (dead), treat the object as defunct anyway. This ensures that the new focus will still be reported. (#12528) Fixes #12527 Summary of the issue: In UI Automation browse mode implementations, sometimes the new focus is not announced if the old focus is removed from the DOM as the focus changes. This is true even in focus mode. PR #12252 introduced a check for STATE_DEFUNCT on the old focus's states property. Although this works fine in Firefox / Chrome (they leave dead objects around with a defunct state), some UI Automation elements become completely unusable, and therefore an exception is raised when trying to fetch the states property. As reported in issue #12527 The newly focused tweet in the Twitter app is no longer announced when pressing j / keys. And If using Edge/Chromium with UIA, some focus changes in Microsoft Word 365 Online are not announced. Description of how this pull request fixes the issue: In BrowseMode document's event_gainFocus method, Catch all exceptions when fetching states to check for defunct, and if an exception is raised, log the error and treat the object as being defunct.
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:
fixes #12147
Summary of the issue:
The new focus target is not always announced when:
This is because of the overlapping logic for browse mode focus changes that is used to prevent duplicate announcements. That overlapping logic did not account for this scenario.
See the linked codepens in #12147 for more examples.
I can not specially point to an instance of this in the wild, as all of my use-cases are behind a firewall. However, I know that this behavior is not unusual in SPA-style web apps.
Description of how this pull request fixes the issue:
This PR fixes the issue by altering the overlapping detecting logic so that:
DEFUNCT(no longer available) and an overlap was detected, then continue as if there was not an overlap.Testing strategy:
Known issues with pull request:
Change log entry:
Section: New features, Changes, Bug fixes
Code Review Checklist:
This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x:
[ ]becomes[x].You can also check the checkboxes after the PR is created.