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

fix screen reader accessibility of rustdoc item documentation #87059

Closed
ahicks92 opened this issue Jul 11, 2021 · 26 comments
Closed

fix screen reader accessibility of rustdoc item documentation #87059

ahicks92 opened this issue Jul 11, 2021 · 26 comments
Assignees
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Milestone

Comments

@ahicks92
Copy link
Contributor

I opened this as one issue because the overarching problem is the problem, if you will. if I should have made a bunch, let me know.

Looks like the following applies to beta. Example page: https://doc.rust-lang.org/beta/std/collections/struct.BTreeMap.html

I'm a screen reader user (e.g. blind). Specifically NVDA. Whatever changes are coming down the pipeline with Rustdoc make the above-linked page difficult to navigate. It used to be the case that you could get used to the weird heading structure (specifically that Rustdoc headings are higher levels than example/error headings) then be mostly fine. But now:

  • A lot of what used to be headings are exposed as buttons.
  • All of those buttons like to start with [-], which screen readers are forced to listen to as "left bracket dash right bracket", e.g. "left bracket dash right bracket pub fn".
  • There's a table of contents, but it only shows up if the page is maximized, and if there's a button to expand it I can't quickly find it.
  • Said table of contents provides no way to quickly navigate it, e.g. "methods" being a heading, using lists, etc. I have to ctrl+f the categories.

I manage because I'm a screen reader power user who has previous familiarity with these pages, which meant that mostly getting around/through this was working out what actually changed rather than how to do it from scratch. It still hit my efficiency quite a bit. Most screen reader users aren't going to know to maximize their browser and ctrl+f "methods" and so on, or work out that actually sometimes you need next heading and sometimes you need next button.

Some concrete fixes:

  • Put the h tags back (maybe we can do this with aria, if the buttons are important).
  • move [-] to the end of the line, where it used to be. I think this can be faked with css if we need the visual representation to stay the same, but I'm a backend guy.
  • Change the "headings" in the table of contents to be actual h tags, perhaps with nested links, or make the table of contents nested lists like mdbook.
  • make sure that the table of contents is always visible, or that the button to expand/collapse it is more obvious if there is one.

I'm happy to discuss other fixes as well; my goal is to provide some sort of conversation starter.

@jonas-schievink jonas-schievink added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 11, 2021
@jyn514
Copy link
Member

jyn514 commented Jul 11, 2021

cc @jsha

@jyn514
Copy link
Member

jyn514 commented Jul 11, 2021

@ahicks92 when you say "coming down the pipeline" you specifically mean that these are changes in beta, that haven't made it to stable, right?

@jyn514
Copy link
Member

jyn514 commented Jul 11, 2021

Also, a broader question: your suggested changes seem fine to me, and I will try to make sure one or more of them land before the beta release, but how can we avoid regressions like this in the future? Do you know some software that can "rate" how accessible a page is, something like Google PageSpeed insights but for HTML layouts? Or are you maybe willing to donate a bit of your time every six weeks before a release? I can ask contributors to try their changes with a screen reader, but that seems like a burden to contributing to me; neither they nor I would really know what to look for.

@jyn514 jyn514 added the A-rustdoc-ui Area: Rustdoc UI (generated HTML) label Jul 11, 2021
@jyn514
Copy link
Member

jyn514 commented Jul 11, 2021

All of those buttons like to start with [-], which screen readers are forced to listen to as "left bracket dash right bracket", e.g. "left bracket dash right bracket pub fn".

Is there a way to tell screen readers not to read aloud certain buttons/divs? That seems like a better fix than just moving the buttons between the content.

@nagisa
Copy link
Member

nagisa commented Jul 11, 2021

Looking at the accessiblity panel in the Firefox Developer tools it seems like at least as far as [-] is concerned, the problem is that the [-] has become the "summary" of some of the <details> elements. Since it is not a summary, the default roles are struggling to make the content sensible. I believe finding an appropriate aria role here and applying it to all of the <details> elements is going to be ideal.


Is there a way to tell screen readers not to read aloud certain buttons/divs? That seems like a better fix than just moving the buttons between the content.

Using aria-hidden="true" on some of the <summary> elements seems to hide the <summary> away from the screen reader tree, but I'm not sure how good of a practice that is. It won't work on the <summary>-ies of the methods, though as these populate [ - ] via CSS's ::before.

move [-] to the end of the line, where it used to be. I think this can be faked with css if we need the visual representation to stay the same, but I'm a backend guy.

Alas, the CSS pseudo elements are still included into the screen reader output and removing it is not trivial. It doesn't seem like WAI-ARIA has (ahem…) a way to specifically target CSS pseudo elements either.

We need to re-factor the DOM somewhat significantly here to get things work out I suspect.


when you say "coming down the pipeline" you specifically mean that these are changes in beta, that haven't made it to stable, right?

From looking at the accessibility trees and given the descriptions it seems like that's the correct interpretation.


A lot of what used to be headings are exposed as buttons.

I believe this refers to the fact that what previously was a

<h4 id="method.clear" class="method">
    <code>pub fn <a href="#method.clear" class="fnname">clear</a>(&amp;mut self)</code><a href="#method.clear" class="anchor"></a><a class="srclink" href="../../src/alloc/collections/btree/map.rs.html#512" title="goto source code">[src]</a><a href="javascript:void(0)" class="collapse-toggle">[<span class="inner"></span>]</a>
</h4>

is now a

<summary><div id="method.clear" class="method has-srclink">
    <code>pub fn <a href="#method.clear" class="fnname">clear</a>(&amp;mut self)</code><a href="#method.clear" class="anchor"></a><a class="srclink" href="../../src/alloc/collections/btree/map.rs.html#512" title="goto source code">[src]</a>
</div></summary>

(i.e. the h4 was replaced with a div).


make sure that the table of contents is always visible, or that the button to expand/collapse it is more obvious if there is one.

I don't believe there is a way to hide the ToC (sidebar)…

EDIT: Ah, I see what you mean. It is hidden into a hamburger menu in mobile layout. And the button to display it is… not focus-able. There is attempt to make this hamburger act like a button:

<div class="sidebar-menu" role="button">☰</div>

but for some reason that doesn't work?

@nagisa
Copy link
Member

nagisa commented Jul 11, 2021

Do you know some software that can "rate" how accessible a page is, something like Google PageSpeed insights but for HTML layouts?

I mention the Firefox's accessibility developer tools panel in the post above. It can be used to intuit what a screen reader would read without having to set up a full accessibility stack. It also has some lints in there, though they are pretty anemic still, as well as an ability to display tab order in a nice way.

@jyn514 jyn514 added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Jul 11, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 11, 2021
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jul 12, 2021

  • move [-] to the end of the line, where it used to be. I think this can be faked with css if we need the visual representation to stay the same, but I'm a backend guy.

Maybe I'm missing something but it has always been on the left of the items.

I don't believe there is a way to hide the ToC (sidebar)…

EDIT: Ah, I see what you mean. It is hidden into a hamburger menu in mobile layout. And the button to display it is… not focus-able. There is attempt to make this hamburger act like a button:

<div class="sidebar-menu" role="button">☰</div>

but for some reason that doesn't work?

This button is only relevant on mobile or on devices with small width. There is no way to hide the sidebar otherwise.

Also, overall our heading strategy was a complete mess, which was simplified/fixed by using the <details> and <summary> tags. Adding aria labels might make the situation better I think?

  • make sure that the table of contents is always visible, or that the button to expand/collapse it is more obvious if there is one.

This button is only relevant for mobile devices where you can't have the sidebar alongside the content.

Anyway, we have a lot of progress to make on accessibility. I think integrated tools to ensure there is no big stepback in this regard would be a very nice improvement.

@nagisa
Copy link
Member

nagisa commented Jul 12, 2021

Maybe I'm missing something but it has always been on the left of the items.

Previously at the DOM level they came after the pub fn ..., and were repositioned to appear on the “left” via CSS. From an excerpt I posted above (formatted further)

<h4 id="method.clear" class="method">
  <code>pub fn 
    <a href="#method.clear" class="fnname">clear</a>
    (&amp;mut self)</code>
    <a href="#method.clear" class="anchor"></a>
    <a class="srclink" href="../../src/alloc/collections/btree/map.rs.html#512" title="goto source code">[src]</a>
    <a href="javascript:void(0)" class="collapse-toggle">[<span class="inner"></span>]</a>
    <!-- ^^^ HERE ^^^ -->
</h4>

This button is only relevant on mobile or on devices with small width. There is no way to hide the sidebar otherwise.

@ahicks92's complaint was specifically about the ToC only appearing when the browser window is maximized. Given that people using screenreaders don't usually need browsers rendering an entire screen worth of content, windows could get pretty small during their normal use.

@GuillaumeGomez
Copy link
Member

Previously at the DOM level they came after the pub fn ..., and were repositioned to appear on the “left” via CSS. From an excerpt I posted above (formatted further)

Oh I see! I'm way to used at thinking about display and not about DOM... Thanks for the clarification!

@ahicks92's complaint was specifically about the ToC only appearing when the browser window is maximized. Given that people using screenreaders don't usually need browsers rendering an entire screen worth of content, windows could get pretty small during their normal use.

Is there a way to check if the page is rendered on a screenreader? If so, we could simply ignore the width in such a case and put the sidebar anyway.

@ahicks92
Copy link
Contributor Author

Happy to see quick feedback. To answer questions in no particular order and hopefully without missing anything:

It does seem to be beta only. I've got an ongoing project in 1.53 which renders the old way, for lack of better phrasing.

For some reason my browser starts not fully maximized. When I maximize it I get the table of contents.

Screen readers like to read things in DOM order, so "end of the line" is indeed not really the best term for that. You can use CSS to make the visual order not match DOM and the screen readers still generally follow DOM order.

aria-hidden=true is generally fine, but I always caution people that there's no way for a screen reader user to know that it was applied without opening the web debugger, and if you put it on the wrong thing you will just make parts of the page vanish with no trace. That's not hard to avoid, but given that it's one of the more common "how do people misuse Aria in a way that I have to open the web debugger to fix it" bugs it's worth mentioning.

Fixing the table of contents probably "fixes" the order of elements on the line of the item itself, because the workflow here is/was using the h tags to find out what is available, and the point as far as I'm concerned is fixing the workflow. For myself, I'm satisfied as long as there's a quick way to skim method names.

To the best of my knowledge, there isn't any good software for detecting this sort of accessibility issue, because this sort of accessibility issue is kind of subjective. You'd have to try very hard to make rustdoc so bad it's literally unusable at all, and the sorts of things like this aren't nearly so simple as "this font is white on white". I've never heard anyone managing to automate a screen reader navigation efficiency test. But I'm a non-accessibility backend dev so maybe there is. I believe there's more objective linters (e.g. this is an unlabeled link) but you would have to ask someone else for them, and frankly every browser/screen reader pair has different and uniquely special bugs. There's at least 16 that can matter before you even consider version numbers.

You can find a lot of good accessibility-related info at WebAIM or in the WCAG and the various guides and things people have written about WCAG. It's not all entirely on point though, and it's also kind of interesting in that by the time someone is a programmer they're also a power user and efficiency starts mattering and shifts the trade-off from nice, long labels to a preference for terseness (or does for me--but that may be subjective).

I probably don't have the bandwidth to do a lengthy audit every 6 weeks, but I'm happy to at least try to serve as a smoke test if someone pings me about it.

@ahicks92
Copy link
Contributor Author

Going to also drop this here: https://docs.rs/prost/0.8.0/prost/trait.Message.html

Which has "left bracket minus right bracket go to source code" before everything. Looking at rust beta stdlib, that's not there for at least PartialOrd, so maybe it's a docs.rs theming issue, or docs.rs is on a newer nightly than beta.

@DataTriny
Copy link

Hello,
As an NVDA user on Windows, and an Orca user on Linux, I can confirm that what @ahicks92 said is right. This new version of rustdoc pages is bad in terms of accessibility for users relying on screen reading software.
Even though I also am an advanced screen reader user, I really hope this does not land on stable as is or else it will very much hit my ability to navigate through the documentation.
I currently don't have much free time but I could at least provide some feedback too.
Kind regards.

@GuillaumeGomez
Copy link
Member

One thing I thought about to fix the "table of content" not being visible for screen readers would be to move it outside of the viewport when not being used. Would that work for you?

@apiraino
Copy link
Contributor

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 14, 2021
@ahicks92
Copy link
Contributor Author

Usually moving things out of the viewport works. There is info on making screen reader only content here that I believe works: https://webaim.org/techniques/css/invisiblecontent/

That said, if we go that route you then need a way to skip it. My preference would be for us to overall fix the header hierarchy because most screen readers including myself use those by reflex and nothing else in HTML land to my knowledge offers the same thing, but that would require rewriting user markdown I think, so as alternatives we could look at aria landmarks. Putting these before every function just means that screen readers hear every function twice because the screen reader will read the landmark and the heading, so care should be taken not to do that. It solves only the problem of e.g. "contents", "impl block", "trait impls" sorts of navigation. Also, it will be easy to walk another step down the rabbit hole and end up at aria regions, which are incredibly verbose and not something I would personally like to see here (though others may disagree).

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 16, 2021
…le, r=notriddle

Fix sidebar display on small devices

Part of rust-lang#87059.

Instead of hiding the sidebar on small devices, we instead move it out of the viewport so that it remains "visible" to our text only users.

Could you confirm it works for you `@ahicks92` and `@DataTriny` please? You can give it a try at [this URL](https://guillaume-gomez.fr/rustdoc-test/test_docs/index.html).

r? `@notriddle`
notriddle added a commit to notriddle/rust that referenced this issue Jul 17, 2021
This way, we can show the plus and minus buttons on screens, while voice
control will read off actual words "Collapse" and "Expand" instead of reading
"open brace minus close brace" and "open brace plus close brace".

Part of rust-lang#87059
@pietroalbini pietroalbini added this to the 1.54.0 milestone Jul 17, 2021
@ahicks92
Copy link
Contributor Author

that definitely does what we want with NVDA. The example on that page also appears to work with Voiceover on Big Sur. I don't have access to jaws to see what it does here. Practically speaking narrator isn't relevant: I have yet to hear of anyone using it as a primary screen reader.

I doubt anyone reads docs on the phones for the same reasons no one programs on their phone so we probably needn't worry about those so much in this case.

Good find. I'll have to remember that those tests exist for the next time this comes up.

@GuillaumeGomez
Copy link
Member

I doubt anyone reads docs on the phones for the same reasons no one programs on their phone so we probably needn't worry about those so much in this case.

I do. ^^'

@jyn514 jyn514 self-assigned this Jul 18, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 19, 2021
…-headers, r=GuillaumeGomez

Rustdoc accessibility: make the sidebar headers actual headers

Part of rust-lang#87059

Preview it at: https://notriddle.com/notriddle-rustdoc-test/rustdoc-sidebar-header/std/index.html
@ahicks92
Copy link
Contributor Author

Adding to the "the table of contents doesn't do what we want" discussion, observe this page from nalgebra: https://docs.rs/nalgebra/0.28.0/nalgebra/geometry/struct.Isometry.html

Which manages to have duplicates for all the methods due to the duplicate impl blocks, but not sorted by or indicating the impl block they go with unless you go find that another way. This seems like a bug/improvement/something-unrelated-to-this-discussion, but nonetheless gives another reason why I'm not so fond of saying "the answer is the table of contents". The heading-based solution is to jump up by heading which loses you your place, but at least offers efficiency in one direction if that makes sense.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2021
…atch, r=GuillaumeGomez

Rustdoc accessibility: use real headers for doc items

Part of rust-lang#87059

Partially reverts rust-lang#84703

Preview at: https://notriddle.com/notriddle-rustdoc-test/real-headers/std/index.html
GuillaumeGomez pushed a commit to pietroalbini/rust that referenced this issue Jul 26, 2021
Part of rust-lang#87059

Partially reverts rust-lang#84703

Heavily modified for beta backport needs.
@pietroalbini
Copy link
Member

Hello @ahicks92!

Rust 1.54.0 is going to be released this Thursday, and I think I backported all the PRs referenced in this issue to the upcoming stable. Could you check whether the pre-release documentation still presents regressions compared to 1.53.0?

@ahicks92
Copy link
Contributor Author

Yeah, that looks good. Specifically I hit Vec.

Thanks for prioritizing this.

@ahicks92
Copy link
Contributor Author

Sorry for the double comment. If there's further specific tests you want I'm happy to do them.

@jyn514
Copy link
Member

jyn514 commented Jul 27, 2021

@ahicks92 that's great! Are there any further fixes you'd like, or can the issue be closed?

@ahicks92
Copy link
Contributor Author

We can close this out and I'll open more issues if I find other things. In terms of the urgent my entire workflow broke things, we're done. Thanks.

@GuillaumeGomez
Copy link
Member

Thanks for the feedback!

GuillaumeGomez pushed a commit to GuillaumeGomez/rust that referenced this issue Aug 3, 2021
This way, we can show the plus and minus buttons on screens, while voice
control will read off actual words "Collapse" and "Expand" instead of reading
"open brace minus close brace" and "open brace plus close brace".

Part of rust-lang#87059
JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 3, 2021
…brace, r=notriddle

Rustdoc accessibility: use an icon for the [-]/[+] controls

This is a reopening of rust-lang#87207 with improvement for the way of generating the `background-image` CSS property.

I quote from the original PR:

> This way, we can show the plus and minus buttons on screens, while voice
> control will read off actual words "Collapse" and "Expand" instead of reading
> "open brace minus close brace" and "open brace plus close brace".

Part of rust-lang#87059

r? `@notriddle`
JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 3, 2021
…brace, r=notriddle

Rustdoc accessibility: use an icon for the [-]/[+] controls

This is a reopening of rust-lang#87207 with improvement for the way of generating the `background-image` CSS property.

I quote from the original PR:

> This way, we can show the plus and minus buttons on screens, while voice
> control will read off actual words "Collapse" and "Expand" instead of reading
> "open brace minus close brace" and "open brace plus close brace".

Part of rust-lang#87059

r? ``@notriddle``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants