943541 - Click on Tabzilla triggers Firefox download on /firefox/new/

Status: RESOLVED FIXED

(www.mozilla.org :: Analytics, defect)

Description

[Replica @ModReplica]

Reported by Alexi Mihai via Mozilla Facebook:

I don't really now were I should write, but I've found a small bug on the official website mozilla.org. Maybe you could redirect the message.

Browser: Version 31.0.1650.57 m
OS: Windows 8

I entered on the main page to download the software, clicked on the button and downloaded mozilla.
I got this link: http://www.mozilla.org/en-US/firefox/new/?utm_expid=71153379-28.fvIED7HGROyjOifCfo3hhQ.0&utm_referrer=https%3A%2F%2Fwww.google.hu%2F#download-fx

And the bug is here: When I'm clicking on the top button "mozilla" (<header id="masthead">) it refreshes the page and start to download the program again. And it downloads as many times as I press this button.

Have fun!

Comment 1

[Kohei]

I can reproduce the issue with Chrome and Safari.

Comment 2

[Alex Gibson [:agibson]]

After some debugging it looks like this is a result of a GA bug:

Our helper function in bedrock uses a method called hitCallback:

https://github.com/mozilla/bedrock/blob/master/media/js/base/global.js#L213

This is supposed to fire after only the next GA event, but it looks to be firing after all subsequent events too. In this case, the Tabzilla open/close GA events.

The solution would be either:

1.) not to use hitCallback in the GA helper any longer, and let it fall back to a regular setTimeout

2.) not to use the GA helper method on the download page, and use a custom call that will not trigger hitCallback.

I'd perhaps edge toward 1 - because hitCallback was included under the assumption that it only fires after the next event. Removing it would result in slightly slower page redirect times after clicking a link however.

Comment 3

[Alex Gibson [:agibson]]

Any thoughts on this one jpetto?

Comment 4

[Kohei]

3.) migrating to Universal Analytics

https://developers.google.com/analytics/devguides/collection/analyticsjs/advanced#hitCallback
https://www.domsammut.com/code/setting-up-hitcallback-using-google-universal-analytics

Comment 5

[Kohei]

4.) adding a callback function just after a queue:

http://stackoverflow.com/a/13179408

I didn't know the technique but I think this is the right option for us.

Comment 6

[Alex Gibson [:agibson]]

(In reply to Kohei Yoshino [:kohei] from comment #4)
> 3.) migrating to Universal Analytics

From the info here there's nothing I can see to suggest that hitCallback behaves any differently in universal analytics to the way it does currently.

(In reply to Kohei Yoshino [:kohei] from comment #5)
> 4.) adding a callback function just after a queue:
> 
> http://stackoverflow.com/a/13179408

This one looks interesting! (and simple) - would need some testing though to make everything is working ok

Comment 7

[Kohei]

Pushing Functions onto the Queue
https://developers.google.com/analytics/devguides/collection/gajs/#PushingFunctions

The pushing function technique worked well locally with GA_ACCOUNT_CODE. I tested both gaTrack() in global.js and Infobar.prototype.trackEvent() in tabzilla.js which I have been working on.

About Bug 877849: looks like I had written utility functions like this. I'm checking if those are still useful.
https://github.com/kyoshino/bedrock/commit/0113a73

Comment 8

[Kohei]

Attachment: PR on GitHub

Comment 9

[Chris More [:cmore]]

All: we are not migrating to universal analytics (for various reasons and advice from Google) to at least mid Q1 2014 and thus it shouldn't be an open here to resolve the tabzilla issue.

Comment 10

[Jonathan Petto [:jpetto]]

After a bit of testing locally, it appears the issue wasn't a bug, but instead (intentional?) functionality explicitly tied to our implementation setting a callback function.

Adding a callback in the following manner seems to be global - that is, 'my_callback' will be fired after every track event until/unless a new callback is set.

window._gaq.push(['_set', 'hitCallback', my_callback]);

For example, given the following code in our current implementation:

function callback() {
  console.log('callback fired');
}

$('#ga1').click(function(e) {
  e.preventDefault();
  gaTrack(['_trackEvent', 'TESTING 1', 'TESTING 1', 'TESTING 1'], callback);
});

$('#ga2').click(function(e) {
  e.preventDefault();
  gaTrack(['_trackEvent', 'TESTING 2', 'TESTING 2', 'TESTING 2']);
});

Clicking '#ga2', then '#ga1', then '#ga2' again will result in the following:

_gaq.push processing "_trackEvent" for args: "[TESTING 2,TESTING 2,TESTING 2]":
_gaq.push processing "_trackEvent" for args: "[TESTING 1,TESTING 1,TESTING 1]":
callback fired!
_gaq.push processing "_trackEvent" for args: "[TESTING 2,TESTING 2,TESTING 2]":
callback fired!

Kohei's solution seems to solve the issue - the pushing function technique only fires 'callback' once after the specified track action. Subsequent calls to 'gaTrack' do not remember the originally set 'callback' function.

Comment 11

[Jonathan Petto [:jpetto]]

A little more info...

/firefox/new/ is the only page that has a gaTrack call with a callback that *doesn't* send the user to another page. The bug exists in global.js (so, pretty much every page on the site), but only has the conditions to appear on /firefox/new/.

Did I mention yet that hitCallback isn't documented for our version of GA? :P

Comment 12

[Jonathan Petto [:jpetto]]

Put the branch on demo1 so Gareth can confirm GA is still working as expected:

https://www-demo1.allizom.org/en-US/firefox/new/

Comment 13

[Alex Gibson [:agibson]]

As a side note: Would be great to add an additional unit test for this in the global.js spec, to ensure a callback is not executed by subsequent GA events.

Thanks for the PR Kohei!

Comment 14

[Gareth Cull [:garethc]]

Hi,

I've started testing clicks within Tabzilla and am finding some issues. Specifically:

1) When I open Tabzilla and immediately select a link (eg. /mission/, /about/) i am not registering the second event click on the link in GA. This seems to be impacting the links that drive traffic around  www.mozilla.org vs. those driving traffic to other mozilla sites and sub-domains (eg. webmaker.org)
2) Can you double check to make sure all Tabzilla interactions are being classified under the Tabzilla Event Category. Early in testing i found that some clicks were being captured in '/new Interactions', however can't seem to replicate this.

Searching within Tabzilla seems to be capturing data correctly.

Comment 15

[Kohei]

(In reply to Jon Petto [:jpetto] from comment #10)
> Adding a callback in the following manner seems to be global - that is,
> 'my_callback' will be fired after every track event until/unless a new
> callback is set.
> 
> window._gaq.push(['_set', 'hitCallback', my_callback]);

I should have noticed the issue earlier...

(In reply to Alex Gibson [:agibson] from comment #13)
> As a side note: Would be great to add an additional unit test for this in
> the global.js spec, to ensure a callback is not executed by subsequent GA
> events.

Added a unit test to my PR.

(In reply to Gareth Cull [:garethc] from comment #14)
> 1) When I open Tabzilla and immediately select a link (eg. /mission/,
> /about/) i am not registering the second event click on the link in GA. This
> seems to be impacting the links that drive traffic around  www.mozilla.org
> vs. those driving traffic to other mozilla sites and sub-domains (eg.
> webmaker.org)

I cannot reproduce the issue...

> 2) Can you double check to make sure all Tabzilla interactions are being
> classified under the Tabzilla Event Category. Early in testing i found that
> some clicks were being captured in '/new Interactions', however can't seem
> to replicate this.

new.js is logging outbound links in the same way as tabzilla.js. Because of this, _trackEvent is called twice on click links in Tabzilla. I have fixed the issue in new.js by excluding links in Tabzilla:
https://github.com/mozilla/bedrock/pull/1465#issuecomment-29666864

Comment 16

[Jonathan Petto [:jpetto]]

Fixes have been pushed to demo 3:

https://www-demo3.allizom.org/en-US/firefox/new/

Gareth - Can you test again?

Comment 17

[Gareth Cull [:garethc]]

Thanks guys. Events look like they are coming through now.

Here's a screencap of the events being triggered: http://cl.ly/image/0u1i3o2I3B0v

Comment 18

[[github robot]]

Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/b23c68f60a5663076bc7252714ed75b8a15aca40
Bug 943541 - Click on Tabzilla triggers Firefox download on /firefox/new/

https://github.com/mozilla/bedrock/commit/adbbffb871dbd383ed1ef28a7ad3d3609746db06
Merge pull request #1465 from kyoshino/bug-943541-fix-ga-callback

Bug 943541 - Click on Tabzilla triggers Firefox download on /firefox/new/

Comment 19

[raymond [:retornam] (inactive)]

fixed on stage https://www.allizom.org/en-US/firefox/new/