FortAwesome / Font-Awesome

The iconic SVG, font, and CSS toolkit
https://fontawesome.com
Other
73.63k stars 12.19k forks source link

Update accessibility #6133

Closed dylanb closed 8 years ago

dylanb commented 9 years ago

Please do two things:

  1. Remove the mis-representation from your web site until you have properly fixed the problem, so that you do not continue to make the web a worse place for people with disabilities,
  2. Fix the accessibility problems properly
karlgroves commented 9 years ago

@dylanb It might be good to cite the specific location(s) where the claim is made, this way an interested developer can find & fix the issue and issue a PR

dylanb commented 9 years ago

@karlgroves http://fontawesome.io/

karlgroves commented 9 years ago

Deleted my prior comment.

@dylanb - as an issue description, I think you know it should have better information in it so that action could be taken

@davegandy - The claim made in the footer of the http://fontawesome.io/ website which says " Screen Reader Compatible" is demonstrably false.

Given the icon fa-bed (http://fontawesome.io/icon/bed/)

Using the following markup: <i class="fa fa-bed"></i>

It creates CSS generated content: "\f236";

There are three accessibility problems that may result:

  1. (and this is @dylanb's point) Screen readers may read that CSS generated content. The workaround for this scenario is to use aria-hidden like so: <i class="fa fa-bed" aria-hidden="true"></i> which would allow a screen reader to ignore it. BUT this raises another concern
  2. Because it creates generated content, there's no text alternative.
    Imagine I did this <a href="http://www.github.com"><i class="fa fa-github"></i></a> That has no content in the text node of the DOM. It is, in almost all cases, a blank link. Going back to the above, users may get access to the generated content which, in all likelihood, actually sucks to hear on a screen reader. So here's the fix

    <a href="http://www.github.com"><i class="fa fa-github" aria-hidden="true"></i><span class="sr-only">Github</span></a>

    That uses the 'sr-only' class naming convention from Bootstrap, BTW. That's not necessary, but what is necessary is basically that (if you don't want the word "Github" to appear) the span be positioned off screen to hide it. This is effectively the same as having an alt attribute on an image

    But wait! There's more

3) There's a final problem: Windows High Contrast Mode. With Windows High Contrast mode turned on, the font icon is essentially rendered invisible. The problem is that it even with the offscreen span approach, the text in the span won't be visible. In short, to be fully accessible something else has to happen - namely detecting HCM and doing something to show the previously-hidden span: http://jsfiddle.net/karlgroves/XR8Su/6/

TL;DR: @dylanb is right. Your claim of being "Screen Reader Compatible" is inaccurate. I recommend removing this claim and improving documentation so people understand the accessibility implications of icon fonts.

dylanb commented 9 years ago

+1 what @karlgroves said :-)

tagliala commented 9 years ago

As far as I can tell from the above points, the statement compatible is still valid.

It seems that accessibility needs more docs.

tagliala commented 9 years ago

@dylanb Sorry for this bug but honestly I didn't like your opening post, I found it a little bit aggressive.

@karlgroves thanks for the very detailed explanation.

That claim is probably there because in the past icon fonts didn't use :before pseudoelement and didn't use PUA, so screen readers were "tripped up"

Please take a look #389, including my comments there

ZoeBijl commented 9 years ago

@karlgroves wouldn't a simple aria-label="bed" suffice? You could generate a ::before/::after with its text if HCM is enabled, right?

So something like this: <i class="fa fa-bed" aria-label="bed"></i>

I've made a testcase on GitHub which I've tested successfully in VoiceOver on OS X 10.10 with Safari 8.

Heydon commented 9 years ago

+1, @MichielBijl for aria-label

LJWatson commented 9 years ago

"1. (and this is @dylanb's point) Screen readers may read that CSS generated content."

With the exception of IE, most browsers (used in conjunction with screen readers) expose CSS generated content through the accessibility layer. http://tink.uk/accessibility-support-for-css-generated-content/

hanshillen commented 9 years ago

Just to confirm and / or clarify a few things:

  1. Yes, the generated character needs to be hidden from screen readers. In a lot of cases the character will have no effect, but it's possible that the screen reader will announce something incorrectly. For example, VoiceOver on IOS can announce the characters as (completely unrelated) emoji symbols.

    And yes, aria-hidden should be used for this. Unfortunately, placing the attribute on the element (that has the icon related class name) itself is not going to work. Since font icons use the :before pseudo selector, the inserted character will end up adjacent to the element with the aria-hidden attribute rather than inside of it. In other words, the character will still be exposed to assistive technology, and aria-hidden will unnecessarily hide an empty element.

    So for this to work, you would have to wrap another element around the , ensuring that the character will still be inserted inside of it. For example:

     <span aria-hidden="true">
         <i class="fa fa-bed"></i>
     </span>

    Unfortunately this makes the code a bit more bulky, but it will solve this particular accessibility issue.

  2. Yes, as with any meaningful graphic, alternative text will have to be provided (by the developer, not the icon library). The easiest way to do this, as Karl mentioned, is with Bootstrap's sr-only class (or a similar class used for visually hiding content in an accessible way).

    Combined with the previous item, code would look like this: Bed

    Note that the "appropriate" text to insert will depend on how the icon is used. It could be "bed", "available beds", "bookings" etc. depending on the context. Or the text could be left out altogether if the icon is used redundantly (adjacent to visible text), or decoratively.

    The aria-label approach seemingly makes sense, but is not guaranteed work consistently across browsers and screen readers. The reason is that generally, aria-label would be applied to provide an accessible name to an element with a valid role (e.g. "button", "tab", etc). Adding it to a role-less element such as may cause the aria-label text to be ignored depending on the browser / screen reader combination used.

    For this reason, using hidden text as described above (.sr-only) is more reliable, and it has an additional benefit: If CSS were to be disabled for whatever reason (e.g. by the user's choice) the hidden text would become visible and provide a fallback for the now invisible icon. You would't get this benefit with attributes like title or aria-label.

  3. Font icons are actually Windows High Contrast Mode safe, because HC Mode does not override the icon's font-family (just the color and size). However, the issue Karl describes does occur for user defined stylesheets, which are likely to do exactly that: override font-family for all elements, causing the font based icons to break. It's one of the major accessibility issues related to font icons that is (unlike to the other two issues) hard to resolve by the dev. It requires some workaround, like a script that detects overridden font family or an "invisible font" for the fallback text. Both these approaches are described at http://files.paciellogroup.com/training/CSUN2014/lessonslearned/ (use case 3, see slide deck for background info).

These issues are not specific to FontAwesome, but apply to font based icon sets in general. The first two are solvable through some (perfectly reasonable) effort on the developer's end. Since the average dev is not going to automatically understand all this, it would be good if these issues and approaches were documented. Not just for FontAwesome, but for the other icon sets as well (icomoon.io would be a good place). So yes you can easily use FontAwesome without tripping up a screen reader, but it would help if the documentation didn't make it seem like this is the case out of the box and explain what the dev needs to do to get there.

The third issue is not related to screen readers but still a big problem (which should also be documented).

StommePoes commented 9 years ago

@hanshillen @karlgroves re Hans' item 2: how does the sr span inside the aria-hidden get seen? I thought that covered all descendants?

hanshillen commented 9 years ago

@StommePoes you're absolutely right, I wasn't paying attention. have updated the snippet accordingly!

tagliala commented 9 years ago

@hanshillen thanks for your post

We should definitely document this.

Do you know if the speak: none css property could be used to avoid the extra markup?

edit: it seem it doesn't http://modernwebaccessibility.com/en/blog/demystify-speak-none

StommePoes commented 9 years ago

@tagliala I've never heard of any of the CSS audio stuff working for anything other than nasty IE5.5 hacks :P

ZoeBijl commented 9 years ago

@hanshillen thank you for the clarification. Seems that VO indeed shows the character, it just didn't say it. Might just be the characters in my example, though.

Jonathan-Eyler-Werve commented 9 years ago

Thanks to everyone working on this from a developer deploying Font Awesome who cares about accessibility.

IdanCo commented 9 years ago

I've been working on a very big project with allot of emphasis on accessibility. While looking for a screen-reader friendly solution for font icons, I came across @hanshillen solution and really liked it. So I've implemented it in an angular directive - https://github.com/picardy/angular-fontawesome Hope you'll find it useful

StommePoes commented 9 years ago

I'm re-reading this part by @hanshillen: "Since font icons use the :before pseudo selector, the inserted character will end up adjacent to the element with the aria-hidden attribute rather than inside of it."

Maybe I'm misunderstanding pseudo-elements, but someone with a :before has a new first-child, not a new previous sibling. Someone with an :after has a new last-child, not a new next sibling.

So logically thinking, aria-hidden on someone should cover all of someone's children, and :before and :after are children, not new siblings. Correct?

<i aria-hidden="true">[ :before-child, existing-or-empty-default-children, :after-child ]</i>
IdanCo commented 9 years ago

@StommePoes you just blew my mind. I might be looking at it all wrong. the problem is that i don't have a screen reader at hand, so QAing this is bit of a strech. I'll do some expirements and get back to you.

hanshillen commented 9 years ago

I'll do some more testing as well

hanshillen commented 9 years ago

@StommePoes is right. I tested with most common browser / screen reader combos, and adding aria-hidden="true" directly to the element that has the class name will successfully hide the icon's character inserted through the ::before pseudo selector. I've updated my initial comment accordingly, taking out the step about wrapping the icon in a separate element.

IdanCo commented 9 years ago

Great! I came up with similiar results. I'll update the directive in angular-fontawesome accordingly

tagliala commented 9 years ago

@hanshillen do you confirm that using <span class="fa fa-icon" aria-hidden="true"></span> does not work? Or, at least, it doesn't work with some browser and some screen readers?

hanshillen commented 9 years ago

No, my tests from today show that that snippet does work, so my earlier comment about having to wrap the icon in a separate element is no longer applicable.

Regards,

Hans Hillen

On Sep 24, 2015, at 18:01, Geremia Taglialatela notifications@github.com wrote:

@hanshillen do you confirm that using does not work? Or, at least, it doesn't work with some browser and some screen readers?

— Reply to this email directly or view it on GitHub.

tnguyen14 commented 9 years ago

@hanshillen your original comment still has this code snippet

<span aria-hidden="true">
      <i class="fa fa-bed"></i>
  </span>

Is that still the recommended way, or does @tagliala suggestion works, i.e.

<span class="fa fa-icon" aria-hidden="true"></span>
tagliala commented 9 years ago

Please let me know if something like

<span class="fa fa-icon" aria-hidden="true"></span>
<span class="fa-sr-only">Icon</span>

where fa-sr-only is implemented like bootstrap's sr-only is ok

IdanCo commented 9 years ago

@tagliala I expect it to work great. My tests confirm it as well, also articles here and here.

A similiar fix is now implemented in angular-fontawesome.

hanshillen commented 9 years ago

@tnguyen14 yes @tagliala's snippet is the correct one. For some reason I'm not able to strikethrough a code snippet in Markdown, so my original one is still there in between strikethrough text. I could remove it altogether but I rather leave it there for transparency and to make sure people will understand other comments. @tagliala yes that last snippet is the way to go.

StommePoes commented 9 years ago

@hanshillen after the strikethrough section, end of section one you can just add in non-strike text pointing to one of the comments with the correct code, that'll make the whole comment clearer I think.

ZoeBijl commented 8 years ago

@IdanCo I hate to be that guy, but links with text like “here” aren't that accessible in them selfs ;) And are your test published anywhere? I'm interested in the outcomes.

@hanshillen @StommePoes I agree that would clarify it.

ZoeBijl commented 8 years ago

@a11ydoer and I are doing some testing on this subject at the moment. Any help is welcome!

tagliala commented 8 years ago

@MichielBijl thanks!

teckel12 commented 8 years ago

Why not just:

@media only speech { i.fa { display: none } }

ZoeBijl commented 8 years ago

@teckel12 does that work anywhere?

StommePoes commented 8 years ago

I see in the CSS4 specs there's talk of "a speech device" but... a regular browser that happens to have a screen reader as a secondary desktop application running at the same time... I'm not sure how CSS would know that. It's possible something else was thought of when the writers say "a speech device"...?

If a media feature references a concept which does not exist on the device where the UA is running (for example, speech UAs do not have a concept of “width”), the media feature must always evaluate to false.

Since a browser running along with a screen reader does have the concept of width, I suspect they mean something else.

Additionally, there's plenty of people who, for various reasons (low vision but no totally blind, dyslexia, second-language) a user may need to see what we mean to present to sighted users, while the user is using a screen reader.

ZoeBijl commented 8 years ago

That is my thinking too. If @media speech was able to detect SR's, you'd basically have a SR media query; which we know to be a bad idea. So I don't think it'll work.

StommePoes commented 8 years ago

Oh, here it does specifically refer to screen readers: https://drafts.csswg.org/mediaqueries-4/#valdef-media-speech

But there's still the issue of sighted users with screen readers. We don't want the icon garble-blarble read out, but we don't want to display: none it just because someone's also got a SR (or a screen mag with Reader) running.

karlgroves commented 8 years ago

There's no practical support among screen reading technology for the proposed approach by @teckel12

tagliala commented 8 years ago

Could you please take a look at this PR: #8879?

dylanb commented 8 years ago

I added multiple comments

tagliala commented 8 years ago

@dylanb thanks for the feedback. Just a curiosity of mine: why the role="presentation" stuff didn't come out here? I was stick to https://github.com/FortAwesome/Font-Awesome/issues/6133#issuecomment-143009933

dylanb commented 8 years ago

@tagliala your markup was

<span class="fa fa-icon" aria-hidden="true"></span>
<span class="fa-sr-only">Icon</span>

This is different from the markup in the commits which had an enclosing element and the aria-hidden was placed on the enclosing element (which hides all children). This may seem like an insignificant difference, but it is not.

<i class="fa fa-cog" aria-hidden="true">
    <span class="sr-only">Settings</span>
</i>

Also, I probably wasn't paying close enough attention to recommend the better solution - apologies.

Actually, come to think of it, you may still want aria-hidden to suppress the announcement of the unicode if that is causing trouble but you will certainly want to not put your screen reader text inside the aria-hidden element.

tagliala commented 8 years ago

Actually, come to think of it, you may still want aria-hidden to suppress the announcement of the unicode if that is causing trouble but you will certainly want to not put your screen reader text inside the aria-hidden element.

Sure, is there any code in the commits that does this? I'm not able to find it

Just found this one http://john.foliot.ca/aria-hidden/ are those information up to date? What about both role="presentation" and aria-hidden="true"?

Btw bootstrap only uses aria-hidden for its glyphicons

StommePoes commented 8 years ago

I think what needs to be made clear here is what those two things do. Role=presentation can take something which has a default, native role in HTML (an <img/> has a default, native role of "img", a <button> has a default, native role of "button" etc) and remove that role, making that thing seem like a <div> or <span>. Putting role="presentation" on something that has no default, native roles (like <i>... or <div>) does nothing useful. It can also, of course, override a role assigned earlier by an author (someone made a <div role="button"> for some reason and now wants to remove that role). It does not silence anything inside the tag or remove the content from the accessibility tree in the browser. It just removes any name for that content, if there was any.

aria-hidden removes the thing, and its content, from the accessibility tree. If you were to put aria-hidden="true" on a something that happens to have a default, native (or author-assigned) role, I don't believe there's anything to be gained by also adding role="presentation" to that-- hidden is hidden.

I had seen on github some of the new SVG icons which had both aria-hidden=true and an added role=img. I had asked someone why that was, and the answer was "sometimes the SVG with the role=img isn't aria-hidden" so roles or role=presentation might make sense somewhere if you're concerned about announced roles when removing aria-hidden, I suppose. However in the context of icons in <i> tags, I don't see roles either way being useful.

talbs commented 8 years ago

This is different from the markup in the commits which had an enclosing element and the aria-hidden was placed on the enclosing element (which hides all children). This may seem like an insignificant difference, but it is not.

Actually, come to think of it, you may still want aria-hidden to suppress the announcement of the unicode if that is causing trouble but you will certainly want to not put your screen reader text inside the aria-hidden element.

@dylanb, thanks for the review and these catches. My bad for the incorrect copy and paste location - avoiding placing the text alternatives inside of the aria-hidden="true" element makes perfect basic sense. :)

dylanb commented 8 years ago

@talbs my pleasure, you might also want to take heed of the title attribute suggestion for users with cognitive disabilities

talbs commented 8 years ago

@talbs my pleasure, you might also want to take heed of the title attribute suggestion for compatibility with Dragon

Yeah, can you walk me through your thinking when you noted that (see https://github.com/FortAwesome/Font-Awesome/pull/8879#discussion_r58314675)? Why and exactly when would using a title attribute be better to recommend than the <span class="sr-only">alternative text equivalent</span>?

Thanks for the help.

dylanb commented 8 years ago

Sorry, I corrected my statement above

davegandy commented 8 years ago

First, let's clear the air. None of these issues are intentional misrepresentations. The world changed since that statement was originally made. And we're fixing those problems right now.

Stepping back from this particular issue, I don't believe we've been given the benefit of the doubt here. Some particular folks appear to belive this was malicious on our part. It was not. Just like you would have for your own self, I'll challenge you to assume good motivations on the part of others.

And know that your reputation follows you online. Especially here on GitHub. Folks in the community learn who the bad actors are and don't like dealing with them, no matter how smart they are and what they know. Seriously, think about it. If you think this might be written to you, it probably is. Your involvement is honestly appreciated. But please be polite, respectful, and assume good intentions of others.

For context, when Font Awesome came out 4 years ago, making icons as fonts work with screen readers was a major priority. And it worked well. Since then, the world changed. Screen readers changed, but Font Awesome didn't keep up.

Going forward, vetted accessibility issues will be given priority for any subsequent release. It's not been enough of a priority lately, and we're changing that.

We really do appreciate you experience, knowledge, and expertise. Thank you for helping out the project, and for helping the literally millions of folks that use this.

StommePoes commented 8 years ago

I think the responsiveness of the folks replying showed right away that someone at FA cares. Otherwise this issue would have languished as one of many moldering github issues. Instead, it's got good debate, exchange of code techniques, and open discussion. Seems to reflect well on the FA folks.