Fix unsaved changes modal not closing after browser back

core/src/App.svelte

@@ -395,7 +395,7 @@
         preservedViews = [];
         Iframe.removeInactiveIframes(node);
       }
-      for(let i = mfModalList.length; i--;){
+      for (let i = mfModalList.length; i--; ) {
         closeModal(i);
       }
 
@@ -945,7 +945,7 @@
     );
 
     //  only show the modal path in the URL when the first modal is opened.
-    if (showModalPathInUrl && mfModalList.length===1) {
+    if (showModalPathInUrl && mfModalList.length === 1) {
       const url = new URL(location.href);
       history.pushState(window.state, '', url.href);
       Routing.appendModalDataToUrl(nodepath, settings, url);
@@ -980,19 +980,21 @@
   const closeModal = (index, isClosedInternal) => {
     const resetModalData = (index, isClosedInternal) => {
       const showModalPathInUrl = LuigiConfig.getConfigBooleanValue(
-          'routing.showModalPathInUrl'
-        );
+        'routing.showModalPathInUrl'
+      );
       // only remove the modal path in URL when closing the first modal
-      if (showModalPathInUrl && mfModalList.length===1) {
+      if (showModalPathInUrl && mfModalList.length === 1) {
         Routing.removeModalDataFromUrl(isClosedInternal);
       }
       resetMicrofrontendModalData(index);
-    }
+    };
     const targetModal = mfModalList[index];
     if (targetModal && targetModal.modalIframe) {
-      getUnsavedChangesModalPromise(targetModal.modalIframe.contentWindow).then(() => {
-        resetModalData(index, isClosedInternal);
-      });
+      getUnsavedChangesModalPromise(targetModal.modalIframe.contentWindow).then(
+        () => {
+          resetModalData(index, isClosedInternal);
+        }
+      );
     } else if (targetModal && targetModal.modalWC) {
       resetModalData(index, isClosedInternal);
     }
@@ -1528,7 +1530,12 @@
       if ('luigi.navigation.back' === e.data.msg) {
         const mfModalTopMostElement = mfModalList[mfModalList.length - 1];
 
-        if (IframeHelpers.isMessageSource(e, mfModalTopMostElement && mfModalTopMostElement.modalIframe)) {
+        if (
+          IframeHelpers.isMessageSource(
+            e,
+            mfModalTopMostElement && mfModalTopMostElement.modalIframe
+          )
+        ) {
           closeModal(mfModalList.length - 1, true);
 
           await sendContextToClient(config, {
@@ -1907,8 +1914,8 @@
         nodepath={modalItem.mfModal.nodepath}
         modalIndex={index}
         on:close={() => closeModal(index, true)}
-        on:iframeCreated={event => modalIframeCreated(event, index)}
-        on:wcCreated={event => modalWCCreated(event, index)}
+        on:iframeCreated={(event) => modalIframeCreated(event, index)}
+        on:wcCreated={(event) => modalWCCreated(event, index)}
         {disableBackdrop}
       />
     {/if}

core/src/services/routing.js

@@ -163,8 +163,8 @@ class RoutingClass {
     return LuigiConfig.getConfigValue('routing.useHashRouting')
       ? window.location.hash.replace('#', '') // TODO: GenericHelpers.getPathWithoutHash(window.location.hash) fails in ContextSwitcher
       : window.location.search
-      ? GenericHelpers.trimLeadingSlash(window.location.pathname) + window.location.search
-      : GenericHelpers.trimLeadingSlash(window.location.pathname);
+        ? GenericHelpers.trimLeadingSlash(window.location.pathname) + window.location.search
+        : GenericHelpers.trimLeadingSlash(window.location.pathname);
   }
 
   /**
@@ -188,26 +188,48 @@ class RoutingClass {
   }
 
   /**
-   * Fires an 'Unsaved Changes' modal followed by a subsequent route change handling afterwards
+   * Prevents the browsers default route change by bringing back previous route then
+   * fires an 'Unsaved Changes' modal followed by a subsequent route change handling afterwards
+   *
    * @param {string} path the path of the view to open
    * @param {Object} component current component data
    * @param {Object} iframeElement the dom element of active iframe
    * @param {Object} config the configuration of application
    */
-  showUnsavedChangesModal(path, component, iframeElement, config) {
-    const newUrl = window.location.href;
-    const oldUrl = component.get().unsavedChanges.persistUrl;
-
-    //pretend the url hasn't been changed
-    oldUrl && history.replaceState(window.state, '', oldUrl);
-    component.showUnsavedChangesModal().then(
-      () => {
-        path &&
-          this.handleRouteChange(path, component, iframeElement, config) &&
-          history.replaceState(window.state, '', newUrl);
-      },
-      () => {}
-    );
+  handleUnsavedChangesModal(path, component, iframeElement, config) {
+    const newUrl = window.location.href,
+      oldUrl = component.get().unsavedChanges.persistUrl;
+
+    // pretend the url hasn't been changed by browser default behaviour
+    oldUrl && history.pushState(window.state, '', oldUrl);
+
+    return component
+      .getUnsavedChangesModalPromise()
+      .then(
+        // resolve unsaved changes promise
+        () => {
+          this.resolveUnsavedChanges(path, component, iframeElement, config, newUrl);
+        },
+        // user clicks no, do nothing, reject promise
+        () => {
+        }
+      )
+      .catch(() => { });
+  }
+
+  /**
+   * This function acts as a resolve callback in handleUnsavedChangesModal function
+   * Logic separated to enable better unit testing of the functionality
+   * @param {string} path the path to navigate to
+   * @param {Object} component the current component data
+   * @param {Object} iframeElement the dom element of active iframe
+   * @param {Object} config the configuration of application
+   */
+  resolveUnsavedChanges(path, component, iframeElement, config, newUrl) {
+    if (path) {
+      this.handleRouteChange(path, component, iframeElement, config);
+      history.replaceState(window.state, '', newUrl);
+    }
   }
 
   /**
@@ -335,7 +357,7 @@ class RoutingClass {
     try {
       // just used for browser changes, like browser url manual change or browser back/forward button click
       if (component.shouldShowUnsavedChangesModal()) {
-        this.showUnsavedChangesModal(path, component, iframeElement, config);
+        await this.handleUnsavedChangesModal(path, component, iframeElement, config);
         return;
       }
 
@@ -412,10 +434,10 @@ class RoutingClass {
         Object.assign({}, newNodeData, {
           previousNodeValues: previousCompData
             ? {
-                viewUrl: previousCompData.viewUrl,
-                isolateView: previousCompData.isolateView,
-                viewGroup: previousCompData.viewGroup
-              }
+              viewUrl: previousCompData.viewUrl,
+              isolateView: previousCompData.isolateView,
+              viewGroup: previousCompData.viewGroup
+            }
             : {}
         })
       );

core/test/services/routing.spec.js

@@ -10,7 +10,7 @@ import { Navigation } from '../../src/navigation/services/navigation';
 import { NodeDataManagementStorage } from '../../src/services/node-data-management';
 import { Iframe, ViewUrlDecorator } from '../../src/services';
 
-describe('Routing', function() {
+describe('Routing', function () {
   this.retries(1);
 
   let component;
@@ -684,7 +684,7 @@ describe('Routing', function() {
     it('should call console.warn when node has no children and there is no intention for empty viewUrl', async () => {
       //given
       const path = 'compound';
-      const node = { compound: { renderer: () => {} } };
+      const node = { compound: { renderer: () => { } } };
 
       //when
       console.warn = sinon.spy();
@@ -702,7 +702,7 @@ describe('Routing', function() {
     it('should navigate to rootPath if node can be reached directly', async () => {
       //given
       const path = 'compound2';
-      const node = { compound: { renderer: () => {} } };
+      const node = { compound: { renderer: () => { } } };
 
       //when
       component.viewUrl = path;
@@ -720,7 +720,7 @@ describe('Routing', function() {
     it('should handle nodeObject that is compound', async () => {
       //given
       const path = 'compound3';
-      const node = { compound: { renderer: () => {} } };
+      const node = { compound: { renderer: () => { } } };
 
       //when
       component.viewUrl = path;
@@ -746,7 +746,7 @@ describe('Routing', function() {
     it('should handle nodeObject that is webcomponent', async () => {
       //given
       const path = 'compound-webcomponent';
-      const node = { compound: { renderer: () => {} } };
+      const node = { compound: { renderer: () => { } } };
 
       //when
       component.viewUrl = path;
@@ -987,7 +987,7 @@ describe('Routing', function() {
 
   describe('showPageNotFoundError()', () => {
     let component = {
-      showAlert: () => {}
+      showAlert: () => { }
     };
     let pathToRedirect = '/go/here';
     let pathToRedirect2 = '/go/there';
@@ -1392,4 +1392,66 @@ describe('Routing', function() {
       sinon.assert.notCalled(Routing.handleBookmarkableModalPath);
     });
   });
+
+  describe('handleUnsavedChangesModal', () => {
+    let path, iframeElement, config, oldUrl, newUrl;
+
+    beforeEach(() => {
+      iframeElement = 'iframe';
+      config = 'config';
+      oldUrl = new URL('https://www.oldurl.com');
+      newUrl = new URL('https://www.newUrl.com');
+      window.state = {};
+      window.history.replaceState = sinon.spy();
+
+      component.get = () => {
+        return {
+          unsavedChanges: {
+            persistUrl: oldUrl
+          }
+        };
+      };
+      component.getUnsavedChangesModalPromise = () => { };
+      sinon.stub(component, 'getUnsavedChangesModalPromise').resolves();
+    });
+
+    afterEach(() => {
+      sinon.restore();
+    });
+
+    it('inner function resolved', async () => {
+      const path = 'valid';
+      sinon.stub(Routing, 'resolveUnsavedChanges');
+
+      await Routing.handleUnsavedChangesModal(path, component, iframeElement, config);
+
+      sinon.assert.calledWithExactly(window.history.pushState, window.state, '', oldUrl);
+      sinon.assert.calledOnce(component.getUnsavedChangesModalPromise);
+      sinon.assert.calledOnce(Routing.resolveUnsavedChanges);
+    });
+
+    describe('resolveUnsavedChanges', () => {
+      beforeEach(() => {
+        sinon.stub(Routing, 'handleRouteChange');
+      });
+
+      afterEach(() => {
+        sinon.restore();
+      });
+
+      it('test with valid path', async () => {
+        path = 'valid';
+        Routing.resolveUnsavedChanges(path, component, iframeElement, config);
+        sinon.assert.calledOnce(window.history.replaceState);
+        sinon.assert.calledOnce(Routing.handleRouteChange);
+      });
+
+      it('test with invalid path', async () => {
+        path = '';
+        Routing.resolveUnsavedChanges(path, component, iframeElement, config, newUrl);
+        sinon.assert.notCalled(window.history.replaceState);
+        sinon.assert.notCalled(Routing.handleRouteChange);
+      });
+    });
+  });
 });