Remove title attributes: comment-template.php

[joedolson]

Related: #24766

// comment-template.php
comments_popup_link()
comment_form()

[joedolson]

comments_popup_link() falls into that awkward ground of "not done right, but impacts themes". Backwards-compatibility obviously has to be an issue. The ideal here would be concealed screen-reader-text so that the post title was available in the link text, but making that change would undoubtedly have a huge visual impact on millions of web sites. Would appreciate second opinions on this; it's a themeable characteristic, so can easily be fixed in a theme, with well-chosen values for $zero, $one, and $more.

comment_form() only uses title attribute for the <abbr> elaboration, as far as I can see; which is acceptable, as far as I'm concerned.

[afercia]

Should apply to get_post_reply_link() too?

[joedolson]

Update patch with screen reader text

[joedolson]

New patch removes title attribute and adds screen reader text to WP core default values.

[jorbin]

I'd like to see us have a canonical blog post on the screen-reader-text class and advice for theme on how to use it (and why they should us it) that we can point people to as a part of this change set. Is there one on the make/accessibility blog already?

[DrewAPicture]

@joedolson: What's the status here?

[joedolson]

I can certainly write something up on the usage of the screen-reader-text class and how to use it; I don't believe there's an existing article on that at make/accessibility, but I could get that written fairly quickly.

I'll try to get it written today and have it reviewed by the accessibility team during their meeting this afternoon. No guarantees on that, however.

[joedolson]

Written, open for comments:

​https://docs.google.com/document/d/1Iy7BJmkpbtdeVisNGGxZg_OcgiSEw0cfl_2NEqnKlDE/edit?usp=sharing

[DrewAPicture]

@joedolson: What's the status of the patch on this ticket? Are there changes yet to make, or is it good to go?

[joedolson]

Well, it's had no feedback; it should probably be reviewed. Nobody has suggested a change, at any rate.

[jorbin]

In 31388:

Use screen reader text instead of a title attribute in comments_popup_link

To better understand screen reader text, check out ​https://make.wordpress.org/accessibility/2015/02/09/hiding-text-for-screen-readers-with-wordpress-core/

Screen Reader text improves the user experience for screen reader users. It provides additional context for links, document forms and other pieces of a page that may exist visually, but are lost when looking only at the html of a site. This does change the output of comments_popup_link if you don't pass in values for $zero, $one, $more or $none. Theme authors can and should style <code>.screen-reader-text</code> in ways that are recommended in the above article to hide it visually.

Props joedolson
Fixes #26553

[ocean90]

The strings need some translator comments so translators now what the variables are for.

[jorbin]

Add Translator Comments

[jorbin]

Above patch adds translator comments.

[SergeyBiryukov]

[31389] missed the ticket.

[31388] changed the % in __( '% Comments' ) (later replaced in get_comments_number_text()​) to the actual number. We should either continue to use % and leave the i18n issues for #13651, or use _n() and add number_format_i18n().

26553.2.diff​ does the latter. That still does not resolve #13651 for all themes, just sets a correct default value.

[pento]

26553.3.diff​

We usually put simple HTML into strings for translation. If the order of string elements are being re-arranged in a translated string, it's much easier to make a mistake if you're dealing with placeholders, rather than a tag.

[SergeyBiryukov]

In 31401:

Use _n() in comments_popup_link() when setting the default string to display if there are more than one comment.

see #26553.

[SergeyBiryukov]

In 31402:

Switch to a string placeholder, as number_format_i18n() returns a string.

see #26553.

[joedolson]

Am I correct in thinking that both this ticket is now committed and should be closed as fixed, or are we still considering switching this to incorporate the HTML in the string?

[SergeyBiryukov]

@pento raises a good point in comment:23. Any objections to 26553.3.diff​?

[joedolson]

None from me. The only reason I went with the four argument pattern in the first place was because I thought we were trying to avoid HTML in translations; allowing simple HTML patterns is totally reasonable to me, and certainly makes the job easier to understand for translators and the code more readable.

[DrewAPicture]

Looks like 26553.3.diff​ needs a refresh. Otherwise +1 for commit here.

[jorbin]

Is there a reason to generally eliminate the translator comments? I can count ~100 places in core where we have translator comments for one argument.

[ocean90]

In 31821:

Comments: Move HTML tags for screen reader text into translatable strings.

Placeholders aren't helpful and it's much easier to make a mistake if you're dealing with placeholders.
Introduced in [31388].

props pento.
fixes #26553.