search

signalapp / Signal-Desktop

A private messenger for Windows, macOS, and Linux.
https://signal.org/download
GNU Affero General Public License v3.0
14.51k stars 2.63k forks source link

Accessibility: Items in React Virtualized Lists not accessible on Mac #5296

Open MarcoZehe opened 3 years ago

MarcoZehe commented 3 years ago

Bug Description

Items in both the list of conversations as well as the individual items in a conversation are not accessible when using the VoiceOver navigation commands like CTRL+Option+Right/Left Arrows. Tabbing and simple arrow key navigation does speak the contents, but you cannot explore the items further, or get anything displayed in Braille.

The reason is https://github.com/bvaughn/react-virtualized/issues/1657, where the simplified react-virtualized list component inherits the ARIA "grid" role, but does not also provide the necessary/required "row" children and "grid cell" grandchildren. In Signal's case, it is a mix of labels and buttons that are children of Grid. VoiceOver is a real diva when it comes to incorrect table and grid hierarchies, and simply will not render the contents at all.

Steps to Reproduce

  1. Open Signal Desktop.
  2. Turn on VoiceOver with CMD+F5, or CMD+Triple-clicking the Power button.
  3. Navigate the UI with CTRL+Option+Right/LeftArrows.

Actual Result:

When VoiceOver says "Grid with 0 rows and 0 columns", trying to go one step further doesn't focus the individual items. Also, interacting (CTRL+Option+Shift+DownArrow) does not work.

Expected Result:

The items should be navigable using normal VoiceOver commands.

Screenshots

N/A. Happens with any conversation, no need to post my private stuff here.

Platform Info

Signal Version: v5.2.1

Operating System: MacOS 11.4.

Linked Device Version: iOS 14.6.

Link to Debug Log

N/A, this is an error in the React markup that results in inaccessible content, not a crash or otherwise luggable failure.

Proposed solution

Every <list> component used in Signal Desktop should get a role property of "region" instead of inheriting the "grid" role from the React-virtualized Grid parent component.

I am proposing "region" because the components inside are so mixed that something specific like "listbox" wouldn't be appropriate, since that would require children that have a role of "option", which, in turn, don't handle button children well. So, a generic "region" role is the best solution for these.

Unfortunately, I cannot provide a qualified pull request myself because my M1 Mac fails to meet the very specific version requirements for Node and other components and won't build Signal Desktop. But this should be an easy enough fix for someone experienced with Signal development.

I am happy to provide a review on an associated PR.

MarcoZehe commented 3 years ago

Separate note for Windows and Linux: This will improve the situation there, too, but the consequences of this invalid markup that is in there now, aren't as drastic as they are on Mac with VoiceOver. So the proposed solution will work on all supported desktop platforms.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

rugk commented 2 years ago

shh #badbot :no_bell::robot::no_bell:

indutny-signal commented 2 years ago

Assigned it to me to keep the bot away. Sorry!

josh-signal commented 2 years ago

Thanks for flagging this, we've got a fix incoming in the next beta family release.

LeonarddeR commented 2 years ago

Thanks a lot. Happy to test it.

josh-signal commented 1 year ago

Looks like this is still broken :(

We've got better keyboard navigation on Signal now with tab and the arrow keys but VO is still being a diva. The proposed fix of using region isn't working. @MarcoZehe feel free to provide a patch file or point towards the line in the code you feel like should be changed and I can give that a try.