nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
https://www.nvaccess.org/
Other
2.12k stars 637 forks source link

Selected state isn't reported on treeitems #14352

Open msftedad opened 2 years ago

msftedad commented 2 years ago

Steps to reproduce:

code pen TestSitemap.zip

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 commented 2 years ago

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 commented 2 years ago

More information on an older duplicate: #8587

prabhudevu commented 1 year ago

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 commented 1 year ago

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

msftedad commented 1 year ago

We are following with product team and will response ASAP.

prabhudevu commented 1 year ago

@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 commented 1 year ago

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

prabhudevu commented 1 year ago

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

dpaquette commented 1 year ago

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

fsteffi commented 1 year ago

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 commented 1 year ago

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

Adriani90 commented 1 year ago

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 commented 6 months ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

@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 commented 1 week ago

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

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 commented 1 week ago

@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 commented 1 week ago

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 commented 1 week ago
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 commented 1 week ago

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.

aleventhal commented 4 days ago

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 commented 17 hours ago

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