Make WordPress Core

Opened 13 years ago

Closed 10 years ago

Last modified 10 years ago

#16433 closed enhancement (fixed)

Extend function to optionally include commenter name in comment_reply_link

Reported by: elpie's profile Elpie Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.1 Priority: low
Severity: normal Version: 3.0.4
Component: Comments Keywords: has-patch
Focuses: accessibility Cc:

Description

Screen readers recognize links and make them searchable. Unique names facilitate that search. Links need to have unique names that are descriptive and make sense when read out of context. WordPress allows users to set the text they want to display on the comment_reply_link but then this text is repeated for every comment on a post.

WCAG Checkpoint 13.1 says: Clearly identify the target of each link. [Priority 2]

For improved accessibility it is far better to have the comment_reply_link include the commenter name. "Reply to Tom Thumb" and ideally, include a title on the link that includes both the commenter name and comment number. Users should have the option to filter for their custom text and to include (or not include) the commenter name.

At the moment, anyone who wants to present an accessible front-end cannot use threaded comments unless they are prepared to generate masses of the same, generic, unhelpful link.

Attachments (4)

16433.diff (527 bytes) - added by andrewryno 13 years ago.
16433.patch (1.2 KB) - added by merty 13 years ago.
16433.2.diff (1.2 KB) - added by merty 12 years ago.
This should be the proper way to do it
16433.3.patch (1.2 KB) - added by joedolson 10 years ago.
Uses aria-label to provide name of commenter.

Download all attachments as: .zip

Change History (26)

#1 @Elpie
13 years ago

Somewhat related to #10569.

#2 @scribu
13 years ago

  • Cc scribu added

@andrewryno
13 years ago

#3 @andrewryno
13 years ago

Possible fix. Just gives the user a %s option in 'reply_text' which will add the persons username. Not sure if someone wants to change this to allow different tags (username, first name, last name, nickname, etc.).

#4 @markjaquith
13 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Priority changed from normal to low
  • Type changed from defect (bug) to enhancement

Like that idea, andrewryno.

I had to use this nonsense to do it!

function txfx_reply_to_comment_link( $link, $args, $comment ) {
	if ( !empty( $comment->comment_type ) )
		return '';
	$link = str_replace( '>' . $args['reply_text'] . '<', '>' . esc_html( 'Reply to ' . $comment->comment_author ) . ' &rarr;<', $link );
	return $link;
}

add_filter( 'comment_reply_link', 'txfx_reply_to_comment_link', 10, 3 );

#5 @Elpie
13 years ago

Thanks andrewryno. This will make a significant difference to frontend accessibility for the average user. Accessibility out of the box is far preferable to having to add custom functions on a theme by theme basis. Cheers.

This is a trivial change - any chance it can get into 3.1?

#6 @markjaquith
13 years ago

No chance for 3.1. But 3.2 dev should start soon.

@merty
13 years ago

#7 @merty
13 years ago

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

Applied a patch to the current repository. Also the patch can be found in the attachments section of the ticket.

Last edited 13 years ago by merty (previous) (diff)

#8 @Elpie
13 years ago

Is there any chance of this getting into 3.2 or has it missed the boat again?

#9 @Elpie
13 years ago

  • Cc Elpie added

#10 @merty
13 years ago

  • Cc merty92@… added

#11 @SergeyBiryukov
13 years ago

  • Keywords has-patch added

@merty
12 years ago

This should be the proper way to do it

#12 @scottsweb
11 years ago

  • Cc scottsweb added

Due to the useless / repetitive nature of title attributes when used in screen readers (discussed in #24766), it may not be best to proceed with adding a title attribute for this ticket.

If this was within wp-admin we could alter the actual anchor text to include both the commenter name and comment number, then wrap that new text within <span class="screen-reader-text">. For the front end though it would require themes support the screen-reader-text class along with http://codex.wordpress.org/CSS#WordPress_Generated_Classes which we cannot guarantee at the moment.

#13 @nacin
11 years ago

  • Component changed from Accessibility to Comments
  • Focuses accessibility added

@joedolson
10 years ago

Uses aria-label to provide name of commenter.

#14 follow-up: @joedolson
10 years ago

Added a new patch. This patch uses aria-label; in supporting screen readers, this means that the full text of the aria-label will be used in generated lists of links, although older screen readers will still only get 'Reply'.

#15 in reply to: ↑ 14 @GaryJ
10 years ago

Replying to joedolson:

Added a new patch. This patch uses aria-label; in supporting screen readers, this means that the full text of the aria-label will be used in generated lists of links, although older screen readers will still only get 'Reply'.

The 'Reply' comes from one of the default args that can be overridden with passed-in args. Should your aria-label use this default (instead of hard-coding 'Reply'), or should a new reply_text_extended (or some named variant) be added as a default arg so it too can be overridden?

#16 follow-up: @joedolson
10 years ago

Maybe...two considerations there:

1) that would potentially lead to translation challenges with the connecting term
2) if the theme passes a non-word value, the label could lose meaning.

Those are risks with using the default; but adding another default arg is more reasonable, although it provides themes with an opportunity to decrease the accessibility of their themes in a way I'd prefer not be available as a path, personally.

What do you think?

#17 in reply to: ↑ 16 @GaryJ
10 years ago

Replying to joedolson:

Maybe...two considerations there:

1) that would potentially lead to translation challenges with the connecting term

Agreed. "Reply" and "Reply to ..." may not have the same first word in non-English languages.

2) if the theme passes a non-word value, the label could lose meaning.

Disagree. Theme authors can already filter the whole of the comment reply link anyway (or the comment callback), and I guess they could also filter your new aria-label string on gettext too if they only want to change the "Reply to" bit. So adding it as a default would be more about providing a consistent customisation interface for developers than anything else, allowing them to perhaps reply to comment number (which would be unique), rather than username (which wouldn't, when there are comments by the same author name).

#18 @SergeyBiryukov
10 years ago

  • Milestone changed from Future Release to 4.1
  • Owner changed from merty to SergeyBiryukov

#19 @SergeyBiryukov
10 years ago

16433.3.patch looks good to me.

I don't see a particular benefit in customizing this label, but we also have a customizable 'title_reply_to' label in comment_form(), so I guess it would be consistent.

#20 @SergeyBiryukov
10 years ago

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

In 29822:

Add an aria-label attribute with commenter's name to get_comment_reply_link().

props joedolson, merty, andrewryno.
fixes #16433.

#21 @SergeyBiryukov
10 years ago

In 29823:

Make link construction in get_comment_reply_link() and get_post_reply_link() more readable.

see #16433.

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


10 years ago

Note: See TracTickets for help on using tickets.