Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format library: link: remove aria-label #18742

Open
wants to merge 3 commits into
base: master
from
Open

Conversation

@ellatrix
Copy link
Member

ellatrix commented Nov 26, 2019

Description

Fixes #12325.

I'm removing the aria-label from user created links as the current implementation is broken. The label text can go out of sync, it doesn't apply if you don't select any text, the language may be incorrect, and it doesn't apply consistently for all links in the post content.

The fact that the text may go out of sync is the most serious issue as this enhancement can do more damage than good, incorrectly announcing link content, so I think it's better to remove it until we find a better solution.

Generally, the 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. It should be added to every web page viewed in the browser. 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. (I may open a page in a language I don't understand.) The user agent will also know if the link opens in a new tab or window, and adjust the text accordingly.

Safari does this well it seems:

Screenshot 2019-11-26 at 10 06 49

If browsers are reluctant to fix the issue, a better place to fix the behaviour would be in a browser extension or screen reader software.

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

The last place it should be added is at link creation with the link UI, if we want to label consistently.

How has this been tested?

Insert a link that opens in a new window. Make sure the source code doesn't have an aria-label.

Screenshots

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .
@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 26, 2019

Not opposed to remove the current implementation but I'd like to know what are the plans for a new, better, implementation (maybe PHP side) before removing it. Is there any plan for that?

Also, I'm not sure user agents can reliable for this purpose, as language identification is subject to too many factors (including wrong page headers or lang attributes) or assumptions.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

Also, I'm not sure user agents can reliable for this purpose, as language identification is subject to too many factors (including wrong page headers or lang attributes) or assumptions.

@afercia What do you mean? The language should not be that of the web page, but that of the OS, which only the browser can know. In other words, the browser, or whatever user agent, or an extension is the best place to fix it.

I don't have any plans. I'd rather see this addressed in browser or screen readers, or an accessibility browser extension so it's consistent for all links on all web pages.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

If we insist on fixing this on WordPress pages (which, again, is not a full fix for all the pages a user is browsing...), I would recommend adding a small script on the front end to inject screen reader text in the DOM instead of trying to inject in in HTML.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

@afercia Do you have any resources stating that this is a good practice?

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 26, 2019

I proposed to discuss this issue / PR in the next accessibility meeting so I'd like to ask to wait a bit before merging it.

I'm not sure I understand the operating system thing.

The "opens in a new browser tab" text needs to be in the same language of the post content. Browsers or operating systems don't analyze the page content, not to my knowledge at least. They assume a certain language based on the page headers or lang attribute. Delegating to them seems highly unreliable to me.

Also, WordPress discourages the usage of target="_blank" at the point there's a blessed task on the core Trac to remove as many occurrences as possible from the admin pages. Same applies to content produced by users. It is discouraged, though still allowed. As WordPress allows target="_blank" it should also make sure users are informed properly and the post content is accessible. As I see it, producing accessible content is a WordPress responsibility.

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 26, 2019

@ellatrix when you have a chance, can you please clarify the steps to get that "open in a new tab" text displayed in Safari?

Screenshot 2019-11-26 at 10 06 49

@MarcoZehe

This comment has been minimized.

Copy link

MarcoZehe commented Nov 26, 2019

I agree with @afercia on this. I work for a browser vendor, Firefox, to be exact, and the problem I have with this proposal is that the way language detection works is via the lang or xml-lang attributes either in the page header or overridden somewhere in the DOM. However, that is on an element level, not an attribute level one. Browsers expose that information to screen readers, and they can switch the synthesizer language or voice accordingly if the user has set this up. In that sense, content produced trough ARIA should be treated as if it was visual content for sighted folks, in the sense that it has the same effect. You would probably not want an English text to suddenly show " (öffnet in neuem Tab)" (the German equivalent) written somewhere on the screen. This is what is currently happening with that aria-label to the readers of my English blog. But because aria-label is an attribute whose language cannot be set individually, much less mid-phrase, users will hear this in their English synthesizer voice. I imagine this mix of languages would be just as ugly visually.

The same would be true if browsers suddenly started implementing this for the content authors somehow. I am not a Mac user, so don't know if that tool tip the screenshot shows would be spoken by VoiceOver while reading the page contents. But the fact remains that, even when I am normally reading something in German, if I read English text, I would expect something like "opens in a new tab" to be given to me in the same language, so my brain doesn't have to context-switch.

And as Andrea said, content accessibility is the content producer's responsibility, in this case WordPress. Browsers in this contract are making sure to translate everything in a way screen readers can understand, and screen readers then convey that information to the users who must of course know their part to get at all possible information.

I am OK with removing this particular broken implementation, but a replacement must come from WordPress, not user agents. What Apple are doing here is non-standard. And besides, it is annoying to hear that whole URL spoken for something like the notification that this will open in a new tab, so to me as a user, that particular announcement, if VoiceOver reads it, is more an annoyance than actually helpful.

As for WCAG standards: Informing users about the fact that a link will open in a new tab falls under success criterion 3.2, the web site must operate in predictable ways. So yes, this is WordPress responsibility.

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Nov 26, 2019

Will add this issue to the accessibility meeting agenda for this Friday.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

The "opens in a new browser tab" text needs to be in the same language of the post content. Browsers or operating systems don't analyze the page content, not to my knowledge at least.

@afercia I'm not saying that the browser should analyse the post content. I'm saying that the browser should use the language of the OS and browser. For example, my OS and browser language is English. If I open a web page in Japanese, the "open ... in a new tab" text will still be in English, because that's the language I set for the browser and it's a language I understand as a user of the internet. I'm also not the user agent should look at any lang attribute or page headers, which also changes from web page to web page.

The screenshot is a label that appears in Safari when you hover over a link and press Cmd or Shift+Cmd. I could imagine something like that being announced as well by a screen reader or properly labelled internally by the browser.

Screenshot 2019-11-26 at 20 13 36

In the screenshot above the pointer is over the link and Shift+Cmd is pressed.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

As I see it, producing accessible content is a WordPress responsibility.

As I asked before, do you have any resources stating this is the recommended practice for accessible content?

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

You would probably not want an English text to suddenly show " (öffnet in neuem Tab)" (the German equivalent) written somewhere on the screen.

@MarcoZehe I would if the browser language is German. Again, I'm saying we should not look at the web page language at all.

Screenshot 2019-11-26 at 20 20 19

Here you can see that the directions in Safari are in English, even though the page is in German, because my browser and OS language is English.

And as Andrea said, content accessibility is the content producer's responsibility, in this case WordPress.

I don't fully agree with this. Browsers and screen readers provide a lot of built-in logic to enhance accessibility.

@MarcoZehe

This comment has been minimized.

Copy link

MarcoZehe commented Nov 26, 2019

You would probably not want an English text to suddenly show " (öffnet in neuem Tab)" (the German equivalent) written somewhere on the screen.

@MarcoZehe I would if the browser language is German. Again, I'm saying we should not look at the web page language at all.

inside the web content page? After all that is what we're talking about here.

Screenshot 2019-11-26 at 20 20 19

Here you can see that the directions in Safari are in English, even though the page is in German, because my browser and OS language is English.

I actually cannot, because I am blind, but I assume from your previous description that this is outside the actual web page content, somewhere in the browser chrome. Which means that it is indeed not part of the content area. And, if I might add, it is not very discoverable, IMO, if you have to press two modifier keys on hover to get this tooltip to appear.
Again, I think because the content provider, in this case WordPress on behalf of its author(s), is responsible for providing the content, and that should be language-consistent. So if the site language is set to English, that text inlined in the web page content should be in the same language.

And besides, WCAG criterium 3.2 (predictability) seems to agree with me with regards to the indicator for opening in a new tab.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 26, 2019

@MarcoZehe The Safari behaviour is just something to illustrate what I have in mind for a screen reader to announce. I'm not saying it should use that exact same text, it could be be just that the link would open in a new tab or window if the the target attribute is set to _blank.

Let me rephrase why I think it would be better to have it built-in a browser or screen reader.

Even if we add "opens in a new tab" to every link on the page for WordPress pages, the rest of the pages that the user visits won't have this text. So while it might be of help for some pages, it's not a consistent experience at all.

Something at would make more sense to me is the screen reading detecting the target attribute and announcing that the link would open in a new language if it would be pressed. And what language is that? The language that the screen reader is set to, not the language of the web page. The language of the screen reader is not dependent on the language of the web page. No text would be added inside the web page.

For example, in VoiceOver, it could be announced something like "You are currently on a link that opens in a new tab".

@MarcoZehe

This comment has been minimized.

Copy link

MarcoZehe commented Nov 27, 2019

@ellatrix The question is really one of "What's good for one group is most likely good for many more". So if we go with a new tab indicator for all sites that have the target attribute on links, it has to come through the user agent, and be more than just a screen reader thing. It needs to be a clear visual indicator for everybody. So the last thing we want is for the screen reader to start interpreting the HTML. That would take us back to the dark ages of the early 2000s and IE only, where screen readers all had to interpret HTML their own way, and interop was a nightmare.

Having said that, even the current approach in Gutenberg, which this pull request will indeed remedy, is not a good solution because aria-label only is for assistive technologies, and mostly screen readers. Sighted people are left out. But I know many to whom this would mean a serious cognitive challenge without prior knowledge. That's why many web authors have so far adopted putting that text right into the link as well so everybody benefits. And that would, of course, be in the language of the other text around it, not the language the CMS was set to by chance when the content was created. Which takes us back to my original issue in #18727 .

So while I agree in principle that it should be a default indicator for everyone, and I'll actually propose this to Firefox product, it would still mean an inconsistent behavior if Chromium (and that also means Edge) decide to not follow suit. On the other hand, if WordPress would start indicating this clearly and visually by default, it would mean that at least a third of websites on the net today would get this indicator in all user agents.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 27, 2019

Right, adding the text is a fairly easy solution with some immediate benefits. User agents adopting it would be good as well, but as you say, neither solution will be complete across the web and all browsers immediately. Either it's fixed for all WP sites, or it's fixed for one browser. I still believe it would be good to address something like this on the browser level. I see this a bit as a browser user interface, giving directions to the user about the page and navigation. But I can also see how we can make a dent with WordPress.

So while I agree in principle that it should be a default indicator for everyone, and I'll actually propose this to Firefox product

That's great to hear!

So the last thing we want is for the screen reader to start interpreting the HTML.

Yeah, without a specification or best practise it's hard. That's why I'm asking for a resource specifically saying that appending the text "opens in a new tab" is the best practice. It's worth noting that this is something fairly new that we're doing. We never added this text in the classic editor, or on the front end.

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 27, 2019

As I see it, producing accessible content is a WordPress responsibility.

As I asked before, do you have any resources stating this is the recommended practice for accessible content?

There are a number of suggested techniques published by the W3C that everyone can easily find by googling a bit. For example:

G201: Giving users advanced warning when opening a new window
https://www.w3.org/TR/WCAG20-TECHS/G201.html

H83: Using the target attribute to open a new window on user request and indicating this in link text
https://www.w3.org/TR/WCAG20-TECHS/H83.html

Overall, I'm not sure I understand your objections. WordPress should aim to produce accessible, semantic, content regardless of any W3C recommendation or legal requirement. WordPress can do that by providing tools and educating users. Relying on non-existing-yet browsers or assistive technologies features isn't a solution.

Historically, WordPress also aims to educate users to best practices to produce semantic, accessible, content. For a number of years WordPress has been discouraging the use of target="_blank" and should educating users that they should stop taking control of the users browser. We're doing it in the admin since a few years. Gutenberg aims to improve content production thus it would be great to see the best practices we adopted for the admin be implemented also for the front end. Gutenberg offers a good opportunity to do that, and that's the reason why this feature was originally implemented.

That said, the accessibility team expressed some concerns about the implementation details. Not sure an aria-label is the best option. As pointed out in other comments, some plain text would be preferable. Also, sighted users would need a visual indication.

Regarding language identification, I'm really not sure I understand. Why this text should be based on the operating system language? That seems to me a big assumption. Seems pretty obvious to me that it should use the post content language instead. Also, it should be editable as any other content thus Gutenberg should provide a UI to edit this text.

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 27, 2019

The screenshot is a label that appears in Safari when you hover over a link and press Cmd or Shift+Cmd. I could imagine something like that being announced as well by a screen reader or properly labelled internally by the browser.
In the screenshot above the pointer is over the link and Shift+Cmd is pressed.

That doesn't work for me (Safari 13.0.3). Are you sure it doesn't come from some browser extension or some non-default setting?

Regardless, that's pretty different: by pressing Cmd or Shift+Cmd the link is going to open in a new tab by user request. In this case there's really no need to inform users, as the new tab would be the result of an intentional action. The real problem is when content authors take control of the users browser by using target="_blank" and omit to inform them.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 27, 2019

Also, it should be editable as any other content thus Gutenberg should provide a UI to edit this text.

That's news to me. I didn't know that it should be editable. In that case it might be easiest to do what https://www.w3.org/TR/WCAG20-TECHS/G201.html suggests:

<a href="knitting.html" target="_blank">All about Knitting  (opens in new window)</a>

Then the text is editable and is there for everyone. How does that sound?

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 27, 2019

Let's wait till Friday for this issue to be discussed during the accessibility meeting on Slack please 🙂 There are other considerations to be made, including themes support.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 27, 2019

Theme support? Could you elaborate?

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented Nov 27, 2019

I'm pretty sure themes authors would want a way to control how the appended text is rendered. Visible? Visually hidden? With or without icon? Removing it entirely? etc. As I said, WordPress should educate users but should avoid to force any output (maybe there should also be a filter).

@aduth

This comment has been minimized.

Copy link
Member

aduth commented Nov 29, 2019

Will this require additional changes to avoid invalidating existing content blocks?

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Nov 29, 2019

This was discussed during today's accessibility meeting.

The team agrees that removing the current broken functionality is a good first step and we think the ideal solution would be to have the string appended to the link's visible label instead.

This makes this piece of information visible to everyone (not just users of screen readers) and we believe it'll address the other issues of the aria-label not being updated when the link is edited and it being set to the wrong language.

Because the string will be appended to the link's visible label (and thus part of the document), the string's language should be set to the document's language.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 29, 2019

Will this require additional changes to avoid invalidating existing content blocks?

@aduth Within rich text, there's no such thing as invalid content. 😉 It will still be there and remain valid.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 29, 2019

@enriquesanchez Sure, I can do that. To be clear: I will make it so that, when you set a link to open in a new window, the translated text "(opens in a new tab)" will be appended to the link text content.

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Nov 29, 2019

while the solution makes sense a11y wise, I expect a lot of negative feedback from content creators about that.

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Dec 2, 2019

To be clear: I will make it so that, when you set a link to open in a new window, the translated text "(opens in a new tab)" will be appended to the link text content.

Correct.

while the solution makes sense a11y wise, I expect a lot of negative feedback from content creators about that.

@ellatrix @youknowriad Agreed.

It's important to note that the proposed solution is the ideal one for the accessibility team. While we should support authors in creating more accessible content, we can't of course force anyone.

I wonder if we can have a setting, similar to the 'Open in a new tag' toggle that lets you turn this off? That way, if someone really does not want the appended text they can turn it off.

@MarcoZehe what do you think about this idea?

@MarcoZehe

This comment has been minimized.

Copy link

MarcoZehe commented Dec 2, 2019

To be clear: I will make it so that, when you set a link to open in a new window, the translated text "(opens in a new tab)" will be appended to the link text content.

Correct.

And we're clear on the language of the site being the primary one, and the language of the user the fall-back if the former isn't available, right?

I wonder if we can have a setting, similar to the 'Open in a new tag' toggle that lets you turn this off? That way, if someone really does not want the appended text they can turn it off.

That idea sounds good to me.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 6, 2019

Where should this setting go?

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Dec 9, 2019

Where should this setting go?

@ellatrix I think it should go next to the 'Open in a new tab' setting, and it should appear only when that setting has been activated. I'm thinking something like the following:

Screen Shot 2019-12-09 at 15 34 35

The image above shows the default off setting of the 'Open in a new tab' option of a link)

Screen Shot 2019-12-09 at 15 53 52

The image above shows a proposal to have a new setting appear when the 'Open in a new tab' option is turned on. The new setting's label is 'Append “(opens in a new tab)”
to link'
. It's on by default and is accompanied by a notice that says "Leave this setting on to help avoid confusing users who do not easily perceive a change in context.".

The wording of the message is inspired by the Criterion 3.2.2: On Input of WCAG 2.1

I would like to get a second opinion on both the design and the wording of the setting and accompanying notice.

@MarcoZehe what do you think of the wording I used?
@melchoyce @mapk @richtabor can I get your opinion on the proposed design here?

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Dec 9, 2019

Or maybe the setting should read 'Remove “(opens in a new tab)” from link' and be off by default?

@melchoyce

This comment has been minimized.

Copy link
Contributor

melchoyce commented Dec 9, 2019

Instead of using a notice here, we should use the "help" prop for BaseControl:

image

@MarcoZehe

This comment has been minimized.

Copy link

MarcoZehe commented Dec 10, 2019

The image above shows a proposal to have a new setting appear when the 'Open in a new tab' option is turned on. The new setting's label is 'Append “(opens in a new tab)” to link'. It's on by default and is accompanied by a notice that says "Leave this setting on to help avoid confusing users who do not easily perceive a change in context.".

I like this more than the other suggestion because this gives a positive spin on an "on by default" setting whereas the other has a more negative tone, and inspires people to turn it on despite (or even because) it says "remove".
So, this initial suggestion is what I like the most.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 10, 2019

The UI would be sooo much simpler if we just removed the "open in new tab" option, which we should strongly discourage anyway... 😔 Alright, I'll add it then.

@ellatrix ellatrix force-pushed the remove/link-aria-label branch from c1e7d75 to 99a5e03 Dec 10, 2019
@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 10, 2019

@enriquesanchez How does this look? :)

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Dec 10, 2019

For me, the new option is confusing. What happens if I remove the text manually, should we keep the option on or toggle it off? I'd personally think the option should be global (not link specific) or no option at all and deal with the fud 🤷‍♂ .

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Dec 10, 2019

That makes sense, though it might be hard to find. Where should this new option be? The current option could also be global, but still placed where it currently is.

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Dec 10, 2019

We have an "options" modal and we discussed at some point adding an a11y section there.

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Dec 10, 2019

How does this look? :)

@ellatrix that's exactly what I had in mind 👍

What happens if I remove the text manually, should we keep the option on or toggle it off?

@youknowriad makes a really good point that I hadn't considered. The appended text will inevitably be editable and this adds a lot of complexity.

The UI would be sooo much simpler if we just removed the "open in new tab" option, which we should strongly discourage anyway

I'd personally think the option should be global (not link specific) or no option at all and deal with the fud

I couldn't agree more with you both. We're creating complex workarounds in order to support a feature that in the end affects accessibility and should be avoided.

I'm not familiar with why and when this option was added. As much as I'd like to say yes let's remove it, I'm sure it wouldn't be that simple?

@MarcoZehe

This comment has been minimized.

Copy link

MarcoZehe commented Dec 10, 2019

The UI would be sooo much simpler if we just removed the "open in new tab" option, which we should strongly discourage anyway

I'd personally think the option should be global (not link specific) or no option at all and deal with the fud

I couldn't agree more with you both. We're creating complex workarounds in order to support a feature that in the end affects accessibility and should be avoided.

I'm not familiar with why and when this option was added. As much as I'd like to say yes let's remove it, I'm sure it wouldn't be that simple?

Well... My personal opinion is that yes, opening in a new tab should be avoided as much as possible. I assumed that this option was there because, as with the classic editor, people should have been given the option as before to set links to open in a new tab. I filed my original issue about the text being in the wrong language assuming that totally removing this would not be an option.

However accessibility-wise, recommendations always say to not open links in a new tab, but let the user decide if they want to do it. They have the means via mouse, keyboard, and touch, in all browsers.

So if removing the Open In New Tab setting entirely is an option... I'd say go for it, remove the thing, that will also fix that particular language problem.

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Dec 11, 2019

For the record, we initially decided to not include the option in the early days of Gutenberg. Unfortunately, people complained a lot about it and even if it's not good a11y wise, we had to restore the option.

@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Dec 11, 2019

View #4583 for historical context on why this setting exists: (thanks @karmatosed for uncovering this!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.