WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 2 weeks ago

#47670 reviewing defect (bug)

RSS widget creates an accessibility problem when used more than once

Reported by: tpaw Owned by: audrasjb
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch 2nd-opinion has-screenshots
Focuses: accessibility Cc:

Description

Please consider the following patch to improve accessibility.

Accessibility guidelines in WCAG's standard 2.4.4 Link Purpose requires that link text should provide a purpose or context for the link.

In the RSS widget, the link text for the link to the RSS feed itself is an image of the RSS icon; its alt text is "RSS" which programmatically determines the link text. This passes the referenced standard.

A problem occurs when multiple instances of the RSS widget are used. There are then multiple links with link text "RSS", each of which lead to different URLs. The text "RSS" then does not provide enough context for visitors to know what each "RSS" link leads to.

The solution is the make each RSS link text unique to each feed's title, thus providing the necessary context.

Simply prepending the title of the widget instance adds the necessary context to the link text without using any additional words which could has i18n issues.

In class-wp-widget-rss.php (lines 89-91 in 5.2.2):

From:

<?php
// lines 89-91 in wp-includes/widgets/class-wp-widget-rss.php (WP 5.2.2)

if ( $title ) {
        $title = '<a class="rsswidget" href="' . esc_url( $url ) . '"><img class="rss-widget-icon" style="border:0" width="14" height="14" src="' . esc_url( $icon ) . '" alt="RSS" /></a> <a class="rsswidget" href="' . esc_url( $link ) . '">' . esc_html( $title ) . '</a>';
}

To:

<?php
if ( $title ) {
        $title = '<a class="rsswidget" href="' . esc_url( $url ) . '"><img class="rss-widget-icon" style="border:0" width="14" height="14" src="' . esc_url( $icon ) . '" alt="' . esc_html( $title ) . ' RSS" /></a> <a class="rsswidget" href="' . esc_url( $link ) . '">' . esc_html( $title ) . '</a>';
}

No harm is done when only a single instance of the RSS widget is used because the RSS link text is simply more explicit.

Attachments (7)

47670.diff (966 bytes) - added by mukesh27 6 weeks ago.
Patch.
before-patch-feed-with-title.png (156.2 KB) - added by audrasjb 2 weeks ago.
Before 47670.1.diff: feed with title
before-patch-feed-without-title.png (160.4 KB) - added by audrasjb 2 weeks ago.
Before 47670.1.diff: feed without title
after-patch-feed-with-title.png (152.6 KB) - added by audrasjb 2 weeks ago.
After 47670.1.diff: feed with title
after-patch-feed-without-title.png (157.3 KB) - added by audrasjb 2 weeks ago.
After 47670.1.diff: feed without title
47670.1.diff (965 bytes) - added by audrasjb 2 weeks ago.
Remove double link in Widget RSS
Capture d’écran 2019-08-02 à 19.48.23.png (186.3 KB) - added by audrasjb 2 weeks ago.
Testing 47670.1.diff with another bundled theme

Download all attachments as: .zip

Change History (14)

#1 @tpaw
6 weeks ago

  • Summary changed from RSS widget creates and accessibility problem when used more than once to RSS widget creates an accessibility problem when used more than once

#2 follow-up: @mukesh27
6 weeks ago

  • Keywords has-patch 2nd-opinion added

Hi @tpaw,

Welcome to WordPress Trac! Thanks for the ticket.

I think we have to remove that alt tag from RSS image because if anyone does not use feed title then image alt display space before RSS text, also RSS was not used any escaping function.

Remove alt text from the widget is a better solution.

@audrasjb can you check this?

@mukesh27
6 weeks ago

Patch.

#3 @audrasjb
6 weeks ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

Yes added to my list!

#4 in reply to: ↑ 2 @tpaw
3 weeks ago

Replying to mukesh27:

I think we have to remove that alt tag from RSS image because if anyone does not use feed title then image alt display space before RSS text, also RSS was not used any escaping function.

Remove alt text from the widget is a better solution.

@audrasjb can you check this?

Thank you for reviewing this.

Unfortunately, removing the alt text means that one will then have an "empty" link, as it would effectively become an <a href="url"></a> in the accessibility DOM.

Links must have some sort of link text, whether it's regular text or alt text from an image, so your proposed solution will still violate WCAG accessibility guidelines.

By leaving the user-specified title in, at least there is a chance and ability for users to correctly adhere to WCAG. Without any link text, it's still basically the same problem of not meeting a11y guidelines.

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


2 weeks ago

#6 @afercia
2 weeks ago

Related: #46978.

@audrasjb
2 weeks ago

Before 47670.1.diff: feed with title

@audrasjb
2 weeks ago

Before 47670.1.diff: feed without title

@audrasjb
2 weeks ago

After 47670.1.diff: feed with title

@audrasjb
2 weeks ago

After 47670.1.diff: feed without title

@audrasjb
2 weeks ago

Remove double link in Widget RSS

#7 @audrasjb
2 weeks ago

  • Keywords has-screenshots added
  • Milestone changed from Awaiting Review to 5.3

In 47670.1.diff:

  • Use only one link for both image and title of the RSS feed
  • Use an empty alt attribute since we doesn't need it anymore

Ping @afercia for code review.

Also moving this ticket to 5.3 milestone.

@audrasjb
2 weeks ago

Testing 47670.1.diff with another bundled theme

Note: See TracTickets for help on using tickets.