1464858 - Visually hidden content with display: none; in reader mode - .sr-only .visually-hidden

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect, P3)

Description

[Joe Watkins]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce:

Add a CSS class to markup that you'd like to visually hide e.g. sr-only, visually-hidden.

<a href="/">Read more<span class="at-only"> about this and that</span>


Actual results:

Firefox assumes you'd like this completely hidden from AT and adds display: none; via Reader stylesheet.

.moz-reader-content .visually-hidden, .moz-reader-content .visuallyhidden, .moz-reader-content .hidden, .moz-reader-content .invisible, .moz-reader-content .sr-only {
  display: none;
}


Expected results:

Respect the original visually hidden result and allow for screen readers to access this content even in reader mode. Currently, if you don't use a popular selector such as .visually-hidden or .sr-only (Bootstrap) Reader will output all of the text - which is ok too but not great, at least SR can get to that content but may be verbose/clunky for sighted users.

A common tactic for handling offscreen content is something like this:

.visuallyhidden {
    border: 0;
    clip: rect(0 0 0 0);
    height: 1px;
    margin: -1px;
    overflow: hidden;
    padding: 0;
    position: absolute;
    width: 1px;
    white-space: nowrap; /* 1 */
}

Perhaps the reader could support this?

Comment 1

[:Gijs (he/him)]

Unfortunately this is difficult/impossible to do because in the current implementation the reader mode algorithm has no access to any non-inline styles. So it has no way to know what classes "mean". In other popular sites (like wikipedia) sr-only/visually-hidden/display:none content is genuinely not supposed to (ever) be displayed, so making the inferences here is very tricky...

Why are you hiding content from everyone-but-screenreaders? Do you have an online example/testcase that we could refer to?

Comment 2

[Joe Watkins]

Hello :) Thanks for taking a look at this one. It's a bit confusing. Two things to consider:

1. display: none; on content will not get announced by a screen reader.
2. For some reason, FF respects the HTML class of .visually-hidden if an element owns that class. It actually leaves it in the DOM and then the browser's stylesheet: aboutReader.css has CSS styles that set that to display: none;

99% of the time, because it's become a proven standard over time, if an author declares something to be visually hidden it is important content that assistive technology still should have access to but not visually clutter the interface. This is done typically to convey greater link purpose, or even state.

Prototype that demonstrates this issue:
(Here we are building greater link context for screen reader users using visually hidden text)
https://s.codepen.io/joe-watkins/debug/gKWJva

Screenshot of what is going on w/display: none;
https://www.screencast.com/t/Yk8CDKP3

.visually-hidden, .visuallyhidden, .at-only = should still accessible to screen readers
.sr-only = stands for screen reader only - should still accessible to screen readers
etc.

Here is my recommendation for Firefox's aboutReader.css styles that are in action for a page in Reader Mode.

Current aboutReader CSS:
.moz-reader-content .visually-hidden, .moz-reader-content .visuallyhidden, .moz-reader-content .hidden, .moz-reader-content .invisible, .moz-reader-content .sr-only {
    display: none;
}

Recommended CSS:
/* hide stuff for reals - dunno about .invisible */
.moz-reader-content .hidden, .moz-reader-content .invisible {
    display: none;
}

/* Visually hide stuff - leave accessible to AT */
https://github.com/h5bp/html5-boilerplate/blob/master/dist/css/main.css#L137
.moz-reader-content .visually-hidden, .moz-reader-content .visuallyhidden, .moz-reader-content .sr-only {
    border: 0;
    clip: rect(0 0 0 0);
    height: 1px;
    margin: -1px;
    overflow: hidden;
    padding: 0;
    position: absolute;
    width: 1px;
    white-space: nowrap; /* 1 */
}

Would something like that be possible?

I'm not able to find visually hidden CSS definition in Wikipedia, but I'm confident that the web leans towards not using display: none; for visually hidden stuff.. and if they are it is not correct. :)

Comment 3

[:Gijs (he/him)]

(In reply to Joe Watkins from comment #2)
> Hello :) Thanks for taking a look at this one. It's a bit confusing. Two
> things to consider:

Hi! Yes, I'm actually reasonably familiar with a11y techniques in HTML / ARIA in general, I was mostly curious about the context in which you were using this particular one. It... can be not ideal because not everyone uses screenreaders, and so while the "kind of but not really" hidden elements might work for users of Jaws/NVDA, it's less useful for e.g. sighted keyboard users, or people using magnifying tools. In this case (links that need more context/text) I would have gone with title attributes, though I'm aware those have drawbacks of their own (too many of them can become distracting). Or just linking actually meaningful text (which isn't always possible, depending on the context in which these links exist). Anyway, your general point stands.

> 99% of the time, because it's become a proven standard over time, if an
> author declares something to be visually hidden it is important content that
> assistive technology still should have access to but not visually clutter
> the interface. This is done typically to convey greater link purpose, or
> even state.

Right... I assumed that the fact that we had the rule we did meant that it was introduced because people used it for display: none content that also shouldn't be accessible to AT. Certainly the .hidden/.invisible classes would be used like that in e.g. bootstrap. But from code archaeology (introduced in bug 1144822) and reading comments in that bug, it seems we just lumped everything together in without really caring about the semantics, which I definitely agree is unfortunate.

> Recommended CSS:
> <snip>
> 
> Would something like that be possible?

Yes, that would work! I would gladly take a patch. Would you be able to submit one, seeing as you seem to have mostly written one already? :-)

> I'm not able to find visually hidden CSS definition in Wikipedia, but I'm
> confident that the web leans towards not using display: none; for visually
> hidden stuff.. and if they are it is not correct. :)

Eh, the wikipedia stuff is complicated... It uses inline display: none for math stuff when duplicating it in images/mathml (cf. bug 1430493) where IMHO there isn't much point in keeping the content (as the mathml could/should be made accessible and would be better than the original fake-ish math-as-plaintext), but it looks like other mathjax things do have semantic-ish classes to hide for AT stuff only (bug 1466044).

Comment 4

[Joe Watkins]

Hi There :) Thanks again for chewing on this with me. I don't think I'd be able to submit a patch.. not sure where I would even start on that. I'd be happy to review a code change / patch if needed though!

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1464858 - make visually hidden / screenreader content not display:none,

Review commit: https://reviewboard.mozilla.org/r/251620/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/251620/

Comment 6

[:Gijs (he/him)]

Going to give this a shot.

One of the open questions that I'm less sure about is whether we should also make any text in these elements unselectable using -moz-user-select: none .

Otherwise, I omitted the clip path and position: absolute but added display: inline-block to enforce that the width/height setting actually has an effect. I *think* this should cover us also because no other CSS from the page should apply anyway. Let me know what you both think!

Comment 7

[Eitan Isaacson [:eeejay]]

Hey! This patch looks ok for what it does. I'm honestly not thrilled about this because there is almost always a markup alternative for this technique when it is applied inline. For example, the given case should probably use a `title` attribute, like Gijs said. In more complex stuff, like "purpose" or "state", aria-role and one of the aria state attributes could be used. This should be preferred for many reasons.

The one time when I see this technique used effectively is for site/page screen reader controls, like jump to content or an outline with anchor links. These kinds of controls should get stripped in reader mode anyway, so I don't see this as useful in that case.

I'm also reluctant about the part where we assume the intention of external class names, but it looks like we do that already.

Joe, outside of a codepen example that could be fixed, is there anything out there in the wild that would benefit from this?

Comment 8

[Joe Watkins]

Hi Eitan, Thanks for your feedback. It is widely accepted these days that use of the title attribute to convey important information isn't recommended for many reasons - with exceptions e.g. <iframe>. I've rounded up some documentation on the topic.

https://developer.paciellogroup.com/blog/2012/01/html5-accessibility-chops-title-attribute-use-and-abuse/
https://inclusive-components.design/tooltips-toggletips/#thetitleattribute
https://developer.paciellogroup.com/blog/2010/11/using-the-html-title-attribute/
https://a11yproject.com/posts/title-attributes/
https://silktide.com/i-thought-title-text-improved-accessibility-i-was-wrong/
http://simplyaccessible.com/article/title-attributes/
https://www.mediacurrent.com/blog/dont-rely-title-attribute-accessibility/

Even your MDN warns:
https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/title

ARIA sure saves the day for many things but only for assistive technology that supports it. We can reach farther back with simple text that is within a link to define the link's accessible name. It sounds edge case, but accessibility is all about reaching as many people as possible and lots of people with disabilities leverage older technology. I recently lead a usability study with people that had disabilities, and one of them was using JAWS 12 w/IE11 - and there wasn't any ARIA support for that user.

Link purpose is going to be the most common use case as it's really not a great idea to have a bunch of screen reader specific content that doesn't match visual content.

This technique is one that the W3C recommends to satisfy WCAG 2.4.4 Link Purpose (their CSS is a bit old school but same thing)
https://www.w3.org/TR/2012/NOTE-WCAG20-TECHS-20120103/C7

Mozilla.org's .visually-hidden {}
https://www.screencast.com/t/KaqS8pkvC

Also, HTML5 Boilerplate is a pretty popular project that many folks get their visually hidden CSS from thinks of it as the following and not as display: none;
https://github.com/h5bp/html5-boilerplate/blob/master/src/css/main.css#L130


The PR looks good to me :) Thanks for your consideration here!

Comment 9

[Eitan Isaacson [:eeejay]]

Hey Joe,

Thanks for all those resources and thought you put into this.

If the goal is to convey purpose of a link to a screen reader user, 'title' will almost always do the trick. The AAM requires the 'title' tag to end up in the accessible's description, or the name if no text content or label is provided. The majority, if not all, screen readers know to pass descriptions on to the user in this case.

https://www.w3.org/TR/html-aam-1.0/#accessible-name-and-description-computation

You are right to say that 'title' has accessibility problems, specifically for keyboard users and magnifier users, but the alternative screen-reader-only content offers no accessibility to those users at all.

You are also right to say that not everyone is on the latest version of JAWS, although it spoke the 'title' attribute as early as 2013. I believe that there is benefit in allowing ATs to present supplementary content to the user in a discernible way that differs from primary (visible) content. I would prefer we make good use of the available semantics to enrich the accessible experience.

Using inline hidden markup gives a screen reader user a false perception of what is actually presented on the page, and can leave the user confused when doing copy/paste or caret navigation. A non screen-reader user who depends on caret browsing will be perplexed when they are arrowing through text and the caret disappears for the duration of the screen-reader only text.

You did make me realize that there is no perfect alternative to 'title' for link purposes, which is unfortunate. But I am fairly convinced that hidden inline content is almost never the right choice. I'm not sure I can persuade you :)

Comment 10

[Marco Zehe (:MarcoZ)]

(In reply to Joe Watkins from comment #8)
> ARIA sure saves the day for many things but only for assistive technology
> that supports it. We can reach farther back with simple text that is within
> a link to define the link's accessible name. It sounds edge case, but
> accessibility is all about reaching as many people as possible and lots of
> people with disabilities leverage older technology. I recently lead a
> usability study with people that had disabilities, and one of them was using
> JAWS 12 w/IE11 - and there wasn't any ARIA support for that user.

That is not strictly true. JAWS 12, which was released in 2010, by the way, did support some ARIA. Even JAWS 10 had initial support for landmarks already IIRC. But fact is that a JAWS 12 user is not able to use current versions of Firefox any more. For technical reasons, Firefox 57 and up only supports JAWS 2018 and newer, and the last currently supported version of Firefox that would, in theory, still run with JAWS 12 is ESR 52, which is going to be end-of-lifed in two months from now.

I'd really hate it if we put in some hacky workarounds for this old technology. Things move on, and so does technology in the accessibility space.

Comment 11

[Joe Watkins]

Hi Marco, 

In this case, JAWS 12 didn't support aria-hidden="true" for the participant in the usability study and received quite a different experience.

Your description is accurate of fragmented FF/JAWS support and that technical reason could be the exact reason why some people would be hesitant to move on with the rest of the fortunate super power users who have access to the latest and greatest. Why would I want to upgrade if all I hear is don't do it!!.. or if I upgrade now, I'm gonna have to shell out $1300 for this version of JAWS that works with the latest version of FF!

What do you recommend instead of visually hidden text so that 'read more' link purpose is meaningful? The new school fancy pants approach of aria-label is also stripped in Reader Mode. Ideally, the link itself would be meaningful but in this case it can't.

Your team doesn't have to make this fix.. it may be easier for us out here in the accessibility community to just throw out yet another warning about how screen readers may not behave consistently w/FF.

Comment 12

[Marco Zehe (:MarcoZ)]

(In reply to Joe Watkins from comment #11)
> Your team doesn't have to make this fix.. it may be easier for us out here
> in the accessibility community to just throw out yet another warning about
> how screen readers may not behave consistently w/FF.

Let me get this straight: Reader Mode is intended to give users a better reading experience with articles. Pages with a bunch of "Read More" links will probably not even have Reader Mode offered, since the patterns don't match and Firefox doesn't think it can improve the reading experience in such cases. If an *article* itself has a bunch of hidden link texts embedded, it should be questioned whether having that un-hidden isn't desirable anyway for even more users than just screen reader users, since it no doubt improves general legibility. And thus, the link text should not be hidden at all in the first place.

However, since we do have a patch that fixes the problem you were raising, and we want to be good citizens and go with the techniques used often in the community, I am grudgingly OK with accepting this fix.

Comment 13

[Eitan Isaacson [:eeejay]]

(In reply to Joe Watkins from comment #11)
> What do you recommend instead of visually hidden text so that 'read more'
> link purpose is meaningful? The new school fancy pants approach of
> aria-label is also stripped in Reader Mode. Ideally, the link itself would
> be meaningful but in this case it can't.
>

That is a legitimate bug, aria attributes should not be stripped.

Comment 14

[Eitan Isaacson [:eeejay]]

(In reply to Joe Watkins from comment #11)
> What do you recommend instead of visually hidden text so that 'read more'
> link purpose is meaningful? The new school fancy pants approach of
> aria-label is also stripped in Reader Mode. Ideally, the link itself would
> be meaningful but in this case it can't.

Basic aria has been supported by browsers and screen readers for 10 years, it's hardly new.
For all the reasons I mentioned above, I would use a title or aria-label attribute.

> Your team doesn't have to make this fix.. it may be easier for us out here
> in the accessibility community to just throw out yet another warning about
> how screen readers may not behave consistently w/FF.

Safari strips hidden content in their reader view too.

Comment 15

[:Gijs (he/him)]

(In reply to Marco Zehe (:MarcoZ) from comment #12)
> However, since we do have a patch that fixes the problem you were raising,
> and we want to be good citizens and go with the techniques used often in the
> community, I am grudgingly OK with accepting this fix.

Right, I don't think there's a significant downside to taking this patch, apart from there being alternatives that we'd like to encourage, which is a domain where I don't think reader mode has a lot of power anyway (ie if we cared that much, maybe we should warn in the developer console about bad patterns or something). It's just some CSS. Eitan, that sound OK to you? :-)

(In reply to Eitan Isaacson [:eeejay] from comment #13)
> (In reply to Joe Watkins from comment #11)
> > What do you recommend instead of visually hidden text so that 'read more'
> > link purpose is meaningful? The new school fancy pants approach of
> > aria-label is also stripped in Reader Mode. Ideally, the link itself would
> > be meaningful but in this case it can't.
> >
> 
> That is a legitimate bug, aria attributes should not be stripped.

Yeah, this should be fixed. I wonder how they're getting lost... we don't intentionally strip them in reader mode code. Perhaps the security sanitizer code (which has a list of allowed things rather than forbidden things, because that's how you write that type of code...) might be stripping it. Anyway, that's fodder for a different bug.

Comment 16

[Eitan Isaacson [:eeejay]]

Comment on attachment 8986204 [details]
Bug 1464858 - make visually hidden / screenreader content not display:none,

https://reviewboard.mozilla.org/r/251620/#review258590

Comment 17

[Eitan Isaacson [:eeejay]]

(In reply to :Gijs (he/him) from comment #15)
> (In reply to Eitan Isaacson [:eeejay] from comment #13)
> > (In reply to Joe Watkins from comment #11)
> > > What do you recommend instead of visually hidden text so that 'read more'
> > > link purpose is meaningful? The new school fancy pants approach of
> > > aria-label is also stripped in Reader Mode. Ideally, the link itself would
> > > be meaningful but in this case it can't.
> > >
> > 
> > That is a legitimate bug, aria attributes should not be stripped.
> 
> Yeah, this should be fixed. I wonder how they're getting lost... we don't
> intentionally strip them in reader mode code. Perhaps the security sanitizer
> code (which has a list of allowed things rather than forbidden things,
> because that's how you write that type of code...) might be stripping it.
> Anyway, that's fodder for a different bug.

Bug 1470229

Comment 18

[Joe Watkins]

Okidokee folks :) Visually hidden text vs. title/aria-label usage is a highly debated topic we can't solve here. 

As a team, you'd surely want to make a decision that aligns with what sits well, and what you advocate for internally/externally around accessibility.

Thanks for the consideration everyone.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b444a5c91bd9
make visually hidden / screenreader content not display:none, r=eeejay

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/b444a5c91bd9

Comment 21

[Joe Watkins]

Comment on attachment 8986204 [details]
Bug 1464858 - make visually hidden / screenreader content not display:none,

https://reviewboard.mozilla.org/r/251620/#review259022