Merge pull request #2118 from vikhyat/firefox-middle-click

Fix Firefox middle-click popup blocker issue
discourse · Mar 21, 2014 · 2dc5e3c · 2dc5e3c
2 parents 0462969 + e798705
commit 2dc5e3c
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 33 deletions.
diff --git 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
@@ -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() {