Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 22 months ago

#55718 closed defect (bug) (fixed)

Make sure the aria-label for the "Logged in as" link in comment form is containing the same string as the visible text

Reported by: juliemoynat's profile juliemoynat Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 5.9.3
Component: Comments Keywords: has-patch has-screenshots commit add-to-field-guide
Focuses: accessibility Cc:

Description

Hi,

In the comment template, for the "Logged in as" link, there is an accessibility problem for speech-input users (WCAG failure to Label in name criterion).

Actually, in English, we have two strings:

  • Logged in as %s. Edit your profile.
  • <a href="%1$s" aria-label="%2$s">Logged in as %3$s</a>. <a href="%4$s">Log out?</a>

But, in the French translation (and I think the error may be present in other translations too), we have this:

<a href="/wp-admin/profile.php" aria-label="Connexion en tant que pseudo. Modifier votre profil.">Connecté en tant que pseudo</a>

The aria-label is different from the visible text and is using different translations for "Logged in" ("Connexion" and "Connecté") while it must be the same.

I think it's not really a translation problem. I think that, by development, we should take the visible text string and put it into the aria-label string to translate it only once and prevent errors.

Attachments (5)

55718.diff (1.4 KB) - added by SergeyBiryukov 2 years ago.
55718.2.diff (2.3 KB) - added by lopo 2 years ago.
Capture d’écran 2022-07-22 à 09.53.47.png (72.2 KB) - added by audrasjb 2 years ago.
Before patch
Capture d’écran 2022-07-22 à 09.54.04.png (78.8 KB) - added by audrasjb 2 years ago.
After patch
c564d50102ab758b68590cf6a8b8ada6.gif (78.0 KB) - added by audrasjb 2 years ago.
After patch, with links hovering

Download all attachments as: .zip

Change History (25)

#1 @juliemoynat
2 years ago

  • Component changed from Comments to I18N

@SergeyBiryukov
2 years ago

#2 @SergeyBiryukov
2 years ago

  • Keywords has-patch added

Hi there, welcome back to WordPress Trac! Thanks for the ticket.

Adding a patch with what I think a fix could look like.

#3 @joedolson
2 years ago

What purpose does this aria-label attribute actually serve? If there's visible text inside the link, I can't see any benefit to retaining the aria-label here.

#4 follow-up: @joedolson
2 years ago

This was introduced in [34525]. It looks like the intent was to provide greater context for screen readers than the default text offered; but I think that's not necessary or helpful. As long as the link's text provides meaningful linking content, the aria-label is unnecessary.

I think we should remove aria-label entirely in this context.

#5 in reply to: ↑ 4 @SergeyBiryukov
2 years ago

Replying to joedolson:

This was introduced in [34525]. It looks like the intent was to provide greater context for screen readers than the default text offered; but I think that's not necessary or helpful. As long as the link's text provides meaningful linking content, the aria-label is unnecessary.

I think the "Edit your profile" part was the point here, per comment:20:ticket:29974 and comment:23:ticket:29974:

that link points to your profile but the link text is your display name, this could be confusing when read out of context. To be honest, I think it's not so clear also for sighted users :)

Does "Edit your profile" in the label makes the link a bit less confusing?

#6 @juliemoynat
2 years ago

If the link label is confusing, here, it's a problem for everyone, not only people using a screen reader. Even if you read the link in its context, there is no information to know the link allows you to modify your profile (there's a heading "Leave a comment" before and then a "Logout" link in the same paragraph + the required fields text, that's all).

It's a UX problem trying to be solved through accessibility spectrum.

If the link label needs clarification, I think it should be for everyone (or no one).

We could have <a href="">Logged in as PSEUDO (Edit your profile)</a> without aria-label. Same label for everyone.

What do you think?

@lopo
2 years ago

#7 @lopo
2 years ago

During the Yoast Contributor Day, we discussed about this ticket and came out with a new patch.
The idea is that the link should just convey the information about its purpose ("Edit your profile") while the information "Logged in as Jane Doe" should just be simple text.

Props to @afercia, @SergeyBiryukov, @poena, @aristath

#8 @juliemoynat
2 years ago

I like this idea!

#9 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.1

#10 @audrasjb
2 years ago

  • Keywords has-screenshots added

The two above screenshots show the difference before/after applying the patch. It looks pretty good to me.

That being said, there's an issue in Twenty Twenty: links are not identified. I'll open a ticket for that :D

In the meantime, I'll share a video screenshot below, so we can validate the implementation :)

@audrasjb
2 years ago

After patch, with links hovering

#11 @audrasjb
2 years ago

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

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


2 years ago

#13 @joedolson
2 years ago

Accessibility team agrees that this would be a good solution. Let's move it forward!

The related Twenty Twenty ticket is #56269.

#14 @audrasjb
2 years ago

  • Keywords commit added

Great! Let's roll!

#16 @audrasjb
2 years ago

Added a PR just to make sure unit tests are passing.

#17 @audrasjb
2 years ago

  • Component changed from I18N to Comments

#18 @audrasjb
2 years ago

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

In 53796:

Comments: Improve accessibility of the "Logged in as" link in comment form.

This changeset ensures the "Logged in as" link conveys the information about its purpose ("Edit your profile") while the information "Logged in as Jane Doe" is only a simple text.

Props juliemoynat, SergeyBiryukov, joedolson, audrasjb, lopo.
Fixes #55718.

#20 @milana_cap
22 months ago

  • Keywords add-to-field-guide added
Note: See TracTickets for help on using tickets.