Merge branch 'fix_back_button_when_switching_mr_tabs' into 'master'

Fix back button when switching mr tabs

Closes #16019

See merge request gitlab-org/gitlab!29862

app/assets/javascripts/merge_request_tabs.js

@@ -126,6 +126,13 @@ export default class MergeRequestTabs {
 
   bindEvents() {
     $('.merge-request-tabs a[data-toggle="tabvue"]').on('click', this.clickTab);
+    window.addEventListener('popstate', event => {
+      if (event.state && event.state.action) {
+        this.tabShown(event.state.action, event.target.location);
+        this.currentAction = event.state.action;
+        this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
+      }
+    });
   }
 
   // Used in tests
@@ -155,6 +162,12 @@ export default class MergeRequestTabs {
       } else if (action) {
         const href = e.currentTarget.getAttribute('href');
         this.tabShown(action, href);
+
+        if (this.setUrl) {
+          this.setCurrentAction(action);
+        }
+
+        this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
       }
     }
   }
@@ -213,11 +226,6 @@ export default class MergeRequestTabs {
         this.resetViewContainer();
         this.destroyPipelinesView();
       }
-      if (this.setUrl) {
-        this.setCurrentAction(action);
-      }
-
-      this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
     } else if (action === this.currentAction) {
       // ContentTop is used to handle anything at the top of the page before the main content
       const mainContentContainer = document.querySelector('.content-wrapper');
@@ -287,19 +295,25 @@ export default class MergeRequestTabs {
     // Ensure parameters and hash come along for the ride
     newState += location.search + location.hash;
 
-    // TODO: Consider refactoring in light of turbolinks removal.
-
-    // Replace the current history state with the new one without breaking
-    // Turbolinks' history.
-    //
-    // See https://github.com/rails/turbolinks/issues/363
-    window.history.replaceState(
-      {
-        url: newState,
-      },
-      document.title,
-      newState,
-    );
+    if (window.history.state && window.history.state.url && window.location.pathname !== newState) {
+      window.history.pushState(
+        {
+          url: newState,
+          action: this.currentAction,
+        },
+        document.title,
+        newState,
+      );
+    } else {
+      window.history.replaceState(
+        {
+          url: window.location.href,
+          action,
+        },
+        document.title,
+        window.location.href,
+      );
+    }
 
     return newState;
   }

changelogs/unreleased/fix_back_button_when_switching_mr_tabs.yml

+---
+title: Fix back button when switching MR tabs
+merge_request: 29862
+author: Lee Tickett
+type: fixed

spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'User clicks on merge request tabs', :js do
+  let(:project) { create(:project, :public, :repository) }
+  let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
+  it 'adds entry to page history' do
+    visit('/')
+    visit(merge_request_path(merge_request))
+    click_link('Changes')
+
+    expect(current_url).to match(/diffs$/)
+
+    page.driver.go_back
+
+    expect(current_url).to match(merge_request_path(merge_request))
+
+    page.driver.go_back
+
+    expect(current_url).to match('/')
+  end
+end

spec/javascripts/merge_request_tabs_spec.js

@@ -30,7 +30,7 @@ describe('MergeRequestTabs', function() {
     setLocation();
 
     this.spies = {
-      history: spyOn(window.history, 'replaceState').and.callFake(function() {}),
+      history: spyOn(window.history, 'pushState').and.callFake(function() {}),
     };
   });
 
@@ -142,6 +142,7 @@ describe('MergeRequestTabs', function() {
 
     afterEach(() => {
       mock.restore();
+      window.history.replaceState({}, '', '/');
     });
 
     it('changes from commits', function() {
@@ -194,11 +195,21 @@ describe('MergeRequestTabs', function() {
       setLocation({
         pathname: '/foo/bar/-/merge_requests/1',
       });
+      window.history.replaceState(
+        {
+          url: window.location.href,
+          action: 'show',
+        },
+        document.title,
+        window.location.href,
+      );
+
       const newState = this.subject('commits');
 
       expect(this.spies.history).toHaveBeenCalledWith(
         {
           url: newState,
+          action: 'commits',
         },
         document.title,
         newState,