Conversation
…ement cannot be invoked, toggled or selected.
… with the NVDAObject's doAction method, keep going up the ancestors until DoAction succeeds.
|
Unless I'm missing something, this is not going to work well for non-semantic clickable controls. Obviously, those are authoring error, but there are plenty of them. This might also result in a click potentially getting routed to a far away, irrelevant ancestor that happens to have a click handler for event listening purposes. With gecko_ia2, when walking ancestors, if doAction fails, NVDA simulates a mouse click if the ancestor has a relevant location and considers that a success. This does occasionally fail and click the wrong place, since location might not be perfect due to bugs, pop-ups obscuring things, etc. That's why Chrome and Firefox implemented click ancestor, but I guess we can't access that with UIA, so click simulation is probably the best we can do. |
…the base BrowseMode's _activateNvDAObject, and remove UIA browseMode's implementation. All browse mode documents will now use gecko_ia2's approach to activating which is to first try doAction, and if that fails, then perform a click if the object is visible, otherwise, go up the ancestors doing the same thing until one of those succeeds.
|
@jcsteh I've now moved that gecko_ia2 logic into the base browse mode _activateNVDAObject. I can't see any reason why all implementations can't use that then if we are happy with it. |
There was a problem hiding this comment.
"Happy with it" is probably a stretch. As I noted, it can result in clicking the wrong thing in some cases. However, I think this is probably the most pragmatic solution given the prevalence of non-semantic clickables with ancestor click handlers.
Could this be a problem for non-web contexts; Microsoft Word, etc.?
| return | ||
| elif self.UIASelectionItemPattern: | ||
| self.UIASelectionItemPattern.select() | ||
| return |
There was a problem hiding this comment.
Why this change? I don't think there's a functional difference, but having elif + return is redundant. If you want to keep the early returns in each branch, I'd change the elifs to ifs.
There was a problem hiding this comment.
Previously it would raise NotImplementedError only if actionIndex was not 0. Now it also raises NotImplementedError if neither invoking, toggling or selecting is possible. At least, that was what I was trying to go for...
There was a problem hiding this comment.
Ah gotcha. Sorry, missed that subtlety. In that case, please change the elifs to ifs and we're good here.
See test results for failed build of commit 5dc72b2c99 |
|
@jcsteh Some quick tests with MS Word seems to suggest things are working okay. It is possibly now more accurate when jumping to references. |
| return | ||
| elif self.UIASelectionItemPattern: | ||
| self.UIASelectionItemPattern.select() | ||
| return |
There was a problem hiding this comment.
Ah gotcha. Sorry, missed that subtlety. In that case, please change the elifs to ifs and we're good here.
See test results for failed build of commit 2bc1100bfa |
seanbudd
left a comment
seanbudd
left a comment
There was a problem hiding this comment.
Please fix the failing lint check before merging
seanbudd
added
the
conceptApproved
5 participants
Link to issue number:
Fixes #14612
Summary of the issue:
when accessing Microsoft Edge with UI Automation, NVDA is unable to activate most buttons and controls in browse mode.
This is because NvDA only tries to invoke/select/toggle the deepest element at the browse mode caret position, which may be a text leaf, or span within the interactive control, and not the control itself.
Description of user facing changes
Microsoft Edge via UI Automation: NVDA can now activate buttons and other controls in browse mode.
Description of development approach
Testing strategy:
Known issues with pull request:
None known.
Code Review Checklist: