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

Tabs should use 'tab' role for better screen reader support #93789

Closed
joanmarie opened this issue Mar 30, 2020 · 10 comments
Closed

Tabs should use 'tab' role for better screen reader support #93789

joanmarie opened this issue Mar 30, 2020 · 10 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues debt Code quality issues workbench-tabs VS Code editor tab issues
Milestone

Comments

@joanmarie
Copy link

Steps to reproduce (in Linux):

  1. Launch the attached pyatspi accessible-event listener page-tab-events.py.txt in a terminal (remove the ".txt" extension)
  2. Launch VSCode, open a couple of files, and use Atl+1 and Alt+2 to switch between them

Expected results: The listener would print out an accessibility event each time the active page changes.

Actual results: (crickets)

Compare with: Any application which supports multiple page tabs and has accessibility enabled (e.g. Gedit, Firefox, Chromium, Terminator, etc.)

Impact: Orca does not announce the newly-selected/active page when Alt+n (where n is the pagetab index) is used in VSCode

Notes:

  1. Orca does get other events from VSCode but the ordering of events is such that Orca uses them to update its state rather than to announce the new location. While I (Orca maintainer) could potentially modify Orca to work around the ordering its getting from VSCode, I believe that if VSCode started emitting page-tab switch/selection changes (like all the other native apps do), Orca would automatically start doing the right thing.
  2. I believe that VSCode could accomplish what I am asking by setting aria-selected on the tab instances in the tablist, which brings me to:
  3. From the inspector, I have yet to find anything with the ARIA tab role on it. As you'll see in ARIA Practices and ARIA spec, tablist elements contain tab children. (And those tab children support aria-selected). If those elements are there and I'm just failing to see them, my apologies. But if they are indeed missing:
  4. From the inspector, it rather looks like the things which should have the ARIA tab role instead have a role of presentation but an aria-label whose value ends with ", tab". I believe the global aria-label causes the presentation role to be ignored by user agent implementations. Thus the accessibility tree winds up with generic-roled elements which have a name which includes the role name. Assuming these elements are the ones which should have the ARIA tab role, they should lose the ", tab" in the aria-label value. Otherwise once the expected events are emitted, screen readers will start presenting the newly-selected tab like "foo, tab. tab" or "foo, tab. page tab."

@isidorn for input. Thanks!

@isidorn isidorn added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label Mar 31, 2020
@isidorn isidorn added this to the April 2020 milestone Mar 31, 2020
@isidorn
Copy link
Contributor

isidorn commented Mar 31, 2020

@joanmarie thanks for filling this issue.
I have juste checked and yes we use the role tablist for the tab containers here

However we do not use the role tab here
The comment says "cannot use role "tab" here due to #8659"
So it seems that if we use the correct role tab screen readers do not read that there is a close button.

@joanmarie This week we are wrapping up our milestone, and I would love to change this start of next milestone to get feedback from insiders in case we break something. In case you want to try it out right now, just change the role on the second code pointer. Also let us know if you have tips with regards to the close button - how to make sure it is being read even if we use the 'tab' role.

And yes we should update the aria-label to not contain the word "tab" if we fix the role.

@isidorn isidorn added the debt Code quality issues label Mar 31, 2020
@isidorn isidorn changed the title Orca doesn't present page tab switches due to missing accessibility events from VSCode Tabs should use 'tab' role for better screen reader support Mar 31, 2020
@bpasero bpasero added the workbench-tabs VS Code editor tab issues label Mar 31, 2020
@joanmarie
Copy link
Author

@isidorn: Devil's advocate question for you: Is it really needed -- or even desired -- for screen readers to read that there is a close button there?

Many page tabs in native apps include a close button, the presence of which is not announced by screen readers (AFAIK).

These close buttons are typically not focusable. And even if it were, navigating up to the page tab via keyboard would be tedious. So screen readers announcing the existence of a tedious-to-access widget strikes me as unwanted chattiness. Plus, Ctrl+W is typically assigned to perform the close operation in all of these apps. And most screen reader users know this.

All of that said, if you want to add it, perhaps you could add information to the accessible description of the tab. Orca -- and I believe other screen readers -- tack on the accessible description at the end of the widget presentation and make description presentation optional. Thus it is easy to interrupt or disable for users who find it too chatty.

Accessible descriptions can be added via aria-describedby (a property that's been around forever). And also by the super-new aria-description which is now supported by Chromium, but perhaps not yet in the version being used by VSCode(??). It landed here.

@joanmarie
Copy link
Author

@isidorn

In case you want to try it out right now, just change the role on the second code pointer.

That was super helpful. Thanks!!

Making that change causes the missing events to be emitted. I will need to make one change in Orca to cause that to be presented which I'll do shortly -- after investigating some chattiness that's resulting from Orca now presenting the page tab changes.

@joanmarie
Copy link
Author

@isidorn: Here is a screen shot from Accerciser (Linux accessibility inspector). It shows what is exposed when I changed the role of the page tab from presentation to tab but left everything else the same.

page-tab

In particular it shows:

  1. What we already discussed and agreed upon: ", tab" is tacked on to the end accessible name ("foo") but should be removed.
  2. New: The accessible description is what is displayed for the page tab. "foo • Untitled-1"

As I mentioned above:

Orca -- and I believe other screen readers -- tack on the accessible description at the end of the widget presentation and make description presentation optional.

By default, Orca presents the description. Therefore, what Orca master will say for the object in the screen shot is:

  'foo, tab'
  'page tab foo • Untitled-1'

So I think what would be desirable to do as part of the fix here is:

  1. Change the accessible name so that it matches the displayed text on the tab, e.g. "foo • Untitled-1"
  2. EITHER get rid of the accessible description entirely OR replace it with some very brief information about the presence of the close button -- assuming screen reader users actually want to hear that for every single page tab.

Make sense?

@isidorn
Copy link
Contributor

isidorn commented Apr 1, 2020

@joanmarie all makes sense. Thanks for trying it out!

  1. We can align the ariaLabel with the title, I think that makes sense.
  2. I agree with you that the presence of the close button does not really matter since most users will just use ctrl + w, and it is not accessible. Thus for now we will get rid of the accessible descriptoin and wait for user feedback

Will do the changes next week. Thanks!

@isidorn isidorn closed this as completed in 29b5338 Apr 9, 2020
@isidorn
Copy link
Contributor

isidorn commented Apr 9, 2020

@joanmarie I have pushed a fix for this as we discussed, try it out and let me know how it goes.

@joanmarie
Copy link
Author

@isidorn Just gave it a spin. Works as expected. Thanks!

There's still a bit of chattiness that I'd like to eliminate. Now when the page-tab is switched, Orca says:

  • 'foo • Untitled-1' (this is the page tab name)
  • 'page tab'

Then, due to a caret-moved event for the newly-focused editor, Orca immediately says:

  • 'Untitled-1 editor' (this is the editor name)
  • 'entry'
  • 'bar' (this is the line of text where the caret moved)

I was thinking that I could filter out the double-speaking of the editor name -- doing a sanity check to be sure the editor name matches the page tab name. But as you can see from the above, it doesn't for a couple of reasons:

  1. You're including what functions as a role in the accessible name. There's a better way to accomplish that. See Use aria-roledescription on (at least) the editor to minimize some chattiness #94828.
  2. The page tab seems to be getting its name from the first line I put in the file, but that's not included in the editor name. I could presumably work around this in Orca because at least the editor name is a subset of the page tab name. Or we could do something to make the page tab name and editor name match.

Thoughts?

@isidorn
Copy link
Contributor

isidorn commented Apr 10, 2020

Thanks for trying it out. For 1. let's continue the discussion in #94828
For 2. can you clarify what you mean by "page tab" and "editor name". I assume by editor name you mean the aria-label of the HTML element with the 'tab' role. And yes we can change the aria-label any way that is needed.

@joanmarie
Copy link
Author

@isidorn When I'm working on Orca, I tend to think in terms of my platform's accessibility APIs rather than in terms of ARIA. Admittedly not especially helpful to you. Sorry about that!

To clarify:

  • "page tab": The div element with ARIA role of tab and, in my example, an aria-label value of foo • Untitled-1
  • "editor": The textarea element with a class of inputarea and, in my example, an aria-label value of Untitled-1 editor. Once Use aria-roledescription on (at least) the editor to minimize some chattiness #94828 is fixed, the aria-label value would presumably be Untitled-1
  • "name" (my platform's accessibility API property where aria-label values get exposed)

What I'm talking about in item 2 from my previous comment is: Should we make the tab-roled element and the textarea element have the same aria-label value?

Related aside: Independent of accessibility, why does thetab get the first 40 characters of the first line added to its displayed text when the document hasn't been saved? See this screenshot:

long-tab-name

If that's really desired, perhaps those 40 characters should continue to be part of the aria-label value of the tab-roled div. Though we should probably get some input from screen reader users.

The reason I think it should continue to be part of the aria-label value is that if a sighted peer were to say something like "switch to the tab which says 'foo bar baz'," there should probably be an accessible tab with an aria-label which says "foo bar baz" so the screen reader user knows what the sighted peer is talking about.

The reason I question the above is because it's chatty. And as a sighted user, seeing the first line of text displayed in the tab doesn't really buy me much. 🤷‍♀️

Anyhoo given the current up-to-40-character tab prefix, I think what I will do is have Orca check to see if a newly-focused textarea has an accessible name which is fully contained in the just-selected tab. And if the answer is "yes", not repeat it. Then, when #94828 is addressed, Orca should be less chatty.

Make sense?

@isidorn
Copy link
Contributor

isidorn commented Apr 10, 2020

@joanmarie thanks for claryfing. Yeah agree the aria-label of the textarea and the aria-label fo the tab should be the same. I think this should be captured by your issue #94828 which I plan to look into
As for the 40 character limit that is only for Untitled editors and @bpasero introduced this becuase he was inspired by Sublime, and users actually like this. I think @bpasero and me decided that it should not be part of tha aria-label to not make it too chatty. Though we can of course change that.

Before you do any workarounds for Orca let's look into #94828
Though the heursitic which you propose makes sense overall.

@bpasero bpasero removed their assignment Apr 11, 2020
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues debt Code quality issues workbench-tabs VS Code editor tab issues
Projects
None yet
Development

No branches or pull requests

3 participants