plausible-tracker: enableAutoOutboundTracking breaks target="_blank" and probably noopener security

This is a pretty critical issue, is there any plan to assign it to someone?

Can you please consider prioritizing this issue higher? It’s still a fairly critical issue today. Just got a complaint from one of my customers about this.

Hello,

Since this package have not been update for two years, I made a fork.

I rewrite the tracker to be more modular and I fix this issue.

https://github.com/Barbapapazes/plausible-tracker

Opened #21 with a fix. It has no tests currently so it’s not ready for merge yet but I hope it helps as a start.

For anyone needing this feature right now, I’m using my fixed fork in a project of mine with git submodule. Let yourself get inspired ((no warranties)) 😄

Not sure if this belongs here, but enableAutoOutboundTracking also broke an interactive site of ours: We had a simple anchor (without href) with onclick event handler attached. The event handler would trigger the opening of a modal. After clicking the link, however, the entire page would reload, killing the modal that was just opening again. Assume this line to be the culprit: https://github.com/plausible/plausible-tracker/blob/a5f8e946263b4e9898c729293f74a095ef71cfcb/src/lib/tracker.ts#L288

So how about this problem?

@smuuf The script prevents the browser from executing the default behavior (which is to immediately switch sites (as fast as possible)), to quickly send an analytics request. Then it waits for a small delay in which the analytics request can be completed. Afterwards, the default behavior has to be invoked manually because it was suppressed originally, therefore a write to location.href is necessary to actually do the navigation that one would expect from a link click by default.

@smuuf @maximedupre as mentioned in my previous comment, there is an open PR to fix this (#21), which you can use by getting inspired from the links in my comment.

@ everyonewhocares The only thing missing to allow #21 to be merged is tests as far as I understand. I do not have the time or interest to dive into the testing infrastructure used in this project but if you do, feel free to comment how to approach them or outright implement some tests on top of my PR (e.g. in your own fork, could then supersede my PR).

No worries 👍🏼 in that case I might submit a PR for your consideration when you have time.

thanks @Joelius300! it’s something we may be able to look at but i cannot make any promise. all of our development resources are focused on the data import and performance improvements at the moment.

Hoping I haven’t included a typo here, yes it worked as following:

document.addEventListener('click', (event) => {
  let link = event.target;
  while (link && (typeof link.tagName === 'undefined' || link.tagName.toLowerCase() !== 'a' || !link.href)) {
    link = link.parentNode;
  }

  if (link && link.href && link.host && link.host !== location.host) {
    const props = { url: link.href };

    console.debug('tracking now', {props});
    window.plausible('Outbound Link: Click', { props });

    // Allow event to be sent before the page is unloaded
    if (!link.target || link.target.match(/^_(self|parent|top)$/i)) {
      setTimeout(function() {
        console.debug('navigating now');
        if (link.target === '_top') window.top.location.href = link.href;
        if (link.target === '_parent') window.parent.location.href = link.href;
        location.href = link.href;
      }, 150);
      event.preventDefault();
    }
  }
})

I understand. But I do think this would warrant an entry in the docs to warn users that this function might break functionality.