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

cmd/cover: (html output) UI accessibility issues, unfriendly to screen reader #36685

Open
dr2chase opened this issue Jan 21, 2020 · 13 comments
Open
Labels
ExpertNeeded help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dr2chase
Copy link
Contributor

What version of Go are you using (go version)?

go 1.13, go 1.14

Recently on golang-nuts, "Is it possible to get code coverage information in a way that does not assume you can see color?"

https://groups.google.com/g/golang-nuts/c/DY4O9UXMr9M?pli=1

In this case, "see color" refers to a screen reader for a totally blind person.

This is a bug, not an enhancement, because accessibility is important.
It does need someone who knows something about UI accessibility to look at it.

@toothrot toothrot added this to the Backlog milestone Jan 22, 2020
@toothrot toothrot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 22, 2020
@toothrot
Copy link
Contributor

/cc @bcmills @jayconrod

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2020

I know that @katiehockman has some expertise in UI accessibility, and she may be aware of others on the project.

I just did a quick skim of the ARIA docs and I suspect that the group role is probably what we want here.

I don't know whether we should use aria-label, aria-describedby, or something else to demarcate which groups are “covered”, “uncovered”, and “uninstrumented”.

CC @matloob @ianthehat

@bcmills bcmills added ExpertNeeded NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 22, 2020
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 22, 2020
@katiehockman
Copy link
Contributor

@dr2chase Thanks for filing this issue! See my related comments on the coverage tool colors from a previous issue: #27553 (comment)

In general, I would want someone from UX to take a look at this too, since I think improving the accessibility of the coverage tool may be a big effort, although of course a valuable one.

I just did a quick skim of the ARIA docs and I suspect that the group role is probably what we want here.

@bcmills are you suggesting the group role be on an HTML tag that surrounds an entire section of either "covered", "uncovered", or "uninstrumented"? That might work. I'll have to look into this a bit more to make a more solid recommendation. My hesitation comes from the fact that the output of the coverage tool isn't a typical web page, so figuring out how to retrofit ARIA tags in the right way will probably be a bit nuanced.

I don't know whether we should use aria-label, aria-describedby, or something else to demarcate which groups are “covered”, “uncovered”, and “uninstrumented”.

aria-label could work. It would be nice if aria-describedby could be used so that if we change the label text then we only change it in one place, rather than changing every aria-label element. However, I don't usually see aria-describedby used in this way. Normally it is to map something like a button to one or more elements on the page that describe it (by listing those element's ids), rather than to map an element to all the elements it describes. Definitely something that we should check with a few screen reader + browser combos if we go that route.

@bcmills
Copy link
Contributor

bcmills commented Jan 23, 2020

are you suggesting the group role be on an HTML tag that surrounds an entire section of either "covered", "uncovered", or "uninstrumented"?

Yep, exactly: throw a <div> around each span of adjacent covered, uncovered, or uninstrumented lines. I suspect that folks visually reading coverage tend to do so block-by-block, so my hypothesis is that the same sort of block-by-block strategy might work for screen-readers as well. (I suspect that a line-by-line annotation would be too noisy for runs of covered or runs of uncovered lines.)

And with a div around each region, we can assign an ARIA group and a CSS class to the same semantic entity, so we can be confident that we're not dropping any semantic information from one channel that we're preserving in the other.

@jimmyfrasche
Copy link
Member

role=group is for grouping interactive elements. It's like the fieldset tag.

It sounds like you just want something like

Line 60, 10% covered, foo(bar) // colored 10% green

right?

You can put the "10% covered" in a span with a visually-hidden class like https://a11yproject.com/posts/how-to-hide-content/

Alternately you could use that for "not covered" lines and, for lines with a coverage %, use an svg rect with an aria-label of the % covered. That would give a nice little series of bar charts to make it easier for everyone to glance the magnitude of relative differences as well as more strictly following WCAG. That would be nice because it survives b&w printing and can be perceived even a room with bad lighting.

@bcmills
Copy link
Contributor

bcmills commented Jan 23, 2020

@jimmyfrasche, generally runs of coverage span multiple lines of code. When you are visually reading a coverage report, you probably don't read it as “line 1 is 100% covered, line 2 is 100%, line 3 is 100% covered, […]”. You probably read it as “lines 1–40 are covered, 41–44 are not, and 50–64 are covered. Maybe I should add a test for the error condition at line 41.”

Arguably the accessible view should present that information at a similar density.

@bcmills
Copy link
Contributor

bcmills commented Jan 23, 2020

@jimmyfrasche, I don't see any mention of an “interactive” requirement in https://www.w3.org/TR/wai-aria-1.1/#group. It is defined as:

A set of user interface objects which are not intended to be included in a page summary or table of contents

And note that https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques places group in the “Document structure roles”, not “Widget roles”.

OTOH, I am not at all well-versed in ARIA usage, and presumably we want to do whatever screen-readers will interpret properly — do you know of other similar applications with good screen-reader support that we could mimic?

@jimmyfrasche
Copy link
Member

I read that as meaning what is being grouped is interactive ("user interface elements") but that does not seem like it is a well defined term in the spec. All the official examples are for using it contain interactive elements. You may be right, though, since the role for a table row is a subclass of the group role. I couldn't find any examples anywhere that did not contain interactive elements. It looks like JAWS ignores it if it doesn't contain interactive elements, so it wouldn't buy anything in this situation https://freedomscientific.github.io/VFO-standards-support/aria.html#index-aria-group

I'm not aware of any similar applications with good screen reader support. Anything similar by microsoft has a good chance, though. They tend to be really good about accessibility stuff.

Asking the OP for other tools and information density preferences would probably be more productive than us guessing.

@jareds
Copy link

jareds commented Jan 29, 2020

I'm the original poster from the golang-nuts group. When reading the following keep in mind I'm a back end programmer who hasn't written HTML in over 10 years so the rest of this should be read with that caveat in mind. Looking at the html generated by the cover tool it appears that code coverage is on a line by line basis and there's no block information. Ideally it would be clear that all the code inside an if statement is not covered. It would also be nice to easily navigate to uncovered code. I've created a prototype of this by generating a coverage report and manually edit the html. See
https://github.com/jareds/gocoverscreenreaders
If you compare the two latest commits in cover.html you will see I added a role="region" to the span that contains uncovered code. In the for loop I also added all uncovered code to a single span since navigating by region would create a lot of noise if you had a bunch of sequential lines that are uncovered. This markup allows me to navigate to the uncovered code with Jaws by using r to find the next region. In NVDA I can use the next landmark navigation keystroke to find the region. I don't have a Mac so have no idea how Voiceover works. I had to change the title because both NVDA and Jaws read the title when navigating to the region. Having "0" announced because it's the title does not provide any useful information. According to the doc at
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Region_role
aria-label may work around the title issue but I have not tried this. Before the region role I tried both role="article" and the article tag. As far as I can tell NVDA does not announce articles in Chrome as of NVDA version 19.2.1.

@jonbodner
Copy link

In light of https://commandcenter.blogspot.com/2020/09/color-blindness-is-inaccurate-term.html and #37796 being closed in favor of this issue, would it be possible for there to be a first step where there's an option for code coverage HTML to use a white background instead of a black background? Maybe with bold to indicate covered code, regular text to indicate uncovered code, and italics to indicate code that can't be covered? Screen reading and UX is important, but maybe some steps along the way could also help.

@grantwwu
Copy link

grantwwu commented Aug 9, 2021

+1 to jonbodner's comment - screen reader support would be great for those who use screen readers, but I think it does make sense to have a separate issue for color configurability. This issue was originally reported about screen reader support, and implementation of these two things is likely going to happen separately.

@willbeason
Copy link
Contributor

willbeason commented Oct 7, 2021

+1 I have difficulty seeing red against the dark background (even with my screen brightness turned all the way up), so I end up having to highlight the lines in order to read them. (Ditto the grayed-out lines, which I use as visual anchors even though the coverage isn't important)

@mpetronic
Copy link

One hack that is actually very easy to do in a couple seconds is to manually adjust the style using the browser inspector (F12). In Firefox, I just uncheck a couple boxes to get the look I want which is smaller font, not bold, and white background. You do have to apply this every time you reload the page but NOT as you pick the different files already in the coverage report. Its at least someone you can do right now while this issue is being considered.

BEFORE:

image

AFTER:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ExpertNeeded help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests