[Molkobain]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / A GitHub Issue / Combodo ticket?
Type of change?	Bug fix

Symptom (bug) / Objective (enhancement)

In the user portal, chaining several modals opening can lead to one of them being displayed behind the previous one as you can see below:

Video

Expected behavior is that every new modal is displayed on top of the previous one:

Video

Reproduction procedure (bug)

1. On iTop 3.2.3
2. With PHP 8.4.18
3. Installed with the standard portal, the FAQs module and the XML delta below
4. Log in the user portal as an administrator
5. Go to the "FAQ" brick and click on a FAQ: it is displayed in view mode in a modal
6. Click on the "Modify..." button: the FAQ is displayed in edit mode in a new modal (the view modal is closed)
7. Click on the "Click me only in the edit FAQ form" button: the "Hello there!" modal is displayed behind the edit modal, whereas it is expected to be displayed above it

<?xml version="1.0" encoding="UTF-8"?>
<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="3.2">
  <snippets>
    <snippet id="PortalModalsSuperpositionReproducer" _delta="define">
      <placement>core</placement>
      <rank>0</rank>
      <content><![CDATA[
/**
 * Adds a "Click me only in the edit FAQ form" button to the FAQ forms of the portal, which opens a modal
 */
class PortalModalsSuperpositionExample implements iPopupMenuExtension
{
	public static function EnumItems($iMenuId, $param)
	{
		$aItems = [];

		if (($iMenuId === iPopupMenuExtension::PORTAL_OBJDETAILS_ACTIONS) && ($param['object'] instanceof FAQ)) {
			$aItems[] = new JSButtonItem(
				'portal-modals-superposition-reproducer',
				'Click me only in the edit FAQ form',
				"CombodoModal.OpenModal({content: '<div class=\"modal-body\">Hello there!</div>'});"
			);
		}

		return $aItems;
	}
}
]]></content>
    </snippet>
  </snippets>
  <module_designs>
    <module_design id="itop-portal" xsi:type="portal">
      <classes>
        <class id="FAQ">
          <scopes>
            <scope id="administrators" _delta="define">
              <oql_view><![CDATA[SELECT FAQ]]></oql_view>
              <oql_edit><![CDATA[SELECT FAQ]]></oql_edit>
              <allowed_profiles>
                <allowed_profile id="Administrator"/>
              </allowed_profiles>
            </scope>
          </scopes>
        </class>
      </classes>
    </module_design>
  </module_designs>
</itop_design>

Cause (bug)

The z-index of a portal modal is computed when it is shown, in the show.bs.modal handler of datamodels/2.x/itop-portal-base/portal/public/js/bootstrap-portal-modal.js, from the number of visible modals:

let zIndex = 1050 + (10 * $('.modal:visible').length);

The "Modify..." button of the view modal calls CombodoModal.OpenUrlInModal(sModifyUrl, true), which closes the view modal (CombodoModal.CloseAllModals()) and immediately opens the edit modal, cloned from #modal-for-all. As the view modal is still visible during its closing animation, and the edit modal is cloned from it while it is displayed, 2 modals are counted as visible when the edit modal is shown. It gets a z-index of 1070 instead of 1060.

Once the view modal is closed, the edit modal is alone but keeps its z-index of 1070, so the next modal opened from it gets 1050 + 10 × 1 = 1060 and is displayed behind it:

Modal	z-index
View modal	1050
Edit modal (opened by "Modify...")	1070
Modal opened from the edit modal	1060 (behind)

More generally, counting the visible modals gives a wrong z-index as soon as modals are not closed in the reverse order of their opening (eg. open A, open B, close A, open C: C gets the same z-index as B).

Mind that the issue is still present in both 3.3 and 3.4 branches on the new portal; fix is the same.

Proposed solution (bug and enhancement)

The modal being shown is now put just above the highest visible modal, instead of computing its z-index from the number of visible modals. The modal being shown is excluded from the computation, as it can already be visible (eg. when cloned from a displayed modal):

let zIndex = 1050;
$('.modal:visible').not(this).each(function () {
	zIndex = Math.max(zIndex, (parseInt($(this).css('z-index'), 10) || 1050) + 10);
});

The first modal still gets a z-index of 1050, and the backdrop handling is unchanged (z-index of the modal - 1).

Results with the reproduction procedure above:

Modal	Before	After
View modal	1050	1050
Edit modal (opened by "Modify...")	1070	1060
Modal opened from the edit modal	1060 (behind)	1070 (above)

The "open A, open B, close A, open C" case is fixed as well (A: 1050, B: 1060, C: 1070).

No unit test was added as the fix is in the JS part of the portal, which isn't covered by any mean yet.

Checklist before requesting a review

• If my PR is a bug fix or an enhancement, I opened an issue that got reviewed and approved by Combodo
• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand without digging in the code?

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Changes how modal stacking order is calculated.

The PR appears safe to merge; no actionable regression was established.

Summary

The PR changes portal modal stacking from a visible-modal count to a z-index above the highest other visible modal. This addresses cases where modals close out of opening order or a displayed modal is cloned while another closes.

_{Reviews (1) · Last reviewed commit: "Fix portal modals superposition issue"}

[steffunky]

Is there any difference with #957 ?

[Molkobain]

Is there any difference with #957 ?

Indeed, seems like the same issue, good catch!

[Hipska]

Except this one has a very clear and detailed description with the form filled in as expected so anyone can have a look and give feedback and understands what it is about. The other one is an internal pull request leaked to the outside with zero context. Lead by example. Well done @Molkobain!