Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aria-sort changes not announced when the change is not on the focused element #10890

Open
smhigley opened this issue Mar 20, 2020 · 13 comments · Fixed by #14234
Open

aria-sort changes not announced when the change is not on the focused element #10890

smhigley opened this issue Mar 20, 2020 · 13 comments · Fixed by #14234
Labels
app/chrome ARIA enhancement p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.

Comments

@smhigley
Copy link

smhigley commented Mar 20, 2020

Steps to reproduce:

  1. Visit https://w3c.github.io/aria-practices/examples/grid/dataGrids.html
  2. Navigate to the grid in the second example
  3. Navigate to the first column header (the sort state is announced)
  4. Change the sort state

Actual behavior:

Changes to the aria-sort property are not announced

Expected behavior:

Changes to aria-sort are announced

System configuration

NVDA installed

NVDA version: 2019.3.1

Windows version: Windows 10, version 1909

Name and version of other software in use when reproducing the issue:

  • Firefox 74.0
  • Chrome 80.0.3987.149
  • Edge 82.0.458.0
@Adriani90
Copy link
Collaborator

at least for Firefox cc: @jcsteh is this correctly exposed by Firefox?

Maybe @aleventhal could also comment on Cromium and how is it exposed there.

@jcsteh
Copy link
Contributor

jcsteh commented Apr 19, 2020

Firefox exposes this correctly and fires the correct events. NVDA even handles these events.

The issue here is that aria-sort is set on the cell, but the element the user actually activates is a button inside the cell. NVDA only reports changes that occur on the focused element, and since the button is the focused element and aria-sort isn't set on the button, no change is reported. Contrast this with aria-sort being set on the focused element activated by the user:

data:text/html,<button aria-sort="ascending" onclick="this.setAttribute('aria-sort', 'descending');">Test</button>

I can understand why it's been done this way. The question is whether it's reasonable to expect NVDA to handle this case, and if so, how NVDA should reasonably handle this. Reporting changes to any element would be bad. Reporting changes to any element in the focus ancestry might do it. Better still, NVDA might want to report changes to any element in the ancestry of the object at the cursor.

@TomAuger
Copy link

Question then: does putting the aria-sort attribute on the button address the use case of helping the AT understand that the column that button is present in is sorted in a particular way? Or does the aria-sort attribute NEED to be on the TH in order for it to have any useful meaning?

Put another way, can we simply start putting aria-sort on our inner buttons (inside the TH) as in your example above, or will that not work in actual practice?

@TomAuger
Copy link

TomAuger commented Apr 22, 2020

As an experiment, I removed the inner element altogether, and instead, put the "onclick" handler on the TH element itself, and then tested using the keyboard. While I was able to change the aria-sort attribute programmatically on the very element that had focus, the change was nonetheless not announced, so I wonder whether the above statement is, in fact, true.

Looks like the current workaround of using a separate aria-live element to announce changes to sorting is still the way to go here for now?

Tested in Google Chrome (latest), Windows 10.

@jcsteh
Copy link
Contributor

jcsteh commented Apr 22, 2020

As an experiment, I removed the inner element altogether, and instead, put the "onclick" handler on the TH element itself

That's what I would have suggested. However, you should also make the th focusable (tabindex) if you haven't already. I don't really see the need for the inner button.

While I was able to change the aria-sort attribute programmatically on the very element that had focus, the change was nonetheless not announced, so I wonder whether the above statement is, in fact, true.

The above statement was specific to Firefox. I tested in Chrome and it is indeed broken there. The problem is that Chrome isn't firing the correct events.

@jcsteh
Copy link
Contributor

jcsteh commented Apr 22, 2020

Question then: does putting the aria-sort attribute on the button address the use case of helping the AT understand that the column that button is present in is sorted in a particular way? Or does the aria-sort attribute NEED to be on the TH in order for it to have any useful meaning?

Putting aria-sort on a button in actual practice is a violation of the ARIA spec. My apologies for using that simplified example. It needs to be on the th.

@jcsteh jcsteh changed the title aria-sort changes not announced in Firefox, Chrome, or Edge aria-sort changes not announced when the change is not on the focused element Apr 22, 2020
@jcsteh
Copy link
Contributor

jcsteh commented Apr 22, 2020

The Chrome specific issue is covered in #10774.

@smhigley
Copy link
Author

@jcsteh a more realistic example of why a grid might control sorting with a button within the cell rather than with the cell itself is when a header cell might have multiple actions that can be taken, e.g. sorting, filtering, selection, etc. Sometimes there are just multiple buttons within the cell, and sometimes there's a single actions menu, where the sort functionality is controlled by a menuitem.

Both of those implementations are allowed by the ARIA spec, but as you mentioned the actual aria-sort value would need to be on the gridcell, even if focus is within it. Do you think supporting that pattern is something you'd consider for NVDA?

@jcsteh
Copy link
Contributor

jcsteh commented Apr 27, 2020 via email

michaelDCurran added a commit that referenced this issue Oct 26, 2022
…ton (#14234)

Fixes #10890

Summary of the issue:
A recommended UI pattern for sortable columns in HTML tables is to embed a button inside each table column header which when pressed sorts the rows by that column and updates aria-sort on the column header.
Although it is possible to have the sorting state of the column reported manually in NVDA, NVDA does not automatically announce the sorting state when it changes.
this is because NVDA currently only announces state changes for the focused object, which in this case is the bembedded button. Where as the column header is an ancestor of the focus.

Description of user facing changes
NVDA will announce the sort state as it changes.
More broadly, NVDA will now announce any state change on a focus ancestor, not just the focus itself. For example, this could be announcing the expand collapse state on a parent when an inner focused button is pressed.

Description of development approach
Base NVDAObject's event_stateChange method: announce the new state not only if this object is the focused object, but also if this object is one of the focus ancestors.
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Oct 26, 2022
@seanbudd seanbudd reopened this Feb 26, 2023
@seanbudd
Copy link
Member

Our system test for this is failing, it seems that this issue is a problem again in Chrome.

@seanbudd seanbudd added p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation. labels Feb 26, 2023
@michaelDCurran
Copy link
Member

It is still working in Firefox. But it looks as though perhaps Chrome is no longer firing a stateChange, or the stateChange is too early, when toggled.

seanbudd added a commit that referenced this issue Feb 26, 2023
Reopens #10890

Summary of the issue:
The system test for #10890 is failing.
It appears the failure is legitimate, hence the issue has been reopened
@seanbudd seanbudd removed this from the 2023.1 milestone Feb 26, 2023
@yhy-1
Copy link

yhy-1 commented Jun 12, 2023

Is there any new update on this? Still experiencing this in Chrome.

@Adriani90
Copy link
Collaborator

@yhy-1, @smhigley, @TomAuger did anyone report this to Chromium bug tracker?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app/chrome ARIA enhancement p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants