Report label of lists in browseMode#8568
Conversation
…tering a list (including: name, level, states etc), not just item count.
| # General cases | ||
| elif ( | ||
| # General cases. | ||
| if ( |
There was a problem hiding this comment.
The conditions for these blocks are complicated, can you explain why this elif must become an if?
There was a problem hiding this comment.
In the past, all of the above if/elifs returned. Now one of them (the
lists one) does not return and needs to get to this point.
Oh I missed the removal of the return.
Perhaps I could move the if for the lists down to inside this general if
and switch it back to being an elif. Might that make the flow more
understandable?
No, I think leave it as is. It's the conditions themselves that make this hard to understand. It's hard to know which components of the conditions are exclusive and not.
|
In the past, all of the above if/elifs returned. Now one of them (the
lists one) does not return and needs to get to this point.
Perhaps I could move the if for the lists down to inside this general if
and switch it back to being an elif. Might that make the flow more
understandable?
|
source/speech.py
Outdated
|
|
||
| # Determine what text to speak. | ||
| # Special cases | ||
| if speakEntry and childControlCount and fieldType=="start_addedToControlFieldStack" and role==controlTypes.ROLE_LIST and controlTypes.STATE_READONLY in states: |
There was a problem hiding this comment.
Is it still important to have speakEntry here? It is checked before this value containerContainsText is used in the "General" section.
|
True. We technically don't have to check speakEntry in that if check
anymore.
|
…d to set containerContainsText variable.
|
@feerrenrut: I removed the check of the speakEntry variable. Does this look okay now? |
| # General cases. | ||
| if ( | ||
| (speakEntry and ((speakContentFirst and fieldType in ("end_relative","end_inControlFieldStack")) or (not speakContentFirst and fieldType in ("start_addedToControlFieldStack","start_relative")))) | ||
| or (speakWithinForLine and not speakContentFirst and not extraDetail and fieldType=="start_inControlFieldStack") |
There was a problem hiding this comment.
It may not be worth doing, but the a & not b & not c part here can be simplified to a & not (b or c)
user_docs/en/changes.t2t
Outdated
| - In the list of languages in NVDA's General Settings, language name for Burmese is displayed correctly on Windows 7. (#8544) | ||
| - In Microsoft Edge, NVDA will announce notifications such as reading view availability and page load progress. (#8423) | ||
| - Similar to other multiline text fields, When positioned at the start of a document in Braille, the display is now panned such that the first character of the document is at the start of the display. (#8406) | ||
| - When navigating into a list on the web, NvDA will now report its label if the web author has provided one. (#7652) |
There was a problem hiding this comment.
Just noticed the 'v' in NVDA is lowercase. I'll fix this.
feerrenrut
left a comment
feerrenrut
left a comment
There was a problem hiding this comment.
Assuming the build passes, this is ready to merge.
4 participants
Link to issue number:
Fixes #7652
Summary of the issue:
Web authors can label ul/ol lists with aria-label or aria-labelledby. However, NVDA does not currently speak these labels when entering the list with quicknav, or arrowing (when the label does not appear anywhere else on the page).
Description of how this pull request fixes the issue:
The information spoken when entering a list has now been generlized so that it includes all the standard properties such as name, states, level etc, which were previously being completely ignored.
Testing performed:
Using Firefox, Tested the testcase in #7652. NVDA announced the name of both lists.
Known issues with pull request:
None.
Change log entry:
Bug fixes: