Format Library: Link aria-label is assigned wrong text

[aduth]

Introduced in #11063
Previously: #11815 (comment)

Steps to reproduce:

1. Navigate to Posts > Add New
2. Add a paragraph with at least a few words
3. Apply a link to some but not all of the text, enabling the open in New Tab option
4. Verify generated HTML in Code Editor

Expected:

<!-- wp:paragraph -->
<p>this is a <a href="http://example.com" target="_blank" rel="noreferrer noopener" aria-label="test (opens in a new tab)">test</a></p>
<!-- /wp:paragraph -->

Actual:

<!-- wp:paragraph -->
<p>this is a <a href="http://example.com" target="_blank" rel="noreferrer noopener" aria-label=" (opens in a new tab)">test</a></p>
<!-- /wp:paragraph -->

(the text of the link, with "(opens in new tab)" appended as a suffix)

Observations:

• At Caret (arrow) followed by a text link breaks content #11815 (comment), the aria-label appeared to be assigned to the text of the parent paragraph
• In my Steps to Reproduce here, the text generated appears to be empty

Relevant code:

gutenberg/packages/format-library/src/link/inline.js

Lines 52 to 59 in 563ef41

	if ( opensInNewWindow ) {
	// translators: accessibility label for external links, where the argument is the link text
	const label = sprintf( __( '%s (opens in a new tab)' ), text ).trim();
	format.attributes.target = '_blank';
	format.attributes.rel = 'noreferrer noopener';
	format.attributes[ 'aria-label' ] = label;
	}

cc @ryanwelcher @mcsf

[ellatrix]

I cannot reproduce this. I get the expected result. Do you have more specific steps to reproduce?

[aduth]

I cannot reproduce this. I get the expected result. Do you have more specific steps to reproduce?

It does seem to be inconsistent. One thing which seems to be more reliable is if the "Opens in New Tab" option isn't checked until after the link is already applied, and after it has been deselected at least once before.

1. New Post
2. Insert paragraph
3. Write "This is a test"
4. Highlight "test"
5. Click Add Link
6. Type "http://example.com"
7. Press Enter
8. Move focus elsewhere, even within the same paragraph
9. Click on link
10. Click link ellipsis menu
11. Click "Open in New Tab"
12. Change to Code Editor

[ellatrix]

Ah, this is probably because the label is added when there is no selection. I'll have a look.

[ellatrix]

Not immediately sure how to fix this problem. We'll probably have to pass an extra property activeFormatText to the format edit component, Not an ideal solution as it will need to be calculated on every rerender.

[ellatrix]

Which is probably ok.

[afercia]

Worth noting, as reported in #12683, special characters used in the link text e.g. > are re-used in the aria-label attribute without escaping, thus breaking the markup.

In #12154 it was suggested to use a screen-reader-text appended to the link content instead of an aria-label. I'd still think that would be a preferable option.

[therevolver]

I was wondering if there was any temporary fix for this other than going into the code editor and removing the aria label part?

[LukaszJaro]

Worth noting, as reported in #12683, special characters used in the link text e.g. > are re-used in the aria-label attribute without escaping, thus breaking the markup.

In #12154 it was suggested to use a screen-reader-text appended to the link content instead of an aria-label. I'd still think that would be a preferable option.

There's known issues with using screen-reader-text appended to the link (reading out-of-order): https://medium.com/@svinkle/why-let-someone-know-when-a-link-opens-a-new-window-8699d20ed3b1

There's also another issue with the AODA text accuracy, shouldn't the text be more informative depending on where a link opens to? Example:

Opens in new window (internal site)
Opens an external site in a new window (external site)

[afercia]

@LukaszJaro thanks for your feedback. The article you pointed at omits to mention the "out of order" issue currently happens only with Safari + VoiceOver, and only in some specific cases. It's a user agent bug that should be fixed upstream (cough cough, Apple?).

That's not to say other, better, options can't be considered. It's just to clarify the actual impact of a potential use of the screen-reader-text CSS class.

I had to google for "AODA" to understand what it is. Please, when using abbreviations, try to expand them on first use or just avoid them 🙂Not everyone is required to know specific countries laws and Acts.

That said, I'm a bit surprised this issue is still open. @aduth when you have a chance: how can I help move this issue on?

[LukaszJaro]

@afercia, woops about the AODA(Accessibility for Ontarians with Disabilities Act) applies to Canada - thanks for pointing that out :)

I hope a resolution comes soon to this (WP 5.3), each project I do, all of the accessibility experts mention the lack of notification about links opening in new tabs.

[mcsf]

That said, I'm a bit surprised this issue is still open. @aduth when you have a chance: how can I help move this issue on?

Hey, @afercia and @LukaszJaro. Andrew is on extended leave. Do you know of anyone who could grab this one?

[ellatrix]

The current implementation is a bit problematic. Even if we set the correct text when editing a link through an uncollapsed selection (fixing this bug), it's possible that the text in the attribute diverges from the actual text. Imagine the following situation:

1. You link a selection and enable "open in a new tab". The HTML would be as follows:

<a ... aria-label="some text (opens in a new tab)" ...>some text</a>

2. You now select “text” and perhaps change the link URL and disable “open in a new tab”. The HTML would be as follows:

<a ... aria-label="some text (opens in a new tab)" ...>some </a>
<a ...>text</a>

Notice the incorrect aria-label attribute. applyFormat doesn’t know that it needs to change any surrounding links. And to be honest, it shouldn’t.

There are many more cases where the text will diverge, because the data is duplicated.

Imagine another situation:

1. You again link a selection and enable "open in a new tab". The HTML would be as follows:

<a ... aria-label="some text (opens in a new tab)" ...>some text</a>

2. You place the caret in the middle of the text and type. The HTML would be as follows:

<a ... aria-label="some text (opens in a new tab)" ...>some extra text</a>

Notice the incorrect aria-label attribute. We don’t have any logic in place on the input event to update format attributes.

In my opinion, we should try to avoid duplicating data and not use the text value in attributes. I believe @afercia mentioned other techniques such as setting a screen reader text. Unfortunately, it’s not so simple to embed additional elements in format elements either, but maybe it’s worth a try. If we do have to use an attribute, perhaps we have to use some sort of placeholder like %text% which gets replaced when converted to HTML or DOM nodes.



Thoughts?

[afercia]

Question: would adding the visually hidden text via PHP on post content save be an option? Much like WordPress adds the rel="noopener noreferrer" to links with target="_blank". See wp_targeted_link_rel() and wp_init_targeted_link_rel_filters().

Aside: I think those should be improved to also remove the rel attribute when target="_blank" gets removed. Same thing should be done for the visually hidden text.

[ellatrix]

Sure, I think a display filter for this would be ideal. It also makes sense for the rest of the text "(opens in a new tab)" as it should probably be dynamically translated.

[MarcoZehe]

There's one more problem with the current approach, which I ran into and documented in #18727 . The language that should be used is not that of the current UI language, but that of the site the article ges published to. In my case, I got German text embedded into the aria-label of a link in an English post, which totally does not make sense. So whatever solution is found for this, it should take into account the actual target language of the site, not the current user. Some of us speak more than one language. ;-)

[ellatrix]

I'm considering removing the aria-label until we find a proper fix.

This whole things seems strange to me. This text should be something for the user agent to add, not the content creator. Ideally it shouldn't be inserted in the source at all. We also can't make it perfect. The user agent will know the language the user has set, which is not necessarily the language of the web page. The user agent will also know if the link opens in a new tab or window.

Safari does this well it seems:

If we really want to do this, we should append screen reader text in PHP display filters, but we lack context (browser language and tab/window setting).

The last place it should be added is to the source content, if we want the label to be consistent.

[joedolson]

Discussed in this in the accessibility meeting on 29 November 2019, and we agree that removing the aria-label is a good temporary solution. Using aria-label creates problems with language consistency, text stability after editing, and doesn't provide accessibility for anybody except screen reader users. We'd like to find a new solution that takes these factors into consideration.

Allowing the user agent to provide this would be theoretically great, except for the lack of cross-browser support.

One discussed suggestion is having this text inserted in PHP on render or on post save. (Render would make it easier to be consistent with the display language, we think.)

[aardrian]

I opened #20129 (now closed as a duplicate of this) because the aria-label is not being auto-translated for users who visit a page in a language other than their own.

@MarcoZehe raised the issue as an author getting the wrong language when creating links in #18727.

Add the problem(s) referenced in this issue, and I feel aria-label should not be used (as @ellatrix and @joedolson suggest). Removing aria-label should not wait for a PHP solution to be in place.

A short-term approach that does not override the accessible name could be a CSS hack in the 2020 theme:

main a[target^="_"]::after {
	content: " (opens in new window/tab)";
}

Even when it does not auto-translate, it does not prevent the entire link from translating.