Merge branch '27922-cmd-click-todo-doesn-t-work' into 'master'

app/assets/javascripts/merge_request_tabs.js.es6

@@ -103,9 +103,10 @@ require('./flash');
     }
 
     clickTab(e) {
-      if (e.target && gl.utils.isMetaClick(e)) {
-        const targetLink = e.target.getAttribute('href');
+      if (e.currentTarget && gl.utils.isMetaClick(e)) {
+        const targetLink = e.currentTarget.getAttribute('href');
         e.stopImmediatePropagation();
+        e.preventDefault();
         window.open(targetLink, '_blank');
       }
     }

app/assets/javascripts/todos.js.es6

@@ -147,24 +147,21 @@
 
     goToTodoUrl(e) {
       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;
       }
 
       if (gl.utils.isMetaClick(e)) {
+        const windowTarget = '_blank';
+        const selected = e.target;
         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');
+
+        if (selected.tagName === 'IMG') {
+          const avatarUrl = selected.parentElement.getAttribute('href');
+          return window.open(avatarUrl, windowTarget);
         } else {
-          return window.open(todoLink, '_blank');
+          return window.open(todoLink, windowTarget);
         }
       } else {
         return gl.utils.visitUrl(todoLink);

changelogs/unreleased/27922-cmd-click-todo-doesn-t-work.yml

+---
+title: Fix regression where cmd-click stopped working for todos and merge request
+  tabs
+merge_request:
+author:

spec/javascripts/merge_request_tabs_spec.js

@@ -62,19 +62,47 @@ require('vendor/jquery.scrollTo');
       });
     });
     describe('#opensInNewTab', function () {
-      var commitsLink;
       var tabUrl;
+      var windowTarget = '_blank';
 
       beforeEach(function () {
-        commitsLink = '.commits-tab li a';
-        tabUrl = $(commitsLink).attr('href');
+        loadFixtures('merge_requests/merge_request_with_task_list.html.raw');
+
+        tabUrl = $('.commits-tab a').attr('href');
 
         spyOn($.fn, 'attr').and.returnValue(tabUrl);
       });
+
+      describe('meta click', () => {
+        beforeEach(function () {
+          spyOn(gl.utils, 'isMetaClick').and.returnValue(true);
+        });
+
+        it('opens page when commits link is clicked', function () {
+          spyOn(window, 'open').and.callFake(function (url, name) {
+            expect(url).toEqual(tabUrl);
+            expect(name).toEqual(windowTarget);
+          });
+
+          this.class.bindEvents();
+          document.querySelector('.merge-request-tabs .commits-tab a').click();
+        });
+
+        it('opens page when commits badge is clicked', function () {
+          spyOn(window, 'open').and.callFake(function (url, name) {
+            expect(url).toEqual(tabUrl);
+            expect(name).toEqual(windowTarget);
+          });
+
+          this.class.bindEvents();
+          document.querySelector('.merge-request-tabs .commits-tab a .badge').click();
+        });
+      });
+
       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');
+          expect(name).toEqual(windowTarget);
         });
 
         this.class.clickTab({
@@ -87,7 +115,7 @@ require('vendor/jquery.scrollTo');
       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');
+          expect(name).toEqual(windowTarget);
         });
 
         this.class.clickTab({
@@ -100,7 +128,7 @@ require('vendor/jquery.scrollTo');
       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');
+          expect(name).toEqual(windowTarget);
         });
 
         this.class.clickTab({

spec/javascripts/todos_spec.js

+require('~/todos');
+require('~/lib/utils/common_utils');
+
+describe('Todos', () => {
+  preloadFixtures('todos/todos.html.raw');
+  let todoItem;
+
+  beforeEach(() => {
+    loadFixtures('todos/todos.html.raw');
+    todoItem = document.querySelector('.todos-list .todo');
+
+    return new gl.Todos();
+  });
+
+  describe('goToTodoUrl', () => {
+    it('opens the todo url', (done) => {
+      const todoLink = todoItem.dataset.url;
+
+      spyOn(gl.utils, 'visitUrl').and.callFake((url) => {
+        expect(url).toEqual(todoLink);
+        done();
+      });
+
+      todoItem.click();
+    });
+
+    describe('meta click', () => {
+      let visitUrlSpy;
+
+      beforeEach(() => {
+        spyOn(gl.utils, 'isMetaClick').and.returnValue(true);
+        visitUrlSpy = spyOn(gl.utils, 'visitUrl').and.callFake(() => {});
+      });
+
+      it('opens the todo url in another tab', (done) => {
+        const todoLink = todoItem.dataset.url;
+
+        spyOn(window, 'open').and.callFake((url, target) => {
+          expect(todoLink).toEqual(url);
+          expect(target).toEqual('_blank');
+          done();
+        });
+
+        todoItem.click();
+        expect(visitUrlSpy).not.toHaveBeenCalled();
+      });
+
+      it('opens the avatar\'s url in another tab when the avatar is clicked', (done) => {
+        const avatarImage = todoItem.querySelector('img');
+        const avatarUrl = avatarImage.parentElement.getAttribute('href');
+
+        spyOn(window, 'open').and.callFake((url, target) => {
+          expect(avatarUrl).toEqual(url);
+          expect(target).toEqual('_blank');
+          done();
+        });
+
+        avatarImage.click();
+        expect(visitUrlSpy).not.toHaveBeenCalled();
+      });
+    });
+  });
+});