From e798705aecef1941889e51de78450ea420ec611e Mon Sep 17 00:00:00 2001 From: Vikhyat Korrapati Date: Thu, 13 Mar 2014 11:03:19 +0530 Subject: [PATCH] Do not call preventDefault on right and middle-click/Ctrl+click. This should fix the middle click popup blocker issue on Firefox. --- .../javascripts/discourse/lib/click_track.js | 33 +++++++++-------- test/javascripts/lib/click_track_test.js | 36 ++++++++++--------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/click_track.js b/app/assets/javascripts/discourse/lib/click_track.js index f234924a9e4..afe2456776c 100644 --- a/app/assets/javascripts/discourse/lib/click_track.js +++ b/app/assets/javascripts/discourse/lib/click_track.js @@ -17,21 +17,7 @@ Discourse.ClickTrack = { var $link = $(e.currentTarget); if ($link.hasClass('lightbox')) return true; - e.preventDefault(); - - // We don't track clicks on quote back buttons - if ($link.hasClass('back') || $link.hasClass('quote-other-topic')) return true; - - // Remove the href, put it as a data attribute - if (!$link.data('href')) { - $link.addClass('no-href'); - $link.data('href', $link.attr('href')); - $link.attr('href', null); - // Don't route to this URL - $link.data('auto-route', true); - } - - var href = $link.data('href'), + var href = $link.attr('href') || $link.data('href'), $article = $link.closest('article'), postId = $article.data('post-id'), topicId = $('#topic').data('topic-id'), @@ -83,8 +69,21 @@ Discourse.ClickTrack = { }, dataType: 'html' }); - window.open(href, '_blank'); - return false; + return true; + } + + e.preventDefault(); + + // We don't track clicks on quote back buttons + if ($link.hasClass('back') || $link.hasClass('quote-other-topic')) return true; + + // Remove the href, put it as a data attribute + if (!$link.data('href')) { + $link.addClass('no-href'); + $link.data('href', $link.attr('href')); + $link.attr('href', null); + // Don't route to this URL + $link.data('auto-route', true); } // If we're on the same site, use the router and track via AJAX diff --git a/test/javascripts/lib/click_track_test.js b/test/javascripts/lib/click_track_test.js index 60e5b87fb92..25be417f200 100644 --- a/test/javascripts/lib/click_track_test.js +++ b/test/javascripts/lib/click_track_test.js @@ -114,35 +114,39 @@ test("right clicks are tracked", function() { equal(fixture('a').first().attr('href'), "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42"); }); +test("preventDefault is not called for right clicks", function() { + var clickEvent = generateClickEventOn('a'); + clickEvent.which = 3; + this.stub(clickEvent, "preventDefault"); + ok(track(clickEvent)); + ok(!clickEvent.preventDefault.calledOnce); +}); -var expectToOpenInANewTab = function(clickEvent) { - ok(!track(clickEvent)); - ok(Discourse.ajax.calledOnce); - ok(window.open.calledOnce); +var testOpenInANewTab = function(description, clickEventModifier) { + test(description, function() { + var clickEvent = generateClickEventOn('a'); + clickEventModifier(clickEvent); + this.stub(clickEvent, "preventDefault"); + ok(track(clickEvent)); + ok(Discourse.ajax.calledOnce); + ok(!clickEvent.preventDefault.calledOnce); + }); }; -test("it opens in a new tab when pressing shift", function() { - var clickEvent = generateClickEventOn('a'); +testOpenInANewTab("it opens in a new tab when pressing shift", function(clickEvent) { clickEvent.shiftKey = true; - expectToOpenInANewTab(clickEvent); }); -test("it opens in a new tab when pressing meta", function() { - var clickEvent = generateClickEventOn('a'); +testOpenInANewTab("it opens in a new tab when pressing meta", function(clickEvent) { clickEvent.metaKey = true; - expectToOpenInANewTab(clickEvent); }); -test("it opens in a new tab when pressing meta", function() { - var clickEvent = generateClickEventOn('a'); +testOpenInANewTab("it opens in a new tab when pressing ctrl", function(clickEvent) { clickEvent.ctrlKey = true; - expectToOpenInANewTab(clickEvent); }); -test("it opens in a new tab when pressing meta", function() { - var clickEvent = generateClickEventOn('a'); +testOpenInANewTab("it opens in a new tab on middle click", function(clickEvent) { clickEvent.which = 2; - expectToOpenInANewTab(clickEvent); }); test("tracks via AJAX if we're on the same site", function() {