Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 6 years ago

#36572 closed enhancement (fixed)

Read more link more accessible

Reported by: travel_girl's profile Travel_girl Owned by: kau-boy's profile Kau-Boy
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch 2nd-opinion dev-feedback
Focuses: accessibility, template Cc:

Description

Dear Core Team,

I just noticed, that the default read more link for the excerpt of a post is not accessable for screen readers. By reading a site with a screen reader you can say - read all the links. At the moment the screen reader only say "...", "...", "...". The most themes has "read more" instead of "...", but the user still does not know, what is behind that link.

The twenty themes added a screen reader class, but the most themes don´t have it.
We could add this as required for themes, but this has only an effect for the themes in the directory, not other premium themes.

I hope there is a solution, to make every "Read More" link accessible directly in the core (maybe via aria???)

Thank you so much!

Attachments (11)

36572.diff (597 bytes) - added by Kau-Boy 8 years ago.
patch using the CSS class
36572-inline-styles.diff (743 bytes) - added by Kau-Boy 8 years ago.
patch using inline styles
36572.2.diff (1.8 KB) - added by flixos90 8 years ago.
improve the excerpt more text as well
36572.3.diff (1.8 KB) - added by flixos90 8 years ago.
added space before span
36572-link-only.diff (590 bytes) - added by Kau-Boy 8 years ago.
36572-link-and-excerpt.diff (1.6 KB) - added by Kau-Boy 8 years ago.
36572-link-and-excerpt-linked.diff (1.7 KB) - added by Kau-Boy 8 years ago.
36572-link-and-excerpt-linked.2.diff (1.8 KB) - added by ramiy 8 years ago.
36572-link-only.2.diff (659 bytes) - added by Kau-Boy 8 years ago.
36572-link-and-excerpt.2.diff (1.8 KB) - added by Kau-Boy 8 years ago.
36572-link-and-excerpt-linked.3.diff (1.8 KB) - added by Kau-Boy 8 years ago.

Download all attachments as: .zip

Change History (39)

#1 @swissspidy
8 years ago

  • Component changed from Posts, Post Types to Themes

#2 @flixos90
8 years ago

  • Component changed from Themes to Posts, Post Types

Maybe we could change the default values for $more_link_text in get_the_content() and for the internal $excerpt_more variable in wp_trim_excerpt().

To make the change more backwards-compatible, we might only apply this new default if the filtered value is still the same like the value before applying filters.

Also, for excerpts, by default there isn't even a link, so I wonder whether we should add that as well.

#3 @flixos90
8 years ago

  • Component changed from Posts, Post Types to Themes

@swissspidy Sorry, I didn't want to undo your change :)

#4 @Kau-Boy
8 years ago

  • Keywords has-patch 2nd-opinion added

I wrote two possible patches for this enhancement. The main problem is not to break current theme. As the core string is "(more...)", we can't just add a string before or after that, as it would not make sense for a screen reader.

The Twentyfifteen theme uses "Continue reading [title of the post]", which makes perfectly sense. So I would suggest to keep the current string for themes without the $more_link_text defined. This default string is wrapped in a span using the aria-hidden attribute to prevent screen readers reading it.

As new themes must have the CSS class screen-reader-text we should be able to use this class. If a wider backward compatibility is desired, the second patch could be used, which adds the CSS properties inline to the span.

@Kau-Boy
8 years ago

patch using the CSS class

@Kau-Boy
8 years ago

patch using inline styles

@flixos90
8 years ago

improve the excerpt more text as well

#5 @flixos90
8 years ago

36572.2.diff adjusts the code in wp_trim_excerpt() as well, based on the first patch, furthermore making the text a link. For backwards compatibility, it only changes in case the filter is not already used by a theme to address this issue.

#6 follow-up: @SergeyBiryukov
8 years ago

  • Milestone changed from Awaiting Review to 4.6

36572.2.diff seems good, the only thing I'd like to add is a space before <span aria-hidden="true"> in case the .screen-reader-text class is not defined.

@flixos90
8 years ago

added space before span

#7 in reply to: ↑ 6 @flixos90
8 years ago

Replying to SergeyBiryukov:

36572.2.diff seems good, the only thing I'd like to add is a space before <span aria-hidden="true"> in case the .screen-reader-text class is not defined.

36572.3.diff has the space before the span attribute.

#8 @swissspidy
8 years ago

Having had a quick look:

  • I'd use sprintf for the whole link, not just the link text. Makes it a bit easier to read instead of doing sprintf() + string concat.
  • Ideally, HTML tags shouldn't be part of a translatable string.

After that, and feedback from the a11y team, +1 from me.

#9 @afercia
8 years ago

As an alternate approach maybe worth considering aria-label. Differences between the two solutions:

  • aria-label: is standard, cleaner, it completely overrides the element's text (i.e. the original text is not read out at all)
  • screen-reader-text: is basically a hack, requires styling and should be used only when there's the need to add something to an existing text

Recently, aria-label usage in the admin has greatly increased, see for example the "row action" links in the Posts screen

<a href="(edit link here...)" aria-label="Edit “Hello world!”">Edit</a>
Last edited 8 years ago by afercia (previous) (diff)

#10 @Kau-Boy
8 years ago

@afercia That sounds like a great idea and we wouldn't need to add inline CSS. I'll prepare a patch for that.

#11 @Kau-Boy
8 years ago

I've wrote some patches for the idea from @afercia. I am not quite sure about linking the "hellip" in the excerpt, as the excerpt might be used as plain text which would print the HTML tags. This would not be backward compatible.

I also don't see, how the excerpt would benefit from the aria-label when it's not a link. So I created three patches, for all three variants. I would suggest the "link-only" patch. But maybe someone from the a11y team can has a 2nd opinion on that.

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


8 years ago

#13 @ramiy
8 years ago

@Kau-Boy, you need to avoid using HTML tags in translation strings.
I fixed your last patch. See the changes.

#14 @Kau-Boy
8 years ago

I have read the discussion on the #accessibility channel about this ticket. Any thoughts about which patch to use from the a11y team? I would still tend to the first patch that only changes the "more link" only.

#15 @Kau-Boy
8 years ago

@ramiy you are right. The translatable string do not need HTML (my previous version needed them). Thanks for the tip!

But your patch had some small syntax errors. I updated my three patch variants with your suggested improvement.

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


8 years ago

#17 @karmatosed
8 years ago

  • Component changed from Themes to Bundled Theme

#18 @ocean90
8 years ago

  • Component changed from Bundled Theme to Themes
  • Focuses template added

Moving back to themes component since this is about the default values for the template tags.

#19 @chriscct7
8 years ago

  • Owner set to Kau-Boy
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core by kau-boy. View the logs.


8 years ago

#21 @Kau-Boy
8 years ago

  • Keywords dev-feedback added

Any thoughts on the patches from one of the a11y team? I still think 36572-link-only.2.diff would be the right way to go. Let's get this ticket finished with WP 4.6 :)

This ticket was mentioned in Slack in #accessibility by kau-boy. View the logs.


8 years ago

This ticket was mentioned in Slack in #accessibility by kau-boy. View the logs.


8 years ago

#24 @SergeyBiryukov
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 37706:

Themes: Make default "read more" link more accessible.

Add an aria-label with the post title to make it available for screen readers.

Props Kau-Boy.
Fixes #36572.

#25 @davidakennedy
8 years ago

Just wanted to pop in and say that @Kau-Boy asked for testing from the accessibility team on this. I didn't get to test until this morning, but everything looks good.

I tested with two themes that do not alter the default more output and everything worked as expected. I also tested with a handful of default themes that do alter the markup. I saw no issues there either.

Looks good – thanks for all the work!

#26 @Travel_girl
8 years ago

Awesome! I want to thank all of you, who was working on this ticket! I'm so glad, that WordPress becomes more accessible :-)

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


8 years ago

This ticket was mentioned in Slack in #accessibility-docs by travelgirl. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.