Using extlinks to open external links in a new window, but not add the icon, I noticed the module js still appended the span tags to external link a tags. The icons weren't printing but extra markup was and added visual blank spaces at the end of the link text. After digging through the module I figured out this was because several config variables were being declared as boolean but used as string.

For example "Place an icon next to external links." (var: extlink_class) is a boolean, defaults to 'ext', and if the checkbox on the admin config page is unchecked the value is set to '0' (string 0).

In this patch I separated out some boolean vars into a boolean and a string var. I thought about making extlink_class a boolean and if it's true, passing in "ext" to the JS. But to let people extend the module and add other classes I kept a boolean toggle for extlink icon and a string var for the class name. I added two variables for the invisible text and added them to the admin config form, updated to the D8 class for hiding the text (.visually-hidden), and added some checks in the JS around not executing some functions if extlink icon or mailto icon are turned off. I also created an update module hook to set the former boolean variables to the correct strings based off the current value.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rjg created an issue. See original summary.

rjg’s picture

Issue summary: View changes
rjg’s picture

Issue summary: View changes
trwill’s picture

Status: Patch (to be ported) » Needs review
trwill’s picture

Status: Needs review » Needs work

Patch applies cleanly - note you will have to run db updates if the module is already installed. This patch does fix the erroneous class being applied (and the whitespace).

However, the patch seems to break the 'open new links in an external window' logic if the 'Place an icon next to external links.' option is not selected. I think this likely is due to the logic removed from old extlink.js:69-83. A new check that replaces this logic (starting at extlink.js:70) looks for the add icon property, and if it exists then apply the target="_blank", but if the 'Place an icon next to external links.' is not selected it never applies the target attr, and external links open in the current window.

rjg’s picture

@trwill I still need to fix that glitch. Need to find time soon.

akalata’s picture

Version: 8.x-1.0-alpha3 » 8.x-1.x-dev
FileSize
22.31 KB

reroll of #1 against current dev

akalata’s picture

I tried giving this a shot but don't know enough js.

I renamed 'icon_not_class' to 'exclude_icon_class', and tried to pull out the repeatable code:

if (exclude_icon_class) {
      $('a:not(' + exclude_icon_class + '), area:not(' + exclude_icon_class + ')', context).each(function() {
        buildElements(this, external_links, mailto_links)
      });
    } else if (drupalSettings.data.extlink.extTarget) {
      $('a, area', context).each(function() {
        buildElements(this, external_links, mailto_links)
      });
    }

...

  function buildElements(el, external_links, mailto_links) {
    try {
      var url = '';
      if (typeof el.href == 'string') {
        url = el.href.toLowerCase();
      }
      // Handle SVG links (xlink:href).
      else if (typeof el.href == 'object') {
        url = el.href.baseVal;
      }
      if (url.indexOf('http') === 0
          && ((!url.match(internal_link) && !(extExclude && url.match(extExclude))) || (extInclude && url.match(extInclude)))
          && !(extCssExclude && $(el).parents(extCssExclude).length > 0)
          && !(extCssExplicit && $(el).parents(extCssExplicit).length < 1)) {
        external_links.push(el);
      }
      // Do not include area tags with begin with mailto: (this prohibits
      // icons from being added to image-maps).
      else if (this.tagName !== 'AREA'
          && url.indexOf('mailto:') === 0
          && !(extCssExclude && $(el).parents(extCssExclude).length > 0)
          && !(extCssExplicit && $(el).parents(extCssExplicit).length < 1)) {
        mailto_links.push(el);
      }
    }
        // IE7 throws errors often when dealing with irregular links, such as:
        // <a href="node/10"></a> Empty tags.
        // <a href="http://user:pass@example.com">example</a> User:pass syntax.
    catch (error) {
      return false;
    }
  }

But I'm not getting elements appended to the external_links or mailto_links arrays.

weseze’s picture

We are also experiencing this issue, but only on older sites, even after updating them. Clean installs do not have this problem.

I'm unsure why this problem exists or were it is coming from, but the solution seems to be to update the config for the "extlink.settings.extlink_class" to "ext" if it is "0".

Below is an update hook that fixes it:

/**
 * Some installs don't have a correct class set. It is set to '0' for some
 * reason. Correct this behaviour.
 */
function extlink_update_8001() {
  $current = \Drupal::configFactory()->get('extlink.settings')->get('extlink_class');
  if ($current == 0) {
    \Drupal::configFactory()->getEditable('extlink.settings')
      ->set('extlink_class', 'ext')
      ->save();
  }
}
Bunty Badgujar’s picture

Hi,
I just made a very small patch which can fix this issue. No need to change js file.

Bunty Badgujar’s picture

Status: Needs work » Needs review

The last submitted patch, 7: 2821803-reroll-7.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 10: type-mismatch-2821803-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

elachlan’s picture

Status: Needs work » Needs review
FileSize
1.44 KB
elachlan’s picture

elachlan’s picture

Status: Needs review » Closed (duplicate)

Work will continue in the other issue.