Hi

I use highslide library on my site to display images in the overlay and internally they use links in the following format to allow user to zoom in into the image:

<a class="highslide-full-expand" href="javascript:hs.expanders[0].doFullExpand();" title="Zoom In" style="display: block;"></a>

By looking at the googleanalytics.js file I figured that this link will be treated as external link, even though it's not and will screw reports on google analytics side by storing javascript:hs.expanders[0].doFullExpand(); as the name of the link.

I will attach the patch with additional check before firing _trackEvent for external links. I hope you will consider it or provide better approach.

Thanks a lot!

Files: 
CommentFileSizeAuthor
#18 1425358+Track+overlays+not+as+downloads+or+external2.patch911 byteshass
PASSED: [[SimpleTest]]: [MySQL] 179 pass(es).
[ View ]
#17 1425358+Track+overlays+not+as+downloads+or+external.patch1.35 KBhass
PASSED: [[SimpleTest]]: [MySQL] 179 pass(es).
[ View ]
#15 1425358+Track+overlays.patch1.34 KBhass
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]
#13 1425358+Track+overlays.patch1.31 KBhass
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]
#12 1425358+Track+overlays.patch1.32 KBhass
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]
#9 1425358+Track+overlays.patch1.34 KBhass
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1425358+Track+overlays.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 external_links_1425358_6.patch645 bytesgansbrest
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]
#3 external_links_1425358_3.patch693 bytesgansbrest
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch external_links_1425358_3.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#1 external_links_1425358.patch689 bytesgansbrest
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch external_links_1425358.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

StatusFileSize
new689 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch external_links_1425358.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Patch is attached.

Status:Active» Needs work

I've seen this myself to with colorbox... But https? Is not ok as it could also be ftp and others.

StatusFileSize
new693 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch external_links_1425358_3.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

What about this one? It should match most common protocols, but skip those like javascript:bla

Version:6.x-3.x-dev» 7.x-1.x-dev
Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, external_links_1425358_3.patch, failed testing.

StatusFileSize
new645 bytes
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]

Ok, here is the patch for D7 and previous one from #3 was for D6, that's why it failed last time.

Status:Needs work» Needs review

You always need to change the issue status

Priority:Normal» Critical

Required for next release

Title:Problem with tracking external linksTrack overlays not as downloads or external
StatusFileSize
new1.34 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1425358+Track+overlays.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

What do you think about this generic solution. We just need to add a configuration option in the administration UI for the classes or just search through all lightbox type modules and add the appropriate classes from these modules. For testing I have added your highslide-full-expand class and the colorbox. In your situation the title is not that cool, but for colorbox it is really helpful. We just need to decide if an overlay is a page view or an event. I think it's more an event except a few edge cases I can think of, but who cares.

Status:Needs review» Needs work

The last submitted patch, 1425358+Track+overlays.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]

New patch.

StatusFileSize
new1.31 KB
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]

Removed tab

There are ways to achieve this without needing to alter the module.

Your current code has no fall-back if JavaScript is either not loaded yet or disabled (for whatever reason).
<a class="highslide-full-expand" href="javascript:hs.expanders[0].doFullExpand();" title="Zoom In" style="display: block;"></a>

If you did this:
<a class="highslide-full-expand" href="/bigger-version-of-my-image.jpg" title="Zoom In" style="display: block;"></a>

And then attach an event via script you get two bonuses, a link to the bigger image that doesn't require script and also doesn't track as an external link.

Better still:
If the feature you are implementing requires script and you don't need a fall-back you should inject the link via script and hook its click event to trigger your overlay. This way users with working script get an extra feature added, users with script disabled or not loaded yet (everyone is a non-JavaScript user till it loads) don't get a broken link which does nothing when clicked.

StatusFileSize
new1.34 KB
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es).
[ View ]

Fixed a missing ].

StatusFileSize
new1.35 KB
PASSED: [[SimpleTest]]: [MySQL] 179 pass(es).
[ View ]

New patch with #1801046: Invalid jQuery selectors causing errors in IE8 using updated jQuery (1.7 + ) in mind. CNW as Colorbox has some special features we can use, too.

StatusFileSize
new911 bytes
PASSED: [[SimpleTest]]: [MySQL] 179 pass(es).
[ View ]

After several tries I'm back to the a match regex on the external links. This does not solve the colorbox issue.

Status:Needs review» Needs work

The last submitted patch, 1425358+Track+overlays+not+as+downloads+or+external2.patch, failed testing.

Status:Needs work» Needs review

Title:Track overlays not as downloads or externalOnly track full-qualified links as external, block "javascript:" and maybe other
Status:Needs review» Fixed

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.