-
-
Notifications
You must be signed in to change notification settings - Fork 641
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
Browse mode: only filter out speaking of focus events caused by NVDA, rather than trying to work out if the caret is still within the last focus. #14611
Conversation
… rather than trying to work out if the caret is still within the last focus.
See test results for failed build of commit 25cb221da3 |
See test results for failed build of commit f17541e498 |
@michaelDCurran Why do you plan to remove this setting? I always have it enabled and it is very useful for me in conjunction with the ObjectLocationTones add-on. It allows me to have a spatial notion of the location of screen elements. If that option is removed, I will no longer be able to automatically know the location of the object, because it will not be automatically focused. Thanks |
Chiming in to say I also keep this setting enabled. Both for this case, and also because some menus/controls will only expand on focus, it makes demos easier when working with sighted folks, and in the past when it's turned off I've had issues with the webpage not scrolling to the position of the virtual cursor. @brunoprietog Unfortunately, I don't have enough data to make a strong case for keeping this feature, and this feels off-topic for this pull request. Shoot me an email; perhaps this is something the two of us should work together on. @michaelDCurran is there any timeline for this removal? I do depend on this feature, and I'd like to advocate for it with something a bit more convincing than "Well, I like it and I think it should be turned on, and sometimes it causes me issues when it's off". |
…n the same link. (#15234) Fixes #15211 Summary of the issue: After merging of pr #14611, tabbing through a message in Windows Mail would cause the caret / focus to get stuck and not move forward to the next element in tab order. This seems to be because BrowseModeDocument's gainFocus event updates the caret to the position of the element in the document that just gained focus. This is appropriate for web documents where NVDA manages the caret itself, but not for documents where the caret is managed by the application, such as the MS Word mail message viewer. This has most likely been a problem in some way for a long time, but pr #14611 made it much more obvious as it greatly increased the amount of focus events within documents now spoken / handled. Description of user facing changes NVDA no longer gets stuck when tabbing through a message in Windows Mail. Description of development approach Suppressing of caret movement in BrowseModeDocument's gainFocus event is controled by a new private variable on the CursorManager class. It is False by default, which does not allow the caret movement to occur. However, for ReviewCursorManager and VirtualBuffer classes, it is True, allowing the existing caret movement behaviour, as in these situations the caret is managed entirely by NVDA. This then means that in the cases where a CursorManager is used that is not VirtualBuffer or ReviewCursorManager, which is just Microsoft Word browse mode, NVDA will not move the caret itself on gainFocus events, as the application itself will already do this.
…ically focus focusable elements disabled (#16016) Fixes #15653 Summary of the issue: Two bugs are seen if automatically focus focus elements in browse mode is disabled. Possibly introduced when that setting was introduced, and made worse when browse mode focus code was further refacted in #14611. In iTunes, arrowing up and down inside a combo box incorrectly switches back to browse mode. In Firefox / Chrome, closing a combo box does not automatically switch back to browse mode. Description of user facing changes NVDA will again switch back to browse mode when closing combo boxes with escape or alt+upArrow in Firefox or Chrome. (Action Items in iTunes not accessible #15653) Arrowing up and down in combo boxes in iTunes will no longer inappropriately switch back to browse mode. (Action Items in iTunes not accessible #15653) Description of development approach BrowseModeDocument's collapseOrExpandControl script: if automatic focus focusable elements is disabled, delay the rest of the script to give time for the control to actually get focus. Webkit (iTunes) virtualBuffer: do not treat the list items of a combo box as being part of the virtualBuffer.
…ically focus focusable elements disabled (nvaccess#16016) Fixes nvaccess#15653 Summary of the issue: Two bugs are seen if automatically focus focus elements in browse mode is disabled. Possibly introduced when that setting was introduced, and made worse when browse mode focus code was further refacted in nvaccess#14611. In iTunes, arrowing up and down inside a combo box incorrectly switches back to browse mode. In Firefox / Chrome, closing a combo box does not automatically switch back to browse mode. Description of user facing changes NVDA will again switch back to browse mode when closing combo boxes with escape or alt+upArrow in Firefox or Chrome. (Action Items in iTunes not accessible nvaccess#15653) Arrowing up and down in combo boxes in iTunes will no longer inappropriately switch back to browse mode. (Action Items in iTunes not accessible nvaccess#15653) Description of development approach BrowseModeDocument's collapseOrExpandControl script: if automatic focus focusable elements is disabled, delay the rest of the script to give time for the control to actually get focus. Webkit (iTunes) virtualBuffer: do not treat the list items of a combo box as being part of the virtualBuffer.
Link to issue number:
Fixes #12896
Fixes #6606
Fixes #14494
Fixes #14495
Fixes #14606
Summary of the issue:
NVDA sometimes fails to report focus in browseMode documents, as it is trying to filter out redundant focus events caused by moving the caret.
Particular cases:
Currently, NVDA chooses to ignore any focus in browseMode that overlaps with the browseMode caret. This is so that any focus event that resalts from moving the caret is not reported as this would be redundant. However, this rule is clearly too limiting as it stops the above examples from working.
Description of user facing changes
In Browse mode, NVDA will no longer incorrectly ignore focus moving to a parent or child control e.g. moving from a control to its parent lit item or gridcell.
Note however that this fix only applies when the Automatically set focus to focusable elements option in Browse Mode settings is turned off (which is the default).
Description of development approach
Rather than ignoring focus changes in browse mode if the new focus's range overlaps the existing caret position, focus is only ignored if it is the same object that NvDA has specifically requestes etting focus to, such as when pressing the applications key or activating a control.
Testing strategy:
Performed steps in issues #14494, #14495, #12896, #6606.
Ensured that fixed issue #8341 is not regressed.
Known issues with pull request:
This fix only applies if the Automatically set focus to focusable elements option in Browse Mode settings is turned off (which is the default). Thus anyone who has turned on this option (to get back very old behaviour before 2019) won't get this fix. However, it is planned to remove this option completely in the near future.
Change log entries:
New features
Changes
Bug fixes
For Developers
Code Review Checklist: