Submitting the current webpage to a bookmarking site

Question

I would like to hear your feedback on the link below that lets users add a link to a website. I read that javascript:void(0) is suggested instead of a # for the <a href="" attribute. Is this still the best practice?

<a href="javascript:void(0)" onclick="window.open('https://www.example.com/add?url='+encodeURIComponent(window.location)+'&text='+encodeURIComponent(document.title))" title="Add link">Add Link</a>

Answer 1

First of all, void is an operator, not a function. The parentheses are extraneous; it should be written as void 0.

href="#" should be avoided for the reasons given in the answer you linked; people will forget to cancel the event and the page will jump to the top when users click the link, or worse, if the page uses a base tag, the page will navigate somewhere else.

Most people will tell you to avoid putting JavaScript in tag attributes at all, instead binding events to the element later on, once the page has been loaded, or binding the event to a parent element and delegating.

If you're going to use an inline onclick attribute anyway, and there's nothing appropriate to put in the href attribute, I'd consider not making it a link at all. Make it a button or some other kind of element, possibly styled to look like a hyperlink.

As a final note, GET requests should be safe and idempotent, meaning following a link or visiting a URL should not cause changes on your server / in your database. https://www.example.com/add?... looks suspiciously like it breaks that rule; you may be in a world of trouble if spiders start crawling those links. Making a POST request, for example by submitting a form, would be the proper way to handle this.

Answer 2

I suggest you take the addEventListener/attachEvent approach instead of using inline scripts. They are much cleaner and upholds separation of concerns. Here's a simple patch which makes it crossbrowser.

//assuming NS is an object, your namespace
NS.addEL = (function binding(){
  if(window.addEventListener){
    return function addEventListener(el, ev, handler){
      el.addEventListener(ev, handler);
    }
  } else if(window.attachEvent) {
    return function attachEvent(el, ev, handler){
      el.attachEvent('on' + ev, handler);
    }
  } else {
    return function(){/*not supported for some reason*/}
  }
}());

Using the said patch, you can have a cleaner HTML. Using # is a safeguard that prevents the link to point to nowhere, but the page jumps up. This will be your "last line of defense" from going somewhere.

<a href="#" id="link" title="Add link">Add Link</a>

And the JS that goes with it, we use our patch. Our main defense against the link moving the page away is event.preventDefault.

Another way is returning false after the entire operation. This prevents eveything, and I mean everything that happens after it. It's link calling event.preventDefault() and event.stopPropagation. You may be doing some delegation, so this might not be advisable.

I suggest using href="#" and event.preventDefault to stop the link from going somewhere.

var link = document.getElementById('link');

NS.addEL(link,'click',function(event){

  //prevent the link from moving us away
  event.preventDefault();

  window.open('https://www.example.com/add?url='+encodeURIComponent(window.location)+'&text='+encodeURIComponent(document.title))

  //one way to prevent default, but stops propagation and everything else
  return false;
});

If this is some tracker code, which displays nothing on the opened window, I suggest you route this to an iframe instead.