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!

CommentFileSizeAuthor
#18 1425358+Track+overlays+not+as+downloads+or+external2.patch911 byteshass
#17 1425358+Track+overlays+not+as+downloads+or+external.patch1.35 KBhass
#15 1425358+Track+overlays.patch1.34 KBhass
#13 1425358+Track+overlays.patch1.31 KBhass
#12 1425358+Track+overlays.patch1.32 KBhass
#9 1425358+Track+overlays.patch1.34 KBhass
#6 external_links_1425358_6.patch645 bytesgansbrest
#3 external_links_1425358_3.patch693 bytesgansbrest
#1 external_links_1425358.patch689 bytesgansbrest

Comments

gansbrest’s picture

gansbrest commented
StatusFileSize
new external_links_1425358.patch689 bytes

Patch is attached.

hass’s picture

hass commented
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

gansbrest commented
StatusFileSize
new external_links_1425358_3.patch693 bytes

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

hass’s picture

hass commented
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

gansbrest commented
StatusFileSize
new external_links_1425358_6.patch645 bytes

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

hass commented
Status: Needs work » Needs review

You always need to change the issue status

hass’s picture

hass commented
Priority: Normal » Critical

Required for next release

hass’s picture

hass commented
Title: Problem with tracking external links » Track overlays not as downloads or external
StatusFileSize
new 1425358+Track+overlays.patch1.34 KB

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

hass commented

Marked #1801020: Track modal dialogs scripts as pageview as duplicate.

Status: Needs review » Needs work

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

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new 1425358+Track+overlays.patch1.32 KB

New patch.

hass’s picture

hass commented
StatusFileSize
new 1425358+Track+overlays.patch1.31 KB

Removed tab

drinkynet’s picture

drinkynet commented

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

hass commented
StatusFileSize
new 1425358+Track+overlays.patch1.34 KB

Fixed a missing ].

hass’s picture

hass commented

#1805660: Style integration from other modules?, FAQ Track ColorBox usage with Google Analytics

hass’s picture

hass commented
StatusFileSize
new 1425358+Track+overlays+not+as+downloads+or+external.patch1.35 KB

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

hass commented
StatusFileSize
new 1425358+Track+overlays+not+as+downloads+or+external2.patch911 bytes

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

hass commented
Status: Needs work » Needs review

#18: 1425358+Track+overlays+not+as+downloads+or+external2.patch queued for re-testing.

hass’s picture

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

D7: http://drupalcode.org/project/google_analytics.git/commit/9a21eff
D6.4: http://drupalcode.org/project/google_analytics.git/commit/5626916
D6.3: http://drupalcode.org/project/google_analytics.git/commit/b3bfe68

Status: Fixed » Closed (fixed)

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