Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#48533 closed defect (bug) (fixed)

TwentyTwenty: Skiplink, author by-line, post date intro text, and comments link reading as one word instead of several while using a screen reader

Reported by: arush's profile arush Owned by: williampatton's profile williampatton
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.3
Component: Bundled Theme Keywords:
Focuses: accessibility Cc:

Description

For the current version of the TwentyTwenty theme, the skiplink, author byline, postdate, and comments link read as one word while using a screen reader. For example, the skiplink reads "skiptothecontent" instead of "skip to the content". "post author" reads as "Postauthor", "post date" reads as "postdate" and "1 comment on hello world!" reads "1 CommentonHelloworld!". Speech has been copied into this ticket. Markup for the skiplink, for example, looks fine, suspecct the problem is in the screen reader text class as there's no aria here to muddy the waters. Tested with: Jaws 2019 and 2020, Firefox 70.0.1, Chrome (latest), NVDA 2019.3, latest Narrator. Behavior is uniform across all. Same goes for the other exmples.

Attachments (1)

twenty twenty breaks screen-reader-text.png (40.6 KB) - added by afercia 4 years ago.

Download all attachments as: .zip

Change History (17)

#1 @williampatton
4 years ago

  • Owner set to williampatton
  • Status changed from new to accepted

I think I have narrowed this down to a problematic piece of CSS and I have been looking for the correct resolution. word-wrap looks like the culprit.

#2 @arush
4 years ago

Looking at the CSS as well, screen reader and trac aren't playing nice so may have accidentally set this to invalid, not sure.

#3 @arush
4 years ago

Changing break-word to normal fixes the issue, but I'm not sure what that does to your visual design. Break-word also appears to have been deppricated according to MDN. https://developer.mozilla.org/en-US/docs/Web/CSS/word-break

#5 @whiskeydragon1
4 years ago

In the style.css file, specifically lines #140, #141, and #1449 the usage of the break-word is being utilized.

Suggest: break-normal, however, this will need to be viewed so nothing breaks visually.

#6 @SergeyBiryukov
4 years ago

  • Component changed from Themes to Bundled Theme
  • Milestone changed from Awaiting Review to 5.3

#7 follow-up: @arush
4 years ago

Yes this is about the TwentyTwenty theme. Should I have filed this on GitHub? I can patch this because it's a simple fix, but not sure where to submit the patch. Waiting on feedback from Will or Carolina.

#8 in reply to: ↑ 7 @williampatton
4 years ago

Replying to arush:

not sure where to submit the patch

Submitting it here is perfectly fine if that is what is easiest for you to do. I can cross post and move things back and forth wherever they may need to end up at.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

#10 @afercia
4 years ago

We faced a similar issue in core a few years ago, see [32509].

So yes, any CSS property that alters text spacing / breaking may affect screen readers. Suggest to theck CSS properties inherited by visually hidden text and reset the offending one.

Also: question: is Twenty Twenty using the same screen-reader-text CSS ruleset used in core? It should.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#11 @williampatton
4 years ago

The CSS that is in use for the theme is this:

.screen-reader-text {
	border: 0;
	clip: rect(1px, 1px, 1px, 1px);
	-webkit-clip-path: inset(50%);
	clip-path: inset(50%);
	height: 1px;
	margin: -1px;
	overflow: hidden;
	padding: 0;
	position: absolute !important;
	width: 1px;
	word-wrap: normal !important;
}

.screen-reader-text:focus {
	background-color: #f1f1f1;
	border-radius: 3px;
	box-shadow: 0 0 2px 2px rgba(0, 0, 0, 0.6);
	clip: auto !important;
	-webkit-clip-path: none;
	clip-path: none;
	color: #21759b;
	display: block;
	font-size: 14px;
	font-size: 0.875rem;
	font-weight: 700;
	height: auto;
	right: 5px;
	line-height: normal;
	padding: 15px 23px 14px;
	text-decoration: none;
	top: 5px;
	width: auto;
	z-index: 100000;
}

It comes from the page here originally but it seems there have been some adjustments: https://make.wordpress.org/accessibility/handbook/markup/the-css-class-screen-reader-text/

#12 @afercia
4 years ago

The reason for this bug is simple ans it's basically the same solved in [32509] for word-wrap.

This CSS, specifically word-break as pointed out by @arush :

*,
*::before,
*::after {
	box-sizing: inherit;
	-webkit-font-smoothing: antialiased;
	word-break: break-word;
	word-wrap: break-word;
}

makes the visually hidden text be laid out vertically because the available width is 1 pixel. See screenshot below. In this vertical layout, spaces between words is removed and screen readers read out multiple words as a single word.

I'd also like to note that such rules set on the universal selector target all elements in the page. Using so broad selectors for properties that affect text is dangerous by its own nature and should be avoided.

The simpler solution seems to be resetting word-break in the screen-reader-text class used by the theme.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#13 @arush
4 years ago

For the time being, I would say use the solution that @afercia is suggesting. John and I are going to tackle this from the screen reader angle because there are legitimate reasons to use break-word, this really is a screen reader bug, and while it's fair to ask devs and designers to adhere to WCAG, it's not fair to ask them to accommodate screen reader bugs, even if it's all the screen readers/screen reader magnification combos which have this bug/feature.

#14 @arush
4 years ago

More on the smushed character thing from one of the Facebook accessibility team members, might be useful for us. https://medium.com/@jessebeach/beware-smushed-off-screen-accessible-text-5952a4c2cbfe

Version 2, edited 4 years ago by SergeyBiryukov (previous) (next) (diff)

#15 @williampatton
4 years ago

I have created a PR on github that I will merge shortly which works to resolve this by resetting word-break on the .screen-reader-text class.

I have noted that the text adjustments done as part of a CSS reset in the theme on universal elements could be problematic (as noted in https://core.trac.wordpress.org/ticket/48533#comment:12) and will investigate what could be done there.

PR at github for reference: https://github.com/WordPress/twentytwenty/pull/988

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#16 @SergeyBiryukov
4 years ago

  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from accepted to closed

Thanks everyone! The PR linked above was merged as part of [46701] for #48557.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.