Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

target attribute being broken #19

Open
stravanato opened this issue Dec 1, 2014 · 3 comments
Open

target attribute being broken #19

stravanato opened this issue Dec 1, 2014 · 3 comments

Comments

@stravanato
Copy link

This breaks 'target' attribute of links (a link with target '_blank' will be opened in the same window):

/**
If this link is not opened in a new tab or window, we need to add
a small delay so the event can fully fire. See:
http://support.google.com/analytics/bin/answer.py?hl=en&answer=1136920 
* We're actually checking for modifier keys or middle-click
*/
if ( ! ( e.metaKey || e.ctrlKey || 1 == e.button ) ) {
  e.preventDefault();
  setTimeout('document.location = "' + $(this).attr('href') + '"', 100);
}

Is it really necessary to add that delay?

@aaroncampbell
Copy link
Owner

It definitely was when it was added. The problem was that the way Google was logging events, there was a slight delay because of some pre-processing, and the href part of the link would be followed and you'd leave the page before the javascript to trigger the event actually happened.

The URL linked in the comment used to actually warn of this problem, but now it looks like they've updated their docs to show the proper way to do this with their new Univeral Analytics. It looks like if we use those we can eliminate the need for the delay. I'm not sure about the standard ga.js stuff that's currently being used. I haven't tested it in quite a while.

We could probably change our selector for this from $('a:external') to '$(a:external:not([target]), a:external[target="_self"]') to target only external links that are also targeting the current frame.

@stravanato
Copy link
Author

Hi, Aaron, thanks a lot.
Firstly I commented those 4 lines but yours is the correct solution.
I think you made a typo with the first single quote and it should be:
$('a:external:not([target]), a:external[target="_self"]')
Again, thank you.

@stravanato
Copy link
Author

Hi, Aaron,
one more thing you could be interested. I got errors when it tries to match objects that don't have href attribute, so I changed these lines like this:

// Adds :external for grabbing external links
$.expr[':'].external = function(obj) {
  return obj.href ? !obj.href.match(/^mailto\:/) && !obj.href.match(/^javascript\:/) && (obj.hostname != location.hostname) : false;
  // return !obj.href.match(/^mailto\:/) && !obj.href.match(/^javascript\:/) && (obj.hostname != location.hostname);
};

Hope this is useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants