[policy, no patch] Formalize how external links are handled in core

Note @mgifford's comment #50 on the issue this was spawned from:

The accessibility pieces are essentially about user control. Opening up a new page to direct folks to a new page can be disorienting. It's a much better practice to teach users how to use their browser. It's really hard for some users to handle multiple screens (especially on a mobile device) even the browser has good support for them.

#2652272-50: Review (and in most cases remove) use of target="_blank" attribute outside the installer or update.php

Maybe external pages are ok for pages that are references for the opener page.

Google's Lighthouse is now recommending https://developers.google.com/web/tools/lighthouse/audits/noopener

In general, always add rel="noopener" when you open an external link in a new window or tab.

Although that looks like it is just for target="_blank" links.

Every link in Drupal 8 automatically gets rel="nofollow" thanks to this bit of code in core/modules/filter/src/Plugin/Filter/FilterHtml.php on line 140:

if ($this->settings['filter_html_nofollow']) {
      $links = $html_dom->getElementsByTagName('a');
      foreach ($links as $link) {
        $link->setAttribute('rel', 'nofollow');
      }
    }

I suggest adding a check here if that link has target="_blank" and if so, also adding the rel 'noopener':

if ($this->settings['filter_html_nofollow']) {
      $links = $html_dom->getElementsByTagName('a');
      foreach ($links as $link) {
        $link->setAttribute('rel', 'nofollow');
        if ($link->hasAttribute('target') && $link->getAttribute('target') == '_blank') {
          $link->setAttribute('rel', 'nofollow noopener');
        }
      }
    }

This will improve performance and prevent security vulnerabilities, as is described here:
https://web.dev/external-anchors-use-rel-noopener/?utm_source=lighthouse...

It will also remove this error from lighthouse site audits and thus improve your best practice scores:

Links to cross-origin destinations are unsafe

Could someone make this into a patch perhaps ?

Use of target="_blank" is extremely disruptive for several groups of users.

It's strongly preferable to let the user decide whether they want to open the link in a new window/tab, or the current window. Using target="_blank" removes this choice from the user, and prevents the user from returning to the previous page by using the browser's "backwards" function. It isn't possible for users to override target="_blank" specified by a web page (if it was, that would be a game changer).

The relevant part of WCAG is success criterion 3.2.5 Change On Request. There are occasional circumstances where it's preferable to open a new window/tab. Some examples are discussed in WCAG advisory technique G200: Opening new windows and tabs from a link only when necessary.

However, I'd expect it to be RARE for a link to actually warrant target="_blank". If you think a link needs it... think again. It probably doesn't. (Rinse and repeat.)

Some bad reasons for using target="_blank":

• The link points to a different web site.
• The link points to documentation.
• The link is on a form.
• Reducing something called a "bounce rate".
• Because the author/designer/developer wants it.

As for the policy, I'd say for any issue which proposes to introduce target="_blank":

• The issue must provide a compelling reason why it is necessary.
• Reviewers and committers should push back hard. No waving it through.
• In rare instances where we do permit target="_blank", we really MUST warn users about what will happen. That means visible text to say the link opens in a new window (preferably inside the link itself).

Re. #2:

in the cases where data loss may occur (eg, form entry), use target=blank

This one is often the reason given (and waved through). It doesn't always hold water though; browsers are good at remembering form state when you use the back button. The mere fact that a link is on a form doesn't make it a good candidate for opening a new window. The steps for data loss should be demonstrated.

If I sound a bit militant here, it's because I am. It's easy to make links which fail WCAG Change on Request. It's even easier to make links which pass.

I like the approach in #13 of checking for target=_blank then adding rel=noopener. The contrib external links module does something similar to insert the "opens in new window" text.

There's an ATAG aspect to this; the system should guide authors to create accessible content. So if Drupal core provides tools for authors to create links with target="_blank", then it should also provide guidance for content editors to use it (effectively, persuade them not to). See ATAG B.2.2. Guide authors to produce accessible content and B.4.2. Ensure that documentation promotes the production of accessible content.

I have no opinion on rel=nofollow :-)

An idea from @xjm in #3142818-19: Remove target=_blank from links in media settings forms

I think there should be a sniff to warn devs about <strong>target="_blank"</strong>, so tagging for a followup. Thanks!

I've added that to the proposed resolution here. Is it feasible? It could go in drupal/coder say. I think some accessibility testing tools have a sniff for this (HTML_codesniffer, aXe, or whichever). We don't have one of those in our CI system yet though.

Oops, fixing bad HTML.

The Drupal core Accessibility core gate states that a goal of core is to conform to WCAG 2.1. That standard has recommendations, https://www.w3.org/WAI/WCAG21/Techniques/general/G200, for this case.

There is an item here to add a sniff for target=_blank. But there are valid cases for using _blank so adding a sniff to to warn about it's use could be disruptive when running commit-code-check.sh.

Therefor, I think everything is done here.

Thanks everyone!

Changing to latest version when this was closed.

@xjm reminded me that this should be check by the Accessibility maintainers. I will ping in slack.

I think #24 is correct and we probably can't add a sniff disallowing target="_blank" to the core ruleset (unless we confirm that there really is not a single valid core usecase presently). It could potentially be a DrupalPractice sniff or something.

That said, I think core is definitely using target="_blank" in a number of places it should not. We should audit and remove most if not all of them.

It looks like #2652272: Review (and in most cases remove) use of target="_blank" attribute outside the installer or update.php already exists for getting rid of those pesky new windows.

Makes sense to me.

@mgifford, thank you for responding here and in slack, confirming that this core gate is correct.

I updated the IS to say the implementation is to be done in #2652272: Review (and in most cases remove) use of target="_blank" attribute outside the installer or update.php.

Also restoring the outdated status.

Saving credits.