Problem/Motivation
The D7 version of extlink provides text alternatives for the external link icons, which are important for screen readers. The icon itself is a CSS background image, and the text alternative is provided in a span.element-invisible element.
This has stopped working in the D8 version of the module. The expected <span> element is present, however:
- It has no text content, i.e. the alternative text itself is missing
- It still uses the D7
element-invisibleclass. This has been renamed tovisually-hiddenin D8.
The problem appears to be incorrect use of $config->get() during hook_page_attachments(). The module is trying to provide default values here, like the D7 variable_get(), but in D8 values are always expected to come from the config store.
Additionally, the installation config and config schema don't include all the settings keys that hook_page_attachments() is trying to use. Neither of these config files mention extlink_label, so it doesn't have a value after installing the module. Hence the JS fails to write the text alternative for the icon.
So overall, this looks like a few steps were omitted when porting the module to D8.
Steps to reproduce
- Install the D8 version of External Link module, and enable the "place an icon..." settings on the module's administration page.
- Create a node which has a link in the body text, with a URL to an external site.
- View the node. The extenal link has an icon after it. This is expected, but...
- A screen reader will read the link text, but there is no indication that the link is external.
- Inspect the link using browser dev tools. The link has a descendant
<span class="element-invisible">but it has no text content. This is where the JS is supposed to put the text alternative. - Inspect the DOM using browser dev tools. There is a
drupalSettings.data.extlinkobject, but theextLabelproperty is an empty string.
Compare this with the D7 version, after the same steps:
- The link contains
<span class="element-invisible">(link is external)</span> - This text comes from
Drupal.settings.extlink.extLabeljavascript object. - A screen reader will include this text in the link name.
Proposed resolution
Complete the module's config (install and schema) so that it provides all the default values needed.
Remaining tasks
- Change
element-invisibleclass tovisually-hiddenin the JS file. This class changed name in D8 core CSS. - Make the same change in the
extlink.min.jsfile - Complete the module's config schema (
config/schema/extlink.setting.yml) so that it describes all of the config keys accessed byextlink_page_attachments()in the module file. - Move the installation config (
config/extlink.setting.yml) to the correct location:config/<strong>install</strong>/extlink.settings.yml. - Update the installation config (
config/install/extlink.settings.js)) so that it has all the default values currently described inextlink_page_attachments(). - In
extlink_page_attachements(), remove the second argument from every calll to$config->get(). This method only accepts one argument in D8. - [possibly...?] Write an update hook so existing installations get the text alternatives for the icon.
User interface changes
Fix markup for the external link icon so it includes a text alternative, like the D7 version of the module does.
API changes
None.
Data model changes
Completes the config schema so it has all the settings which the module makes use of.
Updates the installation config so default values are provided.
Original report by naveenvalecha
Original issue title: Screen reader unable to read out External Links
Steps to Reproduce:
1. Create any external link by creating any node.
2. Enable the Screenreader
3. Navigate to External link using keyboard
*Observation:* Screen reader is unable to read it as an external link.
Patch the external link module for accessibility.
We can have workaround like this site has.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | extlink-default-settings-2892317-17.patch | 12.32 KB | cboyden |
| #15 | extlink-default-settings-2892317-15.patch | 12.06 KB | berdir |
| #14 | interdiff-12-14.txt | 1.11 KB | cboyden |
| #14 | extlink-default-settings-2892317-14.patch | 12.07 KB | cboyden |
Comments
Comment #2
naveenvalechaComment #3
naveenvalechaComment #4
naveenvalechaTagging with these tags.
Comment #5
andrewmacpherson commented"Needs accessibility review" is for Drupal core issues which require an accessibility maintainer to sign-off on a change.
Comment #6
andrewmacpherson commentedThe approach in patches #2 and #3 is not translatable.
The D7 version of this module already has translatable alternative text, inside a
span.element-invisibleelement. It's included as a JS setting, and the JS behaviour includes it correctly.In the D8 version, this has stopped working. An empty span is added, without the text label. The problem appears to be incorrect use of
$config->get()duringhook_page_attachments(). In D7variable_get()took a default value as a second parameter, but in D8Config::get()does allow a default value. Instead the values are always expected to come from the config store. Theconfig/schema/extlink.schema.ymldoes not mentionextlink_label, and it is not included in the installation config either.The D8 markup needs to be updated to use the
visually-hiddenclass, which was calledelement-invisiblein D7.This problem applies to several extlink settings, so updating the issue title, and promoting to major.
Comment #7
cboyden commentedI can work on this. At first glance, the extlink.settings.yml file is in the wrong location. Also it seems the admin form doesn't include fields to override default values of certain settings.
Comment #8
andrewmacpherson commentedUpdating the issue summary, detailing the bug and the proposed fix, together with D7 background.
The workaround mentioned by @naveenvalecha is how the text alternative works for external link icons on WebAIM. This is fine for an individual site in one language, but the text alternative isn't translatable so it's not sufficient for a multilingual CMS like Drupal.
Hoping to encourage more contributors to accessibility issues in contrib, so I've promoted it our a11y channel on drupal.slack.org
Comment #9
andrewmacpherson commentedHmmm, there's an
extlink.min.jsin the D8 module, which hte D7 version doesn't have. I don't really see the point of that, but adding it to the tasks in the issue summary.Comment #10
andrewmacpherson commentedComment #11
andrewmacpherson commentedComment #12
cboyden commentedI've attached a patch that does the following:
Comment #13
cboyden commentedBooleans in schema files can't be 0/1 anymore, they have to be true/false. Also the settings file needed updating. Updated patch is attached.
Comment #14
cboyden commentedI set the wrong default config value for extlink_alert in the previous patch - see updated patch and interdiff.
Comment #15
berdirRerolled the patch against latest 8.x-1.x-dev.
Comment #16
berdirtranslatable strings should be type label so they can be translated using config translation.
Comment #17
cboyden commentedUpdated patch makes the translatable config strings (extlink_label, extlink_mailto_label, extlink_alert_text) into labels.
Comment #18
berdirLooks good!
Comment #21
elachlan commentedComment #22
berdir@elachlan: That was suddenly fast, thanks for the commit! A new release would be really nice as 1.0 is quite old and broken.
Comment #23
elachlan commented@Berdir, I think there is still one outstanding issue that can be resolved. But needs a patch re-roll. I'll be happy to do a release after that.
Comment #24
berdirCommented there and provided an alternative patch. There's always "one more issue to fix" :)
Comment #26
naveenvalechaThanks, @andrewmacpherson for your reviews on accessibility. Thanks, everyone to get it fixed.