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-invisible class. This has been renamed to visually-hidden in 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.extlink object, but the extLabel property 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.extLabel javascript 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-invisible class to visually-hidden in the JS file. This class changed name in D8 core CSS.
  • Make the same change in the extlink.min.js file
  • Complete the module's config schema (config/schema/extlink.setting.yml) so that it describes all of the config keys accessed by extlink_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 in extlink_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.

Comments

naveenvalecha created an issue. See original summary.

naveenvalecha’s picture

Status: Active » Needs review
StatusFileSize
new358 bytes
naveenvalecha’s picture

StatusFileSize
new328 bytes
naveenvalecha’s picture

Tagging with these tags.

andrewmacpherson’s picture

"Needs accessibility review" is for Drupal core issues which require an accessibility maintainer to sign-off on a change.

andrewmacpherson’s picture

Title: Screen reader unable to read out External Links » default values for extlink JS settings not working.
Priority: Normal » Major
Status: Needs review » Needs work

The approach in patches #2 and #3 is not translatable.

The D7 version of this module already has translatable alternative text, inside a span.element-invisible element. 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() during hook_page_attachments(). In D7 variable_get() took a default value as a second parameter, but in D8 Config::get() does allow a default value. Instead the values are always expected to come from the config store. The config/schema/extlink.schema.yml does not mention extlink_label, and it is not included in the installation config either.

The D8 markup needs to be updated to use the visually-hidden class, which was called element-invisible in D7.

This problem applies to several extlink settings, so updating the issue title, and promoting to major.

cboyden’s picture

Assigned: Unassigned » cboyden

I 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.

andrewmacpherson’s picture

Issue summary: View changes

Updating 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

andrewmacpherson’s picture

Issue summary: View changes

Hmmm, there's an extlink.min.js in 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.

andrewmacpherson’s picture

Issue summary: View changes
andrewmacpherson’s picture

Issue summary: View changes
cboyden’s picture

Assigned: cboyden » Unassigned
Status: Needs work » Needs review
StatusFileSize
new11.37 KB

I've attached a patch that does the following:

  • Adds the mailto and extlink labels to the schema
  • Moves the default settings into the correct location config/install/
  • Adds the mailto and extlink labels, plus other defaults, to the default settings
  • Updates the calls to $config->get to remove the unused parameter
  • Changes the class name added by the JS to visually-hidden
  • Adds an update hook to configure the label text alternatives with the defaults
cboyden’s picture

StatusFileSize
new12.07 KB
new1.11 KB

Booleans 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.

cboyden’s picture

StatusFileSize
new12.07 KB
new1.11 KB

I set the wrong default config value for extlink_alert in the previous patch - see updated patch and interdiff.

berdir’s picture

StatusFileSize
new12.06 KB

Rerolled the patch against latest 8.x-1.x-dev.

berdir’s picture

Status: Needs review » Needs work
Issue tags: -Accessibility
+++ b/config/schema/extlink.schema.yml
@@ -28,6 +28,9 @@ extlink.settings:
+    extlink_label:
+      type: string
+      label: 'Alternative text for external link icon'

translatable strings should be type label so they can be translated using config translation.

cboyden’s picture

Status: Needs work » Needs review
StatusFileSize
new12.32 KB

Updated patch makes the translatable config strings (extlink_label, extlink_mailto_label, extlink_alert_text) into labels.

berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

  • elachlan committed 92a73b4 on 8.x-1.x authored by cboyden
    Issue #2892317 by cboyden, naveenvalecha, Berdir: default values for...

elachlan credited elachlan.

elachlan’s picture

Status: Reviewed & tested by the community » Fixed
berdir’s picture

@elachlan: That was suddenly fast, thanks for the commit! A new release would be really nice as 1.0 is quite old and broken.

elachlan’s picture

@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.

berdir’s picture

Commented there and provided an alternative patch. There's always "one more issue to fix" :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

naveenvalecha’s picture

Thanks, @andrewmacpherson for your reviews on accessibility. Thanks, everyone to get it fixed.