OR within complex code

I have this code to avoid the deprecated html target="_blank"


//al posto target=_blank
<!--
function externalLinks() {  
 if (!document.getElementsByTagName) return;  
 var anchors = document.getElementsByTagName("a");  
 for (var i=0; i<anchors.length; i++) {  
   var anchor = anchors[i];  
   if (anchor.getAttribute("href") &&  
       anchor.getAttribute("rel") == "external")  
     anchor.target = "_blank";  
 }  
}  
window.onload = externalLinks;
//-->

And the above code works: if I add rel=“external” the link open a new tab as if I had target="_blank"
Now I wish add another condition: not only if I set rel=“external” within an a element, but also if I set class=“amazon”, f.e.

<a class="amazon">some-url</a>

it should open a new tab.
After several attempts I ask if you could help me. My last attempt was:

//al posto target=_blank
<!--
function externalLinks() {  
 if (!document.getElementsByTagName) return;  
 var anchors = document.getElementsByTagName("a");  
 for (var i=0; i<anchors.length; i++) {  
   var anchor = anchors[i];
   if (anchor.getAttribute("href") &&  
       (anchor.getAttribute("rel") == "external")
   ||  anchor.getAttribute("class") == "amazon"))
   anchor.target = "_blank";
 }
}  
window.onload = externalLinks;
//-->

What code should I write?

I wasn’t aware it was deprecated.

target="_blank" isn’t deprecated anymore…
https://www.w3.org/TR/2011/WD-html-markup-20110525/a.html

and

But to get it to work, this snippet works…

let links = document.querySelectorAll('a');

links.forEach(function(link) {
  if (link.getAttribute("rel") == 'external' || 
      link.getAttribute("class") == "amazon") {
        link.target = "_blank";
  }  
})

Of course, you can shorten it further to this, but I think the previous version is a little more readable…

document.querySelectorAll('a').forEach(function(link) {
  if (link.getAttribute("rel") == 'external' || 
      link.getAttribute("class") == "amazon") {
         link.target = "_blank";
  }  
})
3 Likes

Very good! Perfect!
Thank you very much!
And, btw, if I would make as _blank all links (beginning) with “http” (/“https”), what code I should use?

You would just use something like this, but why would you include something like that when the rel=“external” should already cover those? Unless you’re using absolute linking in ALL your links, which seems like overkill…

link.getAttribute('href').includes('https://')

Here’s how I would deal with things.

First I would bring the code up to date, by removing the traditional element property event and use addEventListener.instead.

window.addEventListener("load", externalLinks);

The load event can take some time to fire, so preferable to that is the DOMContentLoaded event instead. That way the code gets to run while the page is still loading and downloading things.

window.addEventListener("DOMContentLoaded", externalLinks);

To help with organising the code, an event handler function is used. The job of an event handler is to decide when it’s appropriate to call the externalLinks, and to pass needed information to it. Checking tht getElementsByTagName exists is a good example of this.

function externalLinksHandler() {
    if (document.getElementsByTagName) {
        updateExternalLinks();
    }
}
window.addEventListener("DOMContentLoaded", externalLinksHandler);

We then have the function externalLinks, which I’ve renamed to updateExternalLinks for clarity.

To help cope with the bulk of effort, I’ll run that function through JSLint and update it resolve all issues found. This is where a decision occurs. Do I use const and let instead of var? Not now because you are checking that getElementsByTagName exists. You might have a genuine need to cater for some ancient browsers.

function updateExternalLinks() {
    var anchors = document.getElementsByTagName("a");
    var anchor;
    var isHref;
    var isExternal;
    var i = 0;
    while (i < anchors.length) {
        anchor = anchors[i];
        isHref = anchor.getAttribute("href") > "";
        isExternal = anchor.getAttribute("rel") === "external";
        if (isHref && isExternal) {
            anchor.target = "_blank";
        }
        i += 1;
    }
}

Instead of the while loop (which used to be a for loop) I’ll use a forEach method instead, letting me move some var statements into the function.

function updateExternalLinks() {
    var anchors = document.getElementsByTagName("a");
    anchors.forEach(function updateExternalLink(anchor) {
        var isHref = anchor.getAttribute("href") > "";
        var isExternal = anchor.getAttribute("rel") === "external";
        if (isHref && isExternal) {
            anchor.target = "_blank";
        }
    });
}

But getElementsByTagName doesn’t let us use forEach. We should replace getElementsByTagName with querySelectorAll instead.

function updateExternalLinks() {
    const anchors = document.querySelectorAll("a");
    ...
}
function externalLinksHandler() {
    if (document.querySelectorAll) {
        updateExternalLinks();
    }
}

The forEach function can be moved out to be a separate function, along with its variables.

function updateExternalLink(anchor) {
    var isHref = anchor.getAttribute("href") > "";
    var isExternal = anchor.getAttribute("rel") === "external";
    if (isHref && isExternal) {
        anchor.target = "_blank";
    }
}
function updateExternalLinks() {
    var anchors = document.querySelectorAll("a");
    anchors.forEach(updateExternalLink);
}

When it comes to dealing with different types of links, such as your amazon class, I would make that explicitly clear in the code by using guard clauses, using multiple _blank statements.

function updateExternalLink(anchor) {
    var isHref = anchor.getAttribute("href") > "";
    var isExternal = anchor.getAttribute("rel") === "external";
    var isAmazon = anchor.classList.contains("amazon");
    if (isHref && isExternal) {
        anchor.target = "_blank";
        return;
    }
    if (isHref && isAmazon) {
        anchor.target = "_blank";
        return;
    }
}

That way the duplicate code is more apparent, and if need be can be extracted out to a separate function.

function addBlankToOpenInNewTab(anchor) {
    anchor.target = "_blank";
}
function updateExternalLink(anchor) {
    var isHref = anchor.getAttribute("href") > "";
    var isExternal = anchor.getAttribute("rel") === "external";
    var isAmazon = anchor.classList.contains("amazon");
    if (isHref && isExternal) {
        return addBlankToOpenInNewTab(anchor);
    }
    if (isHref && isAmazon) {
        return addBlankToOpenInNewTab(anchor);
    }
}

As for the http requirement, we shouldn’t keep isHref because something different is being checked for. Instead, we update isHref to be isHttp instead.

function updateExternalLink(anchor) {
    var isHttp = anchor.getAttribute("href").substr(0, 4) === "http";
    var isExternal = anchor.getAttribute("rel") === "external";
    var isAmazon = anchor.classList.contains("amazon");
    if (isHttp && isExternal) {
        return addBlankToOpenInNewTab(anchor);
    }
    if (isHttp && isAmazon) {
        return addBlankToOpenInNewTab(anchor);
    }
}

Here’s the full code that we end up with:

function addBlankToOpenInNewTab(anchor) {
    anchor.target = "_blank";
}
function updateExternalLink(anchor) {
    var isHttp = anchor.getAttribute("href").substr(0, 4) === "http";
    var isExternal = anchor.getAttribute("rel") === "external";
    var isAmazon = anchor.classList.contains("amazon");
    if (isHttp && isExternal) {
        return addBlankToOpenInNewTab(anchor);
    }
    if (isHttp && isAmazon) {
        return addBlankToOpenInNewTab(anchor);
    }
}
function updateExternalLinks() {
    var anchors = document.querySelectorAll("a");
    anchors.forEach(updateExternalLink);
}
function externalLinksHandler() {
    if (document.querySelectorAll) {
        updateExternalLinks();
    }
}
window.addEventListener("DOMContentLoaded", externalLinksHandler);

Yes it is longer, but we can afford that. Each section deals with a different concept and helps to reduce the overall complexity, making it easier to understand what each part does.

1 Like

Are your checks for getElementsByTagName intended to cater for prehistoric Netscape 4 web browsers? Or are they are leftover from using older code?

If you aren’t intending to cater for prehistoric web browsers we could then give advice about using other techniques that are supported by the vast majority of web browsers these days.

For example, when forEach methods are being used, there’s absolutely no benefit from checking that getElementsByTagName or querySelectorAll exist. Those checks can be completely removed.

function externalLinksHandler() {
    updateExternalLinks();
}

Also, by moving your script tag to the bottom of the body, you don’t need any load event handler and can just run the code.

// function externalLinksHandler() {
//         updateExternalLinks();
// }
// window.addEventListener("DOMContentLoaded", externalLinksHandler);

updateExternalLinks();

We can use a regex to check for http, and guard clause to return early out of the function.

function updateExternalLink(anchor) {
    const isHttp = /^http.*/.test(anchor.href);
    if (!isHttp) {
        return;
    }
    ...
}

And when we are dealing with web browsers from about 2015 onwards, we can use const instead of var, and other ES6 features too, helping to make the updateExternalLink function easier to understand:

function updateExternalLink(anchor) {
    ...
    const isHttp = anchor.href.substr(0, 4) === "http";
    const isExternal = anchor.rel === "external";
    if (isHttp && isExternal) {
        return addBlankToOpenInNewTab(anchor);
    }
    const isAmazon = anchor.classList.contains("amazon");
    if (isHttp && isAmazon) {
        return addBlankToOpenInNewTab(anchor);
    }
}

The different checks can also be put into an array, so that we can loop through them checking if any of them show that we have an external link.

function updateExternalLink(anchor) {
    ...
    const externalChecks = [
        (anchor) => anchor.rel === "external",
        (anchor) => anchor.classList.contains("amazon")
    ];
    const isExternalLink = externalChecks.some(
        (checkFn) => checkFn(anchor)
    );
    if (isExternalLink) {
        addBlankToOpenInNewTab(anchor);
    }
}

And of course, those checks for if it’s an external link, can be extracted out to a separate function.

function isExternalLink(anchor) {
    const isHttp = /^http.*/.test(anchor.href);
    if (!isHttp) {
        return;
    }
    const externalChecks = [
        (anchor) => anchor.rel === "external",
        (anchor) => anchor.classList.contains("amazon")
    ];
    return externalChecks.some(
        (checkFn) => checkFn(anchor)
    );
}
function updateExternalLink(anchor) {
    if (isExternalLink(anchor)) {
        addBlankToOpenInNewTab(anchor);
    }
}

If you’re not comfortable with arrow-notation you can easily replace that too with a simple function:

// (anchor) => anchor.rel === "external"
function (anchor) {
    return anchor.rel === "external"
}

But we don’t need those fancy checks. It can just be a few simple conditions instead:

function isExternalLink(anchor) {
    const isHttp = /^http.*/.test(anchor.href);
    if (!isHttp) {
        return;
    }
    return (
        anchor.rel === "external" ||
        anchor.classList.contains("amazon")
    );
}

The updateExternalLink function is now reduced down to about the ultimate of simplicity, with a simple condition and a simple outcome.

Here’s the updated code:

function addBlankToOpenInNewTab(anchor) {
    anchor.target = "_blank";
}
function isExternalLink(anchor) {
    const isHttp = /^http.*/.test(anchor.href);
    if (!isHttp) {
        return false;
    }
    return (
        anchor.rel === "external" ||
        anchor.classList.contains("amazon")
    );
}
function updateExternalLink(anchor) {
    if (isExternalLink(anchor)) {
        addBlankToOpenInNewTab(anchor);
    }
}
function updateExternalLinks() {
    var anchors = document.querySelectorAll("a");
    anchors.forEach(updateExternalLink);
}

updateExternalLinks();

I hope that some of this is of benefit to you.

2 Likes

Thank you very much, Dave and Paul.
I didn’t manage to use your code, trying as following (with as less code as possible)

function externalLinks() {  
document.querySelectorAll('a').forEach(function(link) {
  if (link.getAttribute("rel") == 'external' || 
      link.getAttribute("class") == "amazon" || 
  //  link.getAttribute("href").includes('https://')) [not working]
  //  anchor.getAttribute("href").substr(0, 4) === "http";  [not working]
  {
         link.target = "_blank";
  }  
})
}

But, what I didn’t explain enough it that if every http/https worked as external (_blank), I could avoid both rel=external and class=amazon.
Thank you!

Sorry if I am being dense, but doesn’t that include external rels and amazon classes?

Would this not suffice?

function isExternalLink(anchor) {
    return (/^https?:\/{2}/.test(anchor.href))
}

Then not as clean and declarative as Paul’s solution, but…

function updateExternalLinks() {  
  document.querySelectorAll('a')
    .forEach(
      function(anchor) {
        if (isExternalLink(anchor)) anchor.target = '_blank'
      }
    )
}

Just playing now:)

Simple enough using a more general function in place of updateExternalLinks

function updateElements(selector, callback) {           
  document.querySelectorAll(selector).forEach(callback)
}

As we are using querySelectorAll we can use a css selector to target the links

function targetToBlank(anchor) { anchor.target = '_blank' }

updateElements('a[href^="http"]', targetToBlank)

or to target the amazon links

updateElements(
  'a[href^="http"].amazon',
  function (anchor) { 
    anchor.classList.add('amazon-link')
  }
)

2 Likes

Yes, this works (and with a clear and simple code).
Thank you very much! :slightly_smiling_face:

1 Like