From 5f6571a6fec60fee9688eeda2c97c9bb15cc2e3b Mon Sep 17 00:00:00 2001
From: Julien Mougenot <jum@odoo.com>
Date: Tue, 28 Jan 2020 11:25:56 +0000
Subject: [PATCH] [FIX] web,mail: only apply modal activity for Dialog modals

Before this commit, the CSS rule handling the "activity" of the modals
(their z-index compared to the one of their backdrop) was global and would
interfere with front-end or custom modals which do not use the static Dialog
methods giving them the "active" class.

Now, the rule has been inverted: the z-index of a modal is only changed when
it is given an "inactive" class, thus only impacting the modals instantiated
via Dialog or Owl Dialog classes.

closes odoo/odoo#44067

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 addons/mail/static/src/xml/thread.xml         |  2 +-
 addons/web/static/src/js/core/owl_dialog.js   | 22 +++---
 addons/web/static/src/scss/modal.scss         |  4 +-
 .../web/static/tests/core/owl_dialog_tests.js | 71 +++++++++++++++++--
 4 files changed, 79 insertions(+), 20 deletions(-)

diff --git a/addons/mail/static/src/xml/thread.xml b/addons/mail/static/src/xml/thread.xml
index 9384f5a0141..b32b8afa1af 100644
--- a/addons/mail/static/src/xml/thread.xml
+++ b/addons/mail/static/src/xml/thread.xml
@@ -87,7 +87,7 @@
         @param {mail.DocumentViewer} widget
     -->
     <t t-name="DocumentViewer">
-        <div class="modal o_modal_fullscreen o_active_modal" tabindex="-1" data-keyboard="false" role="dialog">
+        <div class="modal o_modal_fullscreen" tabindex="-1" data-keyboard="false" role="dialog">
             <t class="o_document_viewer_content_call" t-call="DocumentViewer.Content"/>
 
             <t t-if="widget.attachment.length !== 1">
diff --git a/addons/web/static/src/js/core/owl_dialog.js b/addons/web/static/src/js/core/owl_dialog.js
index e6221e182c9..940c7c3b7d2 100644
--- a/addons/web/static/src/js/core/owl_dialog.js
+++ b/addons/web/static/src/js/core/owl_dialog.js
@@ -189,20 +189,18 @@ odoo.define('web.OwlDialog', function (require) {
          * @param {(LegacyDialog|OwlDialog)} dialog
          */
         static display(dialog) {
-            // Deactivate previous dialog
-            const activeDialogEl = document.querySelector('.modal.o_active_modal');
-            if (activeDialogEl) {
-                activeDialogEl.classList.remove('o_active_modal');
+            const activeDialog = this.displayed[this.displayed.length - 1];
+            if (activeDialog) {
+                // Deactivate previous dialog
+                const activeDialogEl = activeDialog instanceof this ?
+                    // Owl dialog
+                    activeDialog.modalRef.el :
+                    // Legacy dialog
+                    activeDialog.$modal[0];
+                activeDialogEl.classList.add('o_inactive_modal');
             }
             // Push dialog
             this.displayed.push(dialog);
-            // Add active class
-            const modalEl = dialog instanceof this ?
-                // Owl dialog
-                dialog.modalRef.el :
-                // Legacy dialog
-                dialog.$modal[0];
-            modalEl.classList.add('o_active_modal');
             // Update body class
             document.body.classList.add('modal-open');
         }
@@ -224,7 +222,7 @@ odoo.define('web.OwlDialog', function (require) {
                     lastDialog.modalRef.el :
                     // Legacy dialog
                     lastDialog.$modal[0];
-                modalEl.classList.add('o_active_modal');
+                modalEl.classList.remove('o_inactive_modal');
             } else {
                 document.body.classList.remove('modal-open');
             }
diff --git a/addons/web/static/src/scss/modal.scss b/addons/web/static/src/scss/modal.scss
index d82ddec6213..54bbbc2a696 100644
--- a/addons/web/static/src/scss/modal.scss
+++ b/addons/web/static/src/scss/modal.scss
@@ -106,8 +106,8 @@
     }
 }
 
-.modal:not(.o_active_modal) {
-    z-index: 1030;
+.modal.o_inactive_modal {
+    z-index: $zindex-modal-backdrop - 1;
 }
 
 .o_dialog {
diff --git a/addons/web/static/tests/core/owl_dialog_tests.js b/addons/web/static/tests/core/owl_dialog_tests.js
index f7a74368034..60f9e95bf43 100644
--- a/addons/web/static/tests/core/owl_dialog_tests.js
+++ b/addons/web/static/tests/core/owl_dialog_tests.js
@@ -200,7 +200,7 @@ odoo.define('web.owl_dialog_tests', function (require) {
             unopenedModal.close();
 
             let modals = document.querySelectorAll('.modal');
-            assert.ok(modals[modals.length - 1].classList.contains('o_active_modal'),
+            assert.notOk(modals[modals.length - 1].classList.contains('o_inactive_modal'),
                 "last dialog should have the active class");
             assert.notOk(modals[modals.length - 1].classList.contains('o_legacy_dialog'),
                 "active dialog should not have the legacy class");
@@ -211,7 +211,7 @@ odoo.define('web.owl_dialog_tests', function (require) {
             await testUtils.dom.triggerEvent(modals[modals.length - 1], 'keydown', EscapeKey); // Press Escape
 
             modals = document.querySelectorAll('.modal');
-            assert.ok(modals[modals.length - 1].classList.contains('o_active_modal'),
+            assert.notOk(modals[modals.length - 1].classList.contains('o_inactive_modal'),
                 "last dialog should have the active class");
             assert.notOk(modals[modals.length - 1].classList.contains('o_legacy_dialog'),
                 "active dialog should not have the legacy class");
@@ -221,7 +221,7 @@ odoo.define('web.owl_dialog_tests', function (require) {
             await testUtils.dom.click(modals[modals.length - 1].querySelector('.btn.btn-primary')); // Click on 'Ok' button
 
             modals = document.querySelectorAll('.modal');
-            assert.containsOnce(document.body, '.modal.o_legacy_dialog.o_active_modal',
+            assert.containsOnce(document.body, '.modal.o_legacy_dialog:not(.o_inactive_modal)',
                 "active dialog should have the legacy class");
             assert.containsOnce(document.body, '.o_dialog');
             assert.containsN(document.body, '.o_legacy_dialog', 2);
@@ -230,7 +230,7 @@ odoo.define('web.owl_dialog_tests', function (require) {
             await testUtils.dom.triggerEvent(modals[modals.length - 1], 'keydown', EscapeKey);
 
             modals = document.querySelectorAll('.modal');
-            assert.containsOnce(document.body, '.modal.o_legacy_dialog.o_active_modal',
+            assert.containsOnce(document.body, '.modal.o_legacy_dialog:not(.o_inactive_modal)',
                 "active dialog should have the legacy class");
             assert.containsOnce(document.body, '.o_dialog');
             assert.containsOnce(document.body, '.o_legacy_dialog');
@@ -238,7 +238,7 @@ odoo.define('web.owl_dialog_tests', function (require) {
             await testUtils.dom.click(modals[modals.length - 1].querySelector('.close'));
 
             modals = document.querySelectorAll('.modal');
-            assert.ok(modals[modals.length - 1].classList.contains('o_active_modal'),
+            assert.notOk(modals[modals.length - 1].classList.contains('o_inactive_modal'),
                 "last dialog should have the active class");
             assert.notOk(modals[modals.length - 1].classList.contains('o_legacy_dialog'),
                 "active dialog should not have the legacy class");
@@ -254,4 +254,65 @@ odoo.define('web.owl_dialog_tests', function (require) {
             parent.destroy();
         });
     });
+
+    QUnit.test("Z-index toggling and interactions", async function (assert) {
+        assert.expect(3);
+
+        function createCustomModal(className) {
+            const $modal = $(
+                `<div role="dialog" class="${className}" tabindex="-1">
+                    <div class="modal-dialog medium">
+                        <div class="modal-content">
+                            <main class="modal-body">The modal body</main>
+                        </div>
+                    </div>
+                </div>`
+            ).appendTo('body').modal();
+            const modal = $modal[0];
+            modal.destroy = function () {
+                $modal.modal('hide');
+                this.remove();
+            };
+            return modal;
+        }
+
+        class Parent extends Component {
+            constructor() {
+                super(...arguments);
+                this.state = useState({ showSecondDialog: true });
+            }
+        }
+        Parent.components = { Dialog };
+        Parent.env = makeTestEnvironment();
+        Parent.template = xml`
+            <div>
+                <Dialog/>
+                <Dialog t-if="state.showSecondDialog"/>
+            </div>`;
+
+        const parent = new Parent();
+        await parent.mount(testUtils.prepareTarget());
+
+        const frontEndModal = createCustomModal('modal');
+        const backEndModal = createCustomModal('modal o_technical_modal');
+
+        // querySelector will target the first modal (the static one).
+        const owlIndexBefore = getComputedStyle(document.querySelector('.o_dialog .modal')).zIndex;
+        const feZIndexBefore = getComputedStyle(frontEndModal).zIndex;
+        const beZIndexBefore = getComputedStyle(backEndModal).zIndex;
+
+        parent.state.showSecondDialog = false;
+        await testUtils.nextTick();
+
+        assert.ok(owlIndexBefore < getComputedStyle(document.querySelector('.o_dialog .modal')).zIndex,
+            "z-index of the owl dialog should be incremented since the active modal was destroyed");
+        assert.strictEqual(feZIndexBefore, getComputedStyle(frontEndModal).zIndex,
+            "z-index of front-end modals should not be impacted by Owl Dialog activity system");
+        assert.strictEqual(beZIndexBefore, getComputedStyle(backEndModal).zIndex,
+            "z-index of custom back-end modals should not be impacted by Owl Dialog activity system");
+
+        parent.destroy();
+        frontEndModal.destroy();
+        backEndModal.destroy();
+    });
 });