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

Selected state isn't reported on treeitems #14352

Open
msftedad opened this issue Nov 8, 2022 · 23 comments
Open

Selected state isn't reported on treeitems #14352

msftedad opened this issue Nov 8, 2022 · 23 comments
Labels
app/edge/anaheim MS browser, chromium based, replaces Spartan in 2019 by Anaheim. NVDA access via IA2. app/firefox ARIA needs-technical-investigation A technical investigation is required to progress the issue. p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.

Comments

@msftedad
Copy link

msftedad commented Nov 8, 2022

Steps to reproduce:

code pen
TestSitemap.zip

  • Use the code pen or the Html sample.
  • Tab till left Navigation panel.
  • Now Observe if the Narrator announces the selected tab and differentiates the selected tab from other tab items.

Actual behavior:

NVDA is not announcing the state for tabs present under left side navigation panel.

Expected behavior:

NVDA should announcing the state for tabs present under left side navigation panel.

NVDA logs, crash dumps and other attachments:

System configuration

NVDA installed/portable/running from source:

installed

NVDA version:

2022.3

Windows version:

Version 21H2(OS Build 22000.1098)

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

Microsoft Edge, Version 107.0.1418.35 (Official build) (64-bit)

Other information about your system:

Other questions

Does the issue still occur after restarting your computer?

yes

Have you tried any other versions of NVDA? If so, please report their behaviors.

no

If NVDA add-ons are disabled, is your problem still occurring?

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

@ehollig
Copy link
Collaborator

ehollig commented Nov 9, 2022

Confirmed. NVDA is not communicating state of treeitem role using aria-selected. reference: treeitem role. NVDA does appropriately communicate state if using aria-checked but aria-selected is used for single selects in a tree role

@seanbudd seanbudd added needs-technical-investigation A technical investigation is required to progress the issue. app/edge/anaheim MS browser, chromium based, replaces Spartan in 2019 by Anaheim. NVDA access via IA2. triaged Has been triaged, issue is waiting for implementation. labels Nov 15, 2022
@seanbudd
Copy link
Member

More information on an older duplicate: #8587

@feerrenrut feerrenrut changed the title [Accessibility]NVDA is not announcing the state for tabs present under left side navigation panel. Selected state isn't reported on treeitems Nov 15, 2022
@prabhudevu
Copy link

prabhudevu commented Jul 7, 2023

I have verified the issue in the latest dynamic environment

App name: Customer Service Hub
Organization name: AuroraBAPEnv29074
Server version: 9.2.23073.00002
Client version: 1.4.5968-master

Issue is fixed in primary bug.

14352_Fixed

@seanbudd
Copy link
Member

seanbudd commented Jul 9, 2023

@prabhudevu - this screenshot seems unrelated to the issue. can you please explain more?

@msftedad
Copy link
Author

We are following with product team and will response ASAP.

@prabhudevu
Copy link

@prabhudevu - this screenshot seems unrelated to the issue. can you please explain more?

Hi @seanbudd Sorry, Actually this bug is an external bug to https://dynamicscrm.visualstudio.com/OneCRM/_workitems/edit/3159230/

Issue is fixed in primary Bug and Environment.

@seanbudd
Copy link
Member

Are you sure we should close this? Our understanding was this was a legitimate bug, as per the duplicate #8587

@prabhudevu
Copy link

Please don't close the bug if it is not fixed in the Environment which you used while logging the bug

@dpaquette
Copy link

This issue also exists for role="treegrid". Repro here:
https://codepen.io/dpaquette/pen/WNLoEwp

@fsteffi
Copy link

fsteffi commented Sep 7, 2023

hi @seanbudd sorry for the confusion this bug is external to a primary bug that still repro on azure devops
he code pen is https://codepen.io/dpaquette/pen/WNLoEwp

@seanbudd
Copy link
Member

seanbudd commented Sep 8, 2023

Hi I'm still confused here @fsteffi

Do you believe this is an issue with NVDA?
Could you give some reasoning?

As mentioned earlier, this appears to be related/duplicate of another valid issue with NVDA

@seanbudd seanbudd added the p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority label Sep 11, 2023
@Adriani90
Copy link
Collaborator

Related or might be duplicate of #11986. It seems NVDA does not honor aria_selected.
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-selected

cc: @michaelDCurran

@SaschaCowley
Copy link
Member

This is because of bugs in Firefox and Chromium: Firefox and Chromium are assuming the focus is also the selection, even though the spec says "If any DOM element in the widget is explicitly marked as selected, the user agent MUST NOT convey implicit selection for the widget."

@Adriani90
Copy link
Collaborator

Maybe @jcsteh and @aleventhal can give some more insights on how this is handled in Firefox and Chromium.
Should we create bug reports with both browsers? Or is there anything NVDA can do here to fix this issue?

@SaschaCowley
Copy link
Member

In theory, we could walk the accessibility tree, querying the aria attributes to find if aria-multipleselect is true, and if not, only honour the current focus as selected if no treeitem has aria-selected=true. But this will likely be slow and error-prone, and it's not supposed to be our responsibility anyway.

@aleventhal
Copy link

@SaschaCowley Can you summarize what you think Chrome is doing wrong and how you would like things to work? We can look into making change but I want to make sure I understand how you want single selects to work vs multi selects.

@Adriani90
Copy link
Collaborator

After reading this completely, I try to clarify where the problem exactly is:

  • When having a set of tabs, like e.g. this example, https://www.w3.org/WAI/ARIA/apg/patterns/tabs/examples/tabs-automatic/
    the danish composers tabs are all rendered in browse mode, and the tab which is selected is reported as "selected". Unselected tabs are reported only with their labels.
  • When using browse mode to navigate and pressing enter on a tab which is not selected, the state changes to "selected" and NVDA reports that tab item as "selected".
  • This behavior is also expected for the tree view and its items, both for single and multiselect.

Currently, when using object navigation NVDA reports "not selected" on the tree item "activities". But in browse mode, NVDA does not distinguish between selected and not selected attributes when having tree views.

@SaschaCowley why does this work properly for tabs but not for trees?
By the way, it is better to test with this codepen because it has propper javascript implementation:
https://codepen.io/Ardena/pen/oMdMgq

@SaschaCowley
Copy link
Member

@aleventhal, my expectation would be:

  1. In a single-selection container (I.E. aria-multiselectable=false or unset):
    1. If no children have aria-selected set, selection is implicitly assumed to be the current focus; or
    2. If any child has aria-selected, the explicit assignment overrides the implicit assignment, and only children with aria-selected=true are considered selected.
  2. In a multiple-selection container (I.E. aria-multiselectable=true):
    1. Selection must be communicated explicitly. Only items with aria-selected=true are considered selected, regardless of focus.
    2. Optionally, only children with aria-selected set are considered selectable. This seems to be what the ARIA 1.2 spec mandates, though I don't know if it is how most widgets in the wild are constructed.

From memory, both Firefox and Chrome were always communicating that focused descendents of selection containers were selected, unless aria-selected=false was specified, even if another descendent of the same container had aria-selected=true.

@aleventhal
Copy link

Thanks Sascha, that's very helpful. I created a Google Doc where we can make sure we get all the details right before we implement. I added some Q's for you in the comments. For anyone else interested, here's the link: https://docs.google.com/document/d/1qDQfjDiqCgPiCJccIbOufncYhhBDiNW6XBLSc19jNMo/edit?tab=t.0

@jcsteh
Copy link
Contributor

jcsteh commented Nov 12, 2024

1. In a single-selection container (I.E. `aria-multiselectable=false` or unset):
   
   1. If no children have `aria-selected` set, selection is implicitly assumed to be the current focus; or
   2. If any child has `aria-selected`, the explicit assignment overrides the implicit assignment, and only children with `aria-selected=true` are considered selected.

😩 This is one of those cases where I think the spec was written without considering implementation feasibility. Even on the browser side, this is going to have a significant performance cost in a sufficiently large container, regardless of whether aria-selected is specified on any item or not. Sure, we can try to cache container selection, but then we have to keep that cache up to date whenever items get added or removed or whenever the selection of any item changes, keeping in mind that a tree can have nested treeitems. Even so, that's what the spec has required for years and we've just never implemented it, so i guess it is what it is.

2. In a multiple-selection container (I.E. `aria-multiselectable=true`):
   
   1. Selection must be communicated explicitly. Only items with `aria-selected=true` are considered selected, regardless of focus.

This is actually what both Firefox and Chrome already do:

data:text/html,<div role="tree" aria-multiselectable="true"><div role="treeitem" tabindex="0">blah

If you focus the tree item, observe that it does not get the selected state. Interestingly, I don't see any support for this in the spec. Do either of you?

   2. Optionally, only children with `aria-selected` set are considered selectable. This seems to be what the ARIA 1.2 spec mandates, though I don't know if it is how most widgets in the wild are constructed.

I agree this is what the spec says:

undefined (default): The element is not selectable.

I'd argue this contradicts the rule for single selection containers though. As above, if aria-selected isn't specified in a single selection container, we're supposed to just assume that means it isn't selected, not that it isn't selectable. aria-selected undefined says nothing about being relevant to multiple selection containers only.

I think we'll need to get that fixed in the spec.

From memory, both Firefox and Chrome were always communicating that focused descendents of selection containers were selected, unless aria-selected=false was specified, even if another descendent of the same container had aria-selected=true.

You're correct.

@aleventhal
Copy link

Roles is the spec that are marked as having "aria-selected (required)" in their supported attribute table automatically get a default value of false for aria-selected if the attribute isn't present. I agree this is unclear. And it should only happen when the container is multiselectable.

aarongable pushed a commit to chromium/chromium that referenced this issue Nov 15, 2024
Implicit selection is where currently focused descendant or active
descendant is treated as selected, in terms of the state and
selection events. It is supported only in single selection containers.

Changes to the ARIA spec have affected when it is supported:
* tree/listbox: only support if none of the treeitems/options have an
  explicit aria-selected/aria-checked
* tabpanel: only support if none of the tabs have an explicit
  aria-selected/aria-expanded
* grid/treegrid: not supported (same as before)

Tree updates: If aria-multiselectable changes on the container, or
aria-selected/aria-checked changes on an option/treeitem, or
aria-expanded/aria-selected changes on a tab, then mark their container
as not supporting implicit selection, and mark everything dirty where
the implicit selected state may have changed. Note: this is tricky in
the tab case because when focus is inside a tabpanel controlled by a
tab, implicit selection is on the tab, which has a somewhat indirect
relation with the focus in the tree.

Fixed: 1143451
Bug: nvaccess/nvda#14352
Change-Id: I117cb6210b1c35e976f3962cf17ea39ace7901e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6021043
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Jocelyn Tran <jocelyntran@google.com>
Cr-Commit-Position: refs/heads/main@{#1383813}
@aleventhal
Copy link

Ok, the single selection fixes should be in the next build of Chrome Canary.
If the author uses aria-selected or aria-checked on an option, then the selection won't follow focus in that listbox.

@jcsteh
Copy link
Contributor

jcsteh commented Nov 19, 2024

Oh, looks like i filed this bug against Firefox 7 months ago and forgot about it:
https://bugzilla.mozilla.org/show_bug.cgi?id=1894437

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app/edge/anaheim MS browser, chromium based, replaces Spartan in 2019 by Anaheim. NVDA access via IA2. app/firefox ARIA needs-technical-investigation A technical investigation is required to progress the issue. 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

No branches or pull requests

10 participants