BrowseModeDocumentTextInfo.getControlFieldSpeech: Report the name of groupings (such as fieldsets) for quicknav and focus jumps, similar to how landmark labels are reported.#7435
Conversation
…groupings (such as fieldsets) for quicknav and focus jumps, similar to how landmark labels are reported.
jcsteh
left a comment
There was a problem hiding this comment.
- I think the "grouping" role should be reported. This makes it more consistent with reporting in focus mode.
- Consider moving this into
speech.getControlFieldSpeech. You could put this near the code to support tables (around line 1160).
|
@jcsteh: ready for another review. But: I noticed this PR introduces double speaking if you are in focus mode due to a field needing passThrough, then tabbing directly into a grouping, where the focus lands on a field that does not require passthrough. E.g. Starting on an edit field, and then tabbing into a fieldset and landing on a checkbox. The grouping is announced twice. This is because focusEntered is processed before switching back to browseMode for the focus. Do we care about this? We'd have to stop focusEntereds when switching to browseMode, but we may have originally done this for a particular reason. |
jcsteh
left a comment
There was a problem hiding this comment.
While double speaking is better than what we have now (not speaking potentially important info), i can see this becoming a common gripe, so I think we should fix it now.
We'd have to stop focusEntereds when switching to browseMode, but we may have originally done this for a particular reason.
I don't think there was any reason other than there being no code to prevent it. The issue is that we get the focusEntered events before the mode switch, since the mode switch happens in gainFocus. We have explicit code to replay focus entered events when switching to focus mode because we ignore focusEntered events if passThrough is False. However, we always handle them when passThrough is True. One approach could be to always ignore them and always let gainFocus handle replay for focus mode.
|
|
||
| def getControlFieldSpeech(self, attrs, ancestorAttrs, fieldType, formatConfig=None, extraDetail=False, reason=None): | ||
| textList = [] | ||
| role=attrs.get('role') |
There was a problem hiding this comment.
This line is no longer necessary.
|
The double speaking is not at all limited to this grouping case. This can be also reproduced with a list. However, it is even worse as the problem will only happen the first time, due to yet another issue where focusing a control that requires passThrough, the textInfo speech cache for the treeInterceptor is not updated, thus when it hits another control that uses browse mode, the controlFields are compared with the last browse mode position. |
Fair enough, though it's worth noting that tabbing is more likely (or at least users are more likely to listen to most of the info) in a form, so encountering this for groupings is arguably more problematic than for lists. In other words, I'm suggesting that groupings might perhaps raise the priority of fixing that problem. |
…lse by default. If true, then speakTextInfo will process/cache controlFields and indenting info etc, but not actually speak anything. This is needed to allow browseMode to update its speech cache when moving around in focus mode, so that the next browseMode speech has the latest controlFields. * BrowseMode: Ensure that browseMode's textInfo speech cache is updated even when moving around in focus mode, so that when switching back to browse mode, NVDA does not inappropriately announce entering/exiting controlFields from before focus mode. * BrowseMode: don't speak new focus ancestors via focusEntered event when speaking focus in browseMode straight after being in focus mode.
jcsteh
left a comment
There was a problem hiding this comment.
Nice. If possible, I'd prefer to see REASON_ONLYCACHE used rather than a separate onlyCache argument for consistency. If this causes major problems, though, I could probably be convinced to let this fly. :)
| if obj==self.rootNVDAObject: | ||
| self._enteringFromOutside = True | ||
| if self.passThrough: | ||
| nextHandler() |
There was a problem hiding this comment.
I think it'd be good to have a comment here explaining that we ignore these events here, but event_gainFocus will replay them if appropriate.
…peech.speakTextInfo rather than requiring a new argument. Also comment why BrowseModeDocumentTreeInterceptor.event_focusEntered drops all events.
…A grids in focus mode. (#20091) Fixes #17750. Summary of the issue: When moving to an ARIA grid cell in focus mode in web browsers, NVDA previously reported both the row and column headers even if only the row or only the column changed. Description of user facing changes: When moving to an ARIA grid cell in focus mode in web browsers, NVDA no longer reports both the row and column headers even if only the row or only the column changed. Description of development approach: Previously, there were two problems: After c4c3da6 (BrowseModeDocumentTextInfo.getControlFieldSpeech: Report the name of groupings (such as fieldsets) for quicknav and focus jumps, similar to how landmark labels are reported. #7435), Browse mode was updating the speech properties cache before speaking the focused object, even in focus mode. That meant that the row and column coordinates for the newly focused cell were already cached before NVDA spoke it, which made NVDA behave as if the coordinates never changed. The fix is to update the cache from the TextInfo after speaking the focused object. Virtual buffers just used the IA2 unique id as the table id, but NVDAObjects use both the window handle and id, meaning that these didn't compare correctly in the speech cache. The fix is to make the vbuf normalize table ids to windowHandle and id.
…A grids in focus mode. (nvaccess#20091) Fixes nvaccess#17750. Summary of the issue: When moving to an ARIA grid cell in focus mode in web browsers, NVDA previously reported both the row and column headers even if only the row or only the column changed. Description of user facing changes: When moving to an ARIA grid cell in focus mode in web browsers, NVDA no longer reports both the row and column headers even if only the row or only the column changed. Description of development approach: Previously, there were two problems: After c4c3da6 (BrowseModeDocumentTextInfo.getControlFieldSpeech: Report the name of groupings (such as fieldsets) for quicknav and focus jumps, similar to how landmark labels are reported. nvaccess#7435), Browse mode was updating the speech properties cache before speaking the focused object, even in focus mode. That meant that the row and column coordinates for the newly focused cell were already cached before NVDA spoke it, which made NVDA behave as if the coordinates never changed. The fix is to update the cache from the TextInfo after speaking the focused object. Virtual buffers just used the IA2 unique id as the table id, but NVDAObjects use both the window handle and id, meaning that these didn't compare correctly in the speech cache. The fix is to make the vbuf normalize table ids to windowHandle and id.
Link to issue number:
Fixes #3321
Fixes #6979
Summary of the issue:
When jumping by quicknav or focus in browseMode, legends on fieldsets are not reported.
Description of how this pull request fixes the issue:
Announce the name of the fieldset (grouping) similar to how we announce the label of landmarks, but only do this for reason_focus so that we do not double speak the name due to it also being reachable by the caret within the content.
Known issues with pull request:
This will happen fr all groupings with a name in browseMode. Would there be some groupings that have badly calculated names that should not be spoken? Should we limit this specifically to fieldset somehow?
This works in Firefox and Chrome. But not in Edge as they do not correctly set the name of their fieldset groupings. A bug has been filed. This does not work in IE as we would have to add code to calculate the name of a fieldset from its legend ourselves. Is this worth doing?
Change log entry:
Section: changes
In Browse mode for Firefox and Chrome, the name of form field groups are now announced when moving into them with quick navigation or when tabbing.