Speech and Braille: don't report many clickables in a row in browse mode.#8922
Conversation
| if not inClickable: | ||
| # We have entered an outer most clickable or entered a new clickable after exiting a previous one | ||
| # Report it if there is nothing else interesting about the field, but not if the user turned it off. | ||
| if formatConfig['reportClickable']: |
There was a problem hiding this comment.
Would it be possible to check this earlier on? I guess it is not necessary to check the clickable state if we don't report clickables at all.
| isTextBlank=False | ||
| _speakTextInfo_addMath(speechSequence,info,field) | ||
|
|
||
| # When true, we are inside a clickable field, and should therefore not announce any more new clickable fields |
There was a problem hiding this comment.
Could you also provide this comment in braille.py when declaring inClickable?
| states=field.get('states') | ||
| if states and controlTypes.STATE_CLICKABLE in states: | ||
| # We entered the most outer clickable, so announce it, if we won't be announcing anything else interesting for this field, but not if the user turned them off. | ||
| if formatConfig['reportClickable']: |
There was a problem hiding this comment.
Same question as for de braille module.
| if not inClickable: | ||
| # We have entered an outer most clickable or entered a new clickable after exiting a previous one | ||
| # Announce it if there is nothing else interesting about the field, but not if the user turned it off. | ||
| if formatConfig['reportClickable']: |
Now we simply report clickable for the first clickable in a run of control starts. Hitting text, or control ends resets the run.
|
@LeonarddeR: I have addressed your review comments, and also somewhat
simplified the logic, which actually fixes a particular corner case
which was breaking.
Now we simply report the first clickable in a run of control starts, and
text or a control end breaks the run.
Now all tests in clickables.html in both speech and braille work as
expected. Previously the "Clickable spans in middle of clickable
paragraph" test was not correct in braille.
|
| # Announce it if there is nothing else interesting about the field, but not if the user turned it off. | ||
| presCat=command.field.getPresentationCategory(newControlFieldStack[0:],formatConfig,reason) | ||
| if not presCat or presCat is command.field.PRESCAT_LAYOUT: | ||
| tempTextList.append(controlTypes.stateLabels[controlTypes.STATE_CLICKABLE]) |
There was a problem hiding this comment.
What if we keep a list of roles where all of that type of role is inherently clickable. That way we don't say something is a clibkable link or a clickable button? Or does this code already do that?
There was a problem hiding this comment.
We are already doing this by checking the field's presentationCategory.
| inClickable=True | ||
| text=info.getControlFieldSpeech(command.field,newControlFieldStack,"start_relative",formatConfig,extraDetail,reason=reason) | ||
| if text: | ||
| tempTextList.append(text) |
There was a problem hiding this comment.
Should text being true turn a run of clickables off, or do I misunderstand this? I'm reading it pretty fast.
There was a problem hiding this comment.
Nope. That text variable just holds the speech for that particular control Start.
LeonarddeR
left a comment
There was a problem hiding this comment.
Looks way cleaner now, thanks!
Link to issue number:
Fixes #7430
Summary of the issue:
NVDA reports certain fields in browse mode as 'clickable' if the author has provided an onclick handler and NVDA would not normally report anything else for that field (E.g. has no reportable role).
However, with the increased use of onclick handlers on web pages, there are many cases where NVDA will report multiple clickables in a row E.g. "clickable clickable clickable clickable". This is extremely annoying.
To make things worse, versions of Chrome earlier than 63 also propagated the existence of the click action to all descendant accessibles, meaning that NVDA would report clickable for every descendant of a clickable node. Although this was fixed in Chrome a while back, many 3rd party apps that use Chromium / Electron (such as Microsoft Teams) still have this issue as they are using an older version of Chromium.
Description of how this pull request fixes the issue:
Several solutions were described in issue #7430. This PR goes with solution 3 (collapsing clickables).
Although it wasn't the most perfect solution, it was the easiest to implement, and probably the easiest to follow code-wise. It also hopefully provides a good balance between usability and predictability. It is certainly a major improvement over what we currently have, but also erring on the side of reporting more than less.
In short: in both speech and braille now, NVDA will never report multiple clickables in a row without any other intervening text. As NVDA enters one or more clickable fields, it will only report clickable once. However, if one or more of these fields are exited, and then 1 or more new clickable fields are entered, NVDA will again report clickable, but only once when entering those new fields. To put it another way, NVDA will only report clickable once for any run of field starts.
Testing performed:
Tested example in #7430. Multiple clickables in a row are now only announced as one clickable.
Also tested
clickables.html.txt
which contains various different paragraphs and divs containing single and nested clickable spans.
Known issues with pull request:
None known.
Change log entry:
Bug fixes: