Material-ui: Still having trouble with Modals and removeAttribute in 3.3.1

[grapenut]

See issue #13349 and others referenced.

I have a menu with several items. Clicking on a menu item is supposed to open another modal but it crashes the whole React DOM with the error Uncaught TypeError: Cannot read property 'removeAttribute' of undefined. I have already upgraded to 3.3.1 which was supposed to fix the issue, but it is still happening. Downgrading to 3.2.2 solves the problem and everything works as expected.

[Bosn]

still +1, in version @3.1.1 When right/top menu with items trigger OnClick, page crashed along with error message. Uncaught TypeError: Cannot read property 'removeAttribute' of undefined. 😢

Thx for the timely hotfix @3.3.1.

[oliviertassinari]

How can we reproduce it?

[stefensuhat]

it happen on mine too

[sammy-tam]

confirmed 3.1.1 still have the same behavior. the trace is same as #13349

[nasushkov]

I have the same issue with 3.3.1. Unfortunately, I cannot downgrade to 3.2.2 as suggested above

[jacobweber]

For me it's happening when I launch a Dialog from a Menu > MenuItem component's onClick function. Haven't been able to narrow it down further yet.

Here's the complete stack trace:

Uncaught TypeError: Cannot read property 'removeAttribute' of undefined
    at ariaHidden (manageAriaHidden.js:28)
    at ModalManager.remove (ModalManager.js:188)
    at Portal.Modal._this.handleRendered (Modal.js:133)
    at callCallback (react-dom.development.js:11582)
    at commitUpdateEffects (react-dom.development.js:11622)
    at commitUpdateQueue (react-dom.development.js:11610)
    at commitLifeCycles (react-dom.development.js:15374)
    at commitAllLifeCycles (react-dom.development.js:16705)
    at HTMLUnknownElement.callCallback (react-dom.development.js:146)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:195)
    at invokeGuardedCallback (react-dom.development.js:245)
    at commitRoot (react-dom.development.js:16881)
    at completeRoot (react-dom.development.js:18337)
    at performWorkOnRoot (react-dom.development.js:18259)
    at performWork (react-dom.development.js:18161)
    at performSyncWork (react-dom.development.js:18134)
    at interactiveUpdates$1 (react-dom.development.js:18444)
    at interactiveUpdates (react-dom.development.js:2322)
    at dispatchInteractiveEvent (react-dom.development.js:5132)
The above error occurred in the <Portal> component:
    in Portal (created by Modal)
    in Modal (created by WithStyles(Modal))
    in WithStyles(Modal) (created by Popover)
    in Popover (created by WithStyles(Popover))
    in WithStyles(Popover) (created by Menu)
    in Menu (created by WithStyles(Menu))
...

[rodrigonehring]

I had a similar problem with dialog, using with a fragment children, fixed after replacing for a div

[idrm]

Here's a reproducible demo: https://codesandbox.io/s/o730293xp9

[idrm]

From my somewhat cursory examination of the code it seems MUI tries to access a React ref that may not be ready. It's this piece of code in ModalManager.js:

ariaHidden(data.modals[data.modals.length - 1].modalRef, false);

It happens when one modal goes away and a new one appears on the same cycle.

Here's a new repro that uses two <Menu /> elements, with the second appearing after an item in the first menu is clicked (the and first menu disappearing at the same time): https://codesandbox.io/s/j3ppz0lwq5

I think a check before the above line of code in ModalManager.js would be required. E.g.

if (data.modals[data.modals.length - 1].modalRef)
   ariaHidden(...);

[oliviertassinari]

@idrm Thank you for the reproduction example! I think that we can solve the issue with a simple change. What do you think of:

--- a/packages/material-ui/src/Modal/Modal.js
+++ b/packages/material-ui/src/Modal/Modal.js
@@ -10,6 +10,7 @@ import Portal from '../Portal';
 import { createChainedFunction } from '../utils/helpers';
 import withStyles from '../styles/withStyles';
 import { createChainedFunction } from '../utils/helpers';
 import withStyles from '../styles/withStyles';
 import ModalManager from './ModalManager';
+import { ariaHidden } from './manageAriaHidden';
 import Backdrop from '../Backdrop';

 function getContainer(container, defaultContainer) {
@@ -118,10 +119,7 @@ class Modal extends React.Component {
     if (this.props.open) {
       this.handleOpened();
     } else {
-      const doc = ownerDocument(this.mountNode);
-      const container = getContainer(this.props.container, doc.body);
-      this.props.manager.add(this, container);
-      this.props.manager.remove(this);
+      ariaHidden(this.modalRef, true);
     }
   };

Adding & removing the modal wasn't a good call. We can directly execute the expected outcome.
Do you want to submit a pull request?

[molekilla]

Updated to Node 10 recently and getting the same bug.

[oneclickwebstudio]

Still having the same issue on 3.3.1

function ariaHidden(node, show) {
    if (show) {
        node.setAttribute('aria-hidden', 'true');
    } else {
        node.removeAttribute('aria-hidden'); // Uncaught TypeError: Cannot read property 'removeAttribute' of null
    }
}

[emmagoldblum]

Where are you today on this issue ? We are waiting for a fix really badly :)

[oliviertassinari]

@TomiCake should have fixed the issue in #13389. We will release v3.3.2 later this evening. If you are still experiencing issues once it's released, let us know. At least, we have two new tests!