report busy indicator by feerrenrut · Pull Request #13428 · nvaccess/nvda

feerrenrut · 2022-03-04T08:58:34Z

Link to issue number:

Summary of the issue:

On the web, widgets with role="progressbar" and no aria-valuenow are reported by NVDA as "progress bar half checked zero".
These widgets are exposed by the browser with:

IAccessible accRole ROLE_SYSTEM_PROGRESSBAR
IAccessible accState STATE_SYSTEM_MIXED, STATE_SYSTEM_INDETERMINATE.

In chrome the value is always "0" in this case.
Note, in firefox, if aria-valuetext is provided, STATE_SYSTEM_INDETERMINATE is not set and accValue has the aria-valuetext.

Test cases on code pen editor, full

Description of how this pull request fixes the issue:

An 'indeterminate' progress bar has no value attribute, it can not convey progress, only activity.
Rather than reporting as a progress bar it is given a new NVDA role of "busy indicator" and its value ("zero") is no longer reported.

Technical details.

Code to convert decode the bit flags (integer) supplied by IA / IA2 into a set of NVDA States was repeated, this has been extracted to a fucntion, centralized in IAccessibleHandler, and optimised.
Commits:
- Extract states calculations
- Optimise / simplify state calculations
The initial approach was to report "progress bar indeterminate", see commit: Report as 'progress bar indeterminate'

Value was exposed as the text node in the virtual buffer, in this case space is now used.

Testing strategy:

Test cases on code pen editor, full

Tested chrome and firefox, speech and braille
Focus mode (navigate with tab)
Browse mode (navigate with tab)
Browse mode (navigate with down arrow)

Known issues with pull request:

None

Change log entries:

New features
Changes

- Added new role for "busy indicator" controls. (#10644)

Code Review Checklist:

Pull Request description:
- description is up to date
- change log entries
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
API is compatible with existing add-ons.
Documentation:
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English

Rather than test all 32 bits, just check the values NVDA maps from. Reduces IA states calculation to 18 comparisons Reduces IA2 states calculation to 10 comparisons

A fallback option if 'busy indicator' isn't accepted.

Edge Focus mode: 'loading busy indicator zero' Browse mode: 'loading busy indicator zero` Firefox Focus mode: 'loading busy indicator 50" Browse mode: 'loading busy indicator 50"

CyrilleB79 · 2022-03-04T09:32:39Z

I am afraid this new role can create confusion. Is there a busy indicator object in one of the various interface specifications (IAccessible, UIA, etc.)
More generally, if I speak of a busy indicator, does someone (sighted or not) know what it is?

From a user point of view, busy indicator may let think to the unrelated following concepts:

The browser indicates "busy" when the page is loading
"busy" may also refer to a mouse pointer shape (e.g. hourglass, clock, etc.) indicating that the computer is busy.

And overall, we lose the information that the "busy indicator" is actually a progress bar. In case of collaboration with a sighted person, this creates communication difficulties.

I have not followed all the discussion but could the following solutions be implemented instead?
Give the progress bar with undetermined stat a state or value of "running", "working", etc.? I agree that it's not so easy to find a correct wording for it. But creating a new role instead seems more confusing to me.

Qchristensen

bsyind for Braille looks good.

seanbudd

Hi @feerrenrut,

I've added a couple of minor notes but otherwise LGTM.
Would you like a secondary review for the c++ code? It's a fairly straightforward change in my opinion.

Also, in case you missed it - a unit test needs fixing: https://ci.appveyor.com/project/NVAccess/nvda/builds/42786502/tests
This test exists to ensure each state/role has a display string, but I see that this is unnecessary for State.INDETERMINATE?

feerrenrut · 2022-03-08T06:24:26Z

@CyrilleB79 These kinds of progress bars do not convey progress visually and they can be presented in lots of different ways.

From a user point of view, busy indicator may let think to the unrelated following concepts:

The browser indicates "busy" when the page is loading
"busy" may also refer to a mouse pointer shape (e.g. hourglass, clock, etc.) indicating that the computer is busy.

These are related concepts. The main intent of an indeterminate progress bar is to reassure the user that something is happening, even though the amount of work / time remaining is unknown.

Give the progress bar with undetermined stat a state or value of "running", "working", etc.? I agree that it's not so easy to find a correct wording for it. But creating a new role instead seems more confusing to me.

The main argument against is an indeterminate progress bar widgets do not convey progress, they have no 'value' attribute, so they can't. Announcing "progress bar working" is longer than it needs to be, the role is misleading and the state is redundant. Essentially, an indeterminate progress bar is a mono-state control.

CyrilleB79 · 2022-03-08T07:28:31Z

@CyrilleB79 These kinds of progress bars do not convey progress visually and they can be presented in lots of different ways.

OK Thanks. Since you mention a great variety of presentations, the busy indicator makes sense.

My last remaining concern is the situation where a progress bar transitions from half-checked (undeterminate) state to (determinate) value = 100%. That is, when a task is in progress without any estimation of the advancement and then is completed.
I have no real situation where this occurs.

Can this situation occur? If yes, is it common?
In such case, would NVDA report the role change?

Should this situation be common, I think that the role change may be misleading from a user point of view.
If it is not (what is more likely), there is no need to change the design for such a corner case.

feerrenrut · 2022-03-08T11:29:53Z

Can this situation occur?

Yes, it could. This, would be author intent though, there may be valid reasons to want to do this.
I have updated the sample with this case.

If yes, is it common?

I doubt it. It's certainly an edge case.

In such case, would NVDA report the role change?

Currently, NVDA will report the final beep and "100%" (depending on the user's setting for presentation, progress bar output).
With my current test case approach, Chrome seems to swap the IAccessible object when changing from indeterminate to complete determinate. This is a real corner case (author wants to swap from indeterminate to determinate, user wants to monitor the widget for completion), that I suspect there are work arounds for. If it came to it, we could probably resolve the issues (perhaps requiring changes in chrome) but certainly it shouldn't block this initial implementation.

Use in: - calculateNvdaStates - calculateNvdaRole Also correct IA2States -> IAStates.

Add an exclusion for states and roles that don't have display strings These are checked to ensure they don't have labels.

seanbudd

Thanks @feerrenrut

…ssBar

feerrenrut added 8 commits March 4, 2022 16:15

Extract states calculations

cf3b1ed

Optimise / simplify state calculations

99f4879

Rather than test all 32 bits, just check the values NVDA maps from. Reduces IA states calculation to 18 comparisons Reduces IA2 states calculation to 10 comparisons

Report as 'progress bar indeterminate'

f25cfff

A fallback option if 'busy indicator' isn't accepted.

add typing info

2d532d9

extract NVDARoleFromAttr common pattern

9bb9b2f

Report busy indicator for indeterminate progress bar

6e664ee

Edge Focus mode: 'loading busy indicator zero' Browse mode: 'loading busy indicator zero` Firefox Focus mode: 'loading busy indicator 50" Browse mode: 'loading busy indicator 50"

Supress values for 'busy indicator' in focus and browse mode.

a62dac7

add braille abbreviation for role busy indicator

796f645

feerrenrut requested review from a team as code owners March 4, 2022 08:58

feerrenrut requested review from Qchristensen and seanbudd March 4, 2022 08:58

This comment was marked as outdated.

Sign in to view

Qchristensen approved these changes Mar 7, 2022

View reviewed changes

seanbudd reviewed Mar 8, 2022

View reviewed changes

Comment thread source/speech/speech.py

Comment thread source/IAccessibleHandler/__init__.py Outdated

Comment thread source/IAccessibleHandler/__init__.py Outdated

Comment thread source/IAccessibleHandler/__init__.py Outdated

This comment was marked as outdated.

Sign in to view

feerrenrut added 6 commits March 9, 2022 10:49

Fix docs strings

3e04691

Re-use transformRoleStates

a4b2f5d

Use in: - calculateNvdaStates - calculateNvdaRole Also correct IA2States -> IAStates.

More specific type requirements for NVDARoleFromAttr

f03b472

move special cases transformation to controlTypes

efd75b2

fix tests

1c97dba

Add an exclusion for states and roles that don't have display strings These are checked to ensure they don't have labels.

fix lint

2c46dce

This comment was marked as outdated.

Sign in to view

feerrenrut requested a review from seanbudd March 10, 2022 09:01

lukaszgo1 reviewed Mar 10, 2022

View reviewed changes

Comment thread source/IAccessibleHandler/__init__.py Outdated

type checking import with other imports

47bd239

seanbudd approved these changes Mar 10, 2022

View reviewed changes

feerrenrut added 3 commits March 11, 2022 12:18

Merge remote-tracking branch 'origin/master' into indeterminateProgre…

350c682

…ssBar

update changes file for pr #13428

d3984f0

include nagativeDisplayString tests

202a8cd

feerrenrut merged commit 4df1ded into master Mar 11, 2022

feerrenrut deleted the indeterminateProgressBar branch March 11, 2022 08:45

nvaccessAuto added this to the 2022.2 milestone Mar 11, 2022

CyrilleB79 mentioned this pull request Jul 16, 2022

NVDA announces indeterminate progressbar as half checked #13910

Closed

feerrenrut mentioned this pull request Aug 4, 2022

UIA: Support half checked state for checkboxes and toggle buttons #13980

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

report busy indicator#13428

report busy indicator#13428
feerrenrut merged 18 commits into
masterfrom
indeterminateProgressBar

feerrenrut commented Mar 4, 2022 •

edited

Loading

Uh oh!

This comment was marked as outdated.

CyrilleB79 commented Mar 4, 2022

Uh oh!

Qchristensen left a comment

Uh oh!

seanbudd left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feerrenrut commented Mar 8, 2022

Uh oh!

CyrilleB79 commented Mar 8, 2022

Uh oh!

feerrenrut commented Mar 8, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

seanbudd left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

feerrenrut commented Mar 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

Uh oh!

This comment was marked as outdated.

CyrilleB79 commented Mar 4, 2022

Uh oh!

Qchristensen left a comment

Choose a reason for hiding this comment

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feerrenrut commented Mar 8, 2022

Uh oh!

CyrilleB79 commented Mar 8, 2022

Uh oh!

feerrenrut commented Mar 8, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

feerrenrut commented Mar 4, 2022 •

edited

Loading