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

gansbrest’s picture

FileSize
689 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.

hass’s picture

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.

gansbrest’s picture

FileSize
693 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

hass’s picture

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.

gansbrest’s picture

FileSize
645 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.

hass’s picture

Status: Needs work » Needs review

You always need to change the issue status

hass’s picture

Priority: Normal » Critical

Required for next release

hass’s picture

Title: Problem with tracking external links » Track overlays not as downloads or external
FileSize
1.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.

hass’s picture

Status: Needs review » Needs work

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

hass’s picture

Status: Needs work » Needs review
FileSize
1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es). View

New patch.

hass’s picture

FileSize
1.31 KB
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es). View

Removed tab

drinkynet’s picture

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.

hass’s picture

FileSize
1.34 KB
PASSED: [[SimpleTest]]: [MySQL] 157 pass(es). View

Fixed a missing ].

hass’s picture

hass’s picture

FileSize
1.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.

hass’s picture

FileSize
911 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.

hass’s picture

Status: Needs work » Needs review
hass’s picture

Title: Track overlays not as downloads or external » Only 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.