From b23c68f60a5663076bc7252714ed75b8a15aca40 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 2 Dec 2013 19:59:38 -0500 Subject: [PATCH] Bug 943541 - Click on Tabzilla triggers Firefox download on /firefox/new/ --- .../tabzilla/templates/tabzilla/tabzilla.js | 10 ++-------- media/js/base/global.js | 18 +++++------------- media/js/firefox/new.js | 5 +++-- media/js/firefox/os/firefox-os.js | 5 +---- media/js/test/spec/global.js | 13 +++++++++++++ 5 files changed, 24 insertions(+), 27 deletions(-) diff --git a/bedrock/tabzilla/templates/tabzilla/tabzilla.js b/bedrock/tabzilla/templates/tabzilla/tabzilla.js index 390d6f97b7..1deafe8388 100644 --- a/bedrock/tabzilla/templates/tabzilla/tabzilla.js +++ b/bedrock/tabzilla/templates/tabzilla/tabzilla.js @@ -257,10 +257,7 @@ var Tabzilla = (function (Tabzilla) { } else { e.preventDefault(); timer = setTimeout(callback, 500); - window._gaq.push( - ['_set', 'hitCallback', callback], - ['_trackEvent', 'Tabzilla', 'click', href] - ); + window._gaq.push(['_trackEvent', 'Tabzilla', 'click', href], callback); } } }); @@ -280,10 +277,7 @@ var Tabzilla = (function (Tabzilla) { if (typeof(_gaq) == 'object' && keyword !== '') { timer = setTimeout(callback, 500); - window._gaq.push( - ['_set', 'hitCallback', callback], - ['_trackEvent', 'Tabzilla', 'search', keyword] - ); + window._gaq.push(['_trackEvent', 'Tabzilla', 'search', keyword], callback); } else { $form.submit(); } diff --git a/media/js/base/global.js b/media/js/base/global.js index ccc35e544a..24b70e044c 100644 --- a/media/js/base/global.js +++ b/media/js/base/global.js @@ -202,6 +202,8 @@ function gaTrack(eventArray, callback) { }; } if (typeof(window._gaq) === 'object') { + // send event to GA + window._gaq.push(eventArray); // Only set up timer and hitCallback if a callback exists. if (hasCallback) { // Failsafe - be sure we do the callback in a half-second @@ -210,20 +212,10 @@ function gaTrack(eventArray, callback) { // But ordinarily, we get GA to call us back immediately after // it finishes sending our things. - window._gaq.push( - // https://developers.google.com/analytics/devguides/collection/analyticsjs/advanced#hitCallback - // This is called AFTER GA has sent all pending data: - - // hitCallback is undocumented in ga.js, but the assumption is that it - // will fire after the *next* track event, and not *all* pending track events. - - // If hitCallback continues to cause problems, we should be able to safely - // remove it and use only the setTimeout technique. - ['_set', 'hitCallback', gaCallback] - ); + // https://developers.google.com/analytics/devguides/collection/gajs/#PushingFunctions + // This is called after GA has sent the current pending data: + window._gaq.push(gaCallback); } - // send event to GA - window._gaq.push(eventArray); } else { // GA disabled or blocked or something, make sure we still // call the caller's callback: diff --git a/media/js/firefox/new.js b/media/js/firefox/new.js index bf9314c2e5..1ae6c7a67c 100644 --- a/media/js/firefox/new.js +++ b/media/js/firefox/new.js @@ -76,8 +76,9 @@ var $thankYou = $('.thankyou'); var hash_change = ('onhashchange' in window); - // Add external link tracking - $(document).on('click', 'a', function(e) { + // Add external link tracking, excluding links in Tabzilla that will be + // logged in tabzilla.js + $('#outer-wrapper').on('click', 'a', function(e) { // only track off-site links and don't track download.mozilla.org links if (this.hostname && this.hostname !== location.hostname && this.hostname !== 'download.mozilla.org') { var newTab = (this.target === '_blank' || e.metaKey || e.ctrlKey); diff --git a/media/js/firefox/os/firefox-os.js b/media/js/firefox/os/firefox-os.js index f98d4b9ce0..676f466cbc 100644 --- a/media/js/firefox/os/firefox-os.js +++ b/media/js/firefox/os/firefox-os.js @@ -232,10 +232,7 @@ if (hasCallback) { timer = setTimeout(gaCallback, 500); - window._gaq.push( - ['_set', 'hitCallback', gaCallback], - eventsArray - ); + window._gaq.push(eventsArray, gaCallback); } else { window._gaq.push(eventsArray); } diff --git a/media/js/test/spec/global.js b/media/js/test/spec/global.js index 381a2d8cd2..335994169b 100755 --- a/media/js/test/spec/global.js +++ b/media/js/test/spec/global.js @@ -239,6 +239,19 @@ describe("global.js", function() { expect(callback).toHaveBeenCalled(); }); + it("should not fire a callback twice", function () { + /* For our callback use a jasmine spy, then we can easily test + * to make sure it gets called once gaTrack has finished executing */ + var callback = jasmine.createSpy(); + gaTrack(['_trackEvent', 'GA event test', 'test', 'test'], callback); + clock.tick(600); // must be longer than callback timeout (500ms) in gaTrack + expect(callback.callCount).toEqual(1); + // The callback should not be executed by subsequent GA events + gaTrack(['_trackEvent', 'GA event test', 'test', 'test']); + clock.tick(600); // must be longer than callback timeout (500ms) in gaTrack + expect(callback.callCount).toEqual(1); + }); + it("should still fire a callback if window._gaq is undefined", function () { var callback = jasmine.createSpy(); window._gaq = undefined;