Merge branch '24716-fix-ctrl-click-links' into 'master'

Fix Ctrl+Click support for Todos and Merge Request page tabs

Closes #24716

See merge request !8898

app/assets/javascripts/lib/utils/common_utils.js.es6

@@ -137,6 +137,14 @@
       return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
     };
 
+    gl.utils.isMetaClick = function(e) {
+      // Identify following special clicks
+      // 1) Cmd + Click on Mac (e.metaKey)
+      // 2) Ctrl + Click on PC (e.ctrlKey)
+      // 3) Middle-click or Mouse Wheel Click (e.which is 2)
+      return e.metaKey || e.ctrlKey || e.which === 2;
+    };
+
     gl.utils.scrollToElement = function($el) {
       var top = $el.offset().top;
       gl.navBarHeight = gl.navBarHeight || $('.navbar-gitlab').height();

app/assets/javascripts/merge_request_tabs.js.es6

@@ -82,12 +82,18 @@ require('./flash');
       $(document)
         .on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
         .on('click', '.js-show-tab', this.showTab);
+
+      $('.merge-request-tabs a[data-toggle="tab"]')
+        .on('click', this.clickTab);
     }
 
     unbindEvents() {
       $(document)
         .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
         .off('click', '.js-show-tab', this.showTab);
+
+      $('.merge-request-tabs a[data-toggle="tab"]')
+        .off('click', this.clickTab);
     }
 
     showTab(e) {
@@ -95,6 +101,14 @@ require('./flash');
       this.activateTab($(e.target).data('action'));
     }
 
+    clickTab(e) {
+      if (e.target && gl.utils.isMetaClick(e)) {
+        const targetLink = e.target.getAttribute('href');
+        e.stopImmediatePropagation();
+        window.open(targetLink, '_blank');
+      }
+    }
+
     tabShown(e) {
       const $target = $(e.target);
       const action = $target.data('action');

app/assets/javascripts/todos.js.es6

@@ -146,14 +146,26 @@
     }
 
     goToTodoUrl(e) {
-      const todoLink = $(this).data('url');
+      const todoLink = this.dataset.url;
+      let targetLink = e.target.getAttribute('href');
+
+      if (e.target.tagName === 'IMG') { // See if clicked target was Avatar
+        targetLink = e.target.parentElement.getAttribute('href'); // Parent of Avatar is link
+      }
+
       if (!todoLink) {
         return;
       }
-      // Allow Meta-Click or Mouse3-click to open in a new tab
-      if (e.metaKey || e.which === 2) {
+
+      if (gl.utils.isMetaClick(e)) {
         e.preventDefault();
+        // Meta-Click on username leads to different URL than todoLink.
+        // Turbolinks can resolve that URL, but window.open requires URL manually.
+        if (targetLink !== todoLink) {
+          return window.open(targetLink, '_blank');
+        } else {
           return window.open(todoLink, '_blank');
+        }
       } else {
         return gl.utils.visitUrl(todoLink);
       }

changelogs/unreleased/24716-fix-ctrl-click-links.yml

+---
+title: Fix Ctrl+Click support for Todos and Merge Request page tabs
+merge_request: 8898
+author:

spec/javascripts/lib/utils/common_utils_spec.js.es6

@@ -86,5 +86,37 @@ require('~/lib/utils/common_utils');
         expect(normalized[NGINX].nginx).toBe('ok');
       });
     });
+
+    describe('gl.utils.isMetaClick', () => {
+      it('should identify meta click on Windows/Linux', () => {
+        const e = {
+          metaKey: false,
+          ctrlKey: true,
+          which: 1,
+        };
+
+        expect(gl.utils.isMetaClick(e)).toBe(true);
+      });
+
+      it('should identify meta click on macOS', () => {
+        const e = {
+          metaKey: true,
+          ctrlKey: false,
+          which: 1,
+        };
+
+        expect(gl.utils.isMetaClick(e)).toBe(true);
+      });
+
+      it('should identify as meta click on middle-click or Mouse-wheel click', () => {
+        const e = {
+          metaKey: false,
+          ctrlKey: false,
+          which: 2,
+        };
+
+        expect(gl.utils.isMetaClick(e)).toBe(true);
+      });
+    });
   });
 })();

spec/javascripts/merge_request_tabs_spec.js

@@ -61,6 +61,56 @@ require('vendor/jquery.scrollTo');
         expect($('#diffs')).toHaveClass('active');
       });
     });
+    describe('#opensInNewTab', function () {
+      var commitsLink;
+      var tabUrl;
+
+      beforeEach(function () {
+        commitsLink = '.commits-tab li a';
+        tabUrl = $(commitsLink).attr('href');
+
+        spyOn($.fn, 'attr').and.returnValue(tabUrl);
+      });
+      it('opens page tab in a new browser tab with Ctrl+Click - Windows/Linux', function () {
+        spyOn(window, 'open').and.callFake(function (url, name) {
+          expect(url).toEqual(tabUrl);
+          expect(name).toEqual('_blank');
+        });
+
+        this.class.clickTab({
+          metaKey: false,
+          ctrlKey: true,
+          which: 1,
+          stopImmediatePropagation: function () {}
+        });
+      });
+      it('opens page tab in a new browser tab with Cmd+Click - Mac', function () {
+        spyOn(window, 'open').and.callFake(function (url, name) {
+          expect(url).toEqual(tabUrl);
+          expect(name).toEqual('_blank');
+        });
+
+        this.class.clickTab({
+          metaKey: true,
+          ctrlKey: false,
+          which: 1,
+          stopImmediatePropagation: function () {}
+        });
+      });
+      it('opens page tab in a new browser tab with Middle-click - Mac/PC', function () {
+        spyOn(window, 'open').and.callFake(function (url, name) {
+          expect(url).toEqual(tabUrl);
+          expect(name).toEqual('_blank');
+        });
+
+        this.class.clickTab({
+          metaKey: false,
+          ctrlKey: false,
+          which: 2,
+          stopImmediatePropagation: function () {}
+        });
+      });
+    });
 
     describe('#setCurrentAction', function () {
       beforeEach(function () {