Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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!
Comment | File | Size | Author |
---|---|---|---|
#18 | 1425358+Track+overlays+not+as+downloads+or+external2.patch | 911 bytes | hass |
#17 | 1425358+Track+overlays+not+as+downloads+or+external.patch | 1.35 KB | hass |
#15 | 1425358+Track+overlays.patch | 1.34 KB | hass |
#13 | 1425358+Track+overlays.patch | 1.31 KB | hass |
#12 | 1425358+Track+overlays.patch | 1.32 KB | hass |
Comments
Comment #1
gansbrest CreditAttribution: gansbrest commentedPatch is attached.
Comment #2
hass CreditAttribution: hass commentedI've seen this myself to with colorbox... But https? Is not ok as it could also be ftp and others.
Comment #3
gansbrest CreditAttribution: gansbrest commentedWhat about this one? It should match most common protocols, but skip those like javascript:bla
Comment #4
hass CreditAttribution: hass commentedComment #6
gansbrest CreditAttribution: gansbrest commentedOk, here is the patch for D7 and previous one from #3 was for D6, that's why it failed last time.
Comment #7
hass CreditAttribution: hass commentedYou always need to change the issue status
Comment #8
hass CreditAttribution: hass commentedRequired for next release
Comment #9
hass CreditAttribution: hass commentedWhat 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.Comment #10
hass CreditAttribution: hass commentedMarked #1801020: Track modal dialogs scripts as pageview as duplicate.
Comment #12
hass CreditAttribution: hass commentedNew patch.
Comment #13
hass CreditAttribution: hass commentedRemoved tab
Comment #14
drinkynet CreditAttribution: drinkynet commentedThere 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.
Comment #15
hass CreditAttribution: hass commentedFixed a missing
]
.Comment #16
hass CreditAttribution: hass commented#1805660: Style integration from other modules?, FAQ Track ColorBox usage with Google Analytics
Comment #17
hass CreditAttribution: hass commentedNew 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.
Comment #18
hass CreditAttribution: hass commentedAfter several tries I'm back to the a match regex on the external links. This does not solve the colorbox issue.
Comment #20
hass CreditAttribution: hass commented#18: 1425358+Track+overlays+not+as+downloads+or+external2.patch queued for re-testing.
Comment #21
hass CreditAttribution: hass commentedD7: 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