[modal] Fix aria-hidden should not apply on non-portal modals

[ZeeshanTamboli]

Fixes #19450

Closes #43318 (Old PR)

Closes #48889 (Another PR attempt but does not handle this case)

Do not apply aria-hidden on non-portal modals (i.e when disablePortal) ancestor. This hides the modals from the accessibility tree. Also, mark the siblings of modal as aria-hidden so that they are inaccessible when the modal is opened. Handles cases with nested modals as well.

---

Test the reproductions on Mozilla Firefox because Blink (rendering engine used by Google Chrome etc) blocks the aria-hidden on elements whose descendents have focus by default. So, it's reproducible in Firefox but a general issue on all browsers. Use a screen-reader and test it out. Check modal sibling elements in DOM as well where it should have aria-hidden attribute.

Before: https://stackblitz.com/edit/github-fennfewl?file=src%2FApp.tsx
After fix: https://stackblitz.com/edit/github-fennfewl-pzeg3cck

[code-infra-dashboard]

Deploy preview

https://deploy-preview-49026--material-ui.netlify.app/

Bundle size

Bundle	Parsed size	Gzip size
@mui/material	🔺+409B(+0.08%)	🔺+153B(+0.10%)
@mui/lab	0B(0.00%)	0B(0.00%)
@mui/private-theming	0B(0.00%)	0B(0.00%)
@mui/system	0B(0.00%)	0B(0.00%)
@mui/utils	0B(0.00%)	0B(0.00%)

Details of bundle changes

---

Check out the code infra dashboard for more information about this PR.

[silviuaavram]

Codex found these issues. Could you look further into them? The first one seems relevant to me, when you use it like this: <Modal open disablePortal container={() => unrelatedContainerRef.current}>

[P1] disablePortal still uses an unrelated container for accessibility masking
getKeepChain() returns an empty chain when the supplied container does not contain the modal. This is valid with disablePortal, because Portal ignores container, while useModal still registers against it.
collectHiddenTargets() then hides the unrelated container’s children while leaving the actual inline modal’s siblings accessible. The accessibility container should come from the modal’s real DOM ancestry, independently of the scroll-lock container.

[P1] Removing a lower nested modal can hide the surviving top modal
After syncAriaHidden() preserves the remaining modal’s ancestor chain, remove() immediately reapplies aria-hidden to the removed modal.
If that removed inline modal is an ancestor of the remaining top modal, the top modal becomes inaccessible. I reproduced this with lowerRef.appendChild(upperRef), adding both, then removing lower: upperRef became inaccessible. The final write should be skipped when the removed ref contains the remaining top modal.

[P2] The ancestor walk fails across Shadow DOM boundaries
parentElement becomes null at a ShadowRoot. For an inline modal inside Shadow DOM, the keep chain is discarded and the shadow host receives aria-hidden="true", hiding the modal it contains.
The walk should follow ShadowRoot.host, or select an accessibility container inside the shadow tree.

[P2] This narrows a public API from Element to HTMLElement
Modal.mount and modalRef were changed from Element to HTMLElement. ModalManager is publicly re-exported from both @mui/material and @mui/material/Modal, so this is a source-breaking TypeScript change for Element-typed or SVG refs. The new helpers appear able to remain typed as Element.

[ZeeshanTamboli]

@silviuaavram

1. I think this issue would be on the developer's side that they are passing an unrelated custom container to the modal. In this case, I believe it should be fixed from their side where the custom container should be from the modal's ancestory DOM and not unrelated.

2. I think this would not occur in a practical scenario. Are you able to imagine a scenario where a lower modal is removed first when there are multiple modals opened on top of each other? I am not. I suppose that would be a bad UX.

3. This is pre-existing on master. I can do it in a separate PR if a use case occurs with modals inside shadow DOM.

4. Updated to use HTMLElement | SVGElement instead of only HTMLElement. Better than having Element, the narrower the types the better. And it is more specific sub-type. I don't think now after changing it would be a breaking change. The elements should always be a HTML DOM element.