material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.11k stars 2.15k forks source link

Maximum call stack size exceeded on focusTrap.tryFocus #4298

Open chacalounet opened 5 years ago

chacalounet commented 5 years ago

#####################################

this is the console.log of loop lines in order.

#####################################

material-components-web.js:7460 focusTrap.checkFocusIn => tryFocus(state.mostRecentlyFocusedNode || getInitialFocusNode()) :

null

<a class=​"mdc-list-item" href=​"#" aria-selected=​"true" tabindex=​"0">​…​

material-components-web.js:7514 focusTrap.tryFocus => node.focus() :

<a class=​"mdc-list-item" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

material-components-web.js:6626 focusTrap.tryFocus => node.focus() :

<input type=​"text" class=​"mdc-text-field__input" id=​"username-input" name=​"username">​

material-components-web.js:6626 focusTrap.tryFocus => node.focus() :

<a class=​"mdc-list-item mdc-list-item--activated" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

material-components-web.js:6626 focusTrap.tryFocus => node.focus() :

<input type=​"text" class=​"mdc-text-field__input" id=​"username-input" name=​"username">​

material-components-web.js:7460 focusTrap.checkFocusIn => tryFocus(state.mostRecentlyFocusedNode || getInitialFocusNode()) :

<a class=​"mdc-list-item" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

<a class=​"mdc-list-item mdc-list-item--activated" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

material-components-web.js:7514 focusTrap.tryFocus => node.focus() :

<a class=​"mdc-list-item" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

material-components-web.js:6571 focusTrap.checkFocusIn => tryFocus(state.mostRecentlyFocusedNode || getInitialFocusNode()) :

<a class=​"mdc-list-item mdc-list-item--activated" href=​"#" aria-selected=​"true" tabindex=​"0">​…​

​ <input type=​"text" class=​"mdc-text-field__input" id=​"username-input" name=​"username">​

material-components-web.js:6626 focusTrap.tryFocus => node.focus() :

<a class=​"mdc-list-item mdc-list-item--activated" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

material-components-web.js:6571 focusTrap.checkFocusIn => tryFocus(state.mostRecentlyFocusedNode || getInitialFocusNode()) :

<a class=​"mdc-list-item mdc-list-item--activated" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

<input type=​"text" class=​"mdc-text-field__input" id=​"username-input" name=​"username">​

material-components-web.js:7460 focusTrap.checkFocusIn => tryFocus(state.mostRecentlyFocusedNode || getInitialFocusNode()) :

<a class=​"mdc-list-item" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

<a class=​"mdc-list-item mdc-list-item--activated" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

material-components-web.js:7514 focusTrap.tryFocus => node.focus() :

<a class=​"mdc-list-item" href=​"#" aria-selected=​"true" tabindex=​"0">​…​​

... ...

#################################################

this is the console error Maximum call stack size exceeded.

Order is from bottom to the top

#################################################

material-components-web.js:7514 Uncaught RangeError: Maximum call stack size exceeded. at tryFocus (material-components-web.js:7514) at HTMLDocument.checkFocusIn (material-components-web.js:7460) at tryFocus (material-components-web.js:6625) at HTMLDocument.checkFocusIn (material-components-web.js:6571) at tryFocus (material-components-web.js:7514) at HTMLDocument.checkFocusIn (material-components-web.js:7460) at tryFocus (material-components-web.js:6625) at HTMLDocument.checkFocusIn (material-components-web.js:6571) at tryFocus (material-components-web.js:7514) at HTMLDocument.checkFocusIn (material-components-web.js:7460) tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 checkFocusIn @ material-components-web.js:6571 tryFocus @ material-components-web.js:7514 checkFocusIn @ material-components-web.js:7460 tryFocus @ material-components-web.js:6625 (anonymous) @ material-components-web.js:6488 setTimeout (async) delay @ material-components-web.js:6650 addListeners @ material-components-web.js:6487 activate @ material-components-web.js:6431 trapFocus @ material-components-web.js:14151 (anonymous) @ material-components-web.js:14399 setTimeout (async) (anonymous) @ material-components-web.js:14397 setTimeout (async) (anonymous) @ material-components-web.js:14606 requestAnimationFrame (async) runNextAnimationFrame_ @ material-components-web.js:14603 open @ material-components-web.js:14391 open @ material-components-web.js:14112 ###################################################### window.tools.ui.notifications.dialogs.open @ notifications.js:308

==> ## opening login form dialog and closing the drawer

###################################################### window.tools.current.on @ index.js:498
foreach @ tools.js:1423 foreach @ tools.js:169 _foreach @ tools.js:1299 foreach @ tools.js:1303 foreach @ tools.js:1420 (anonymous function) @ tools.js:1341 ############################################ window.main.login.link.addEventListener @ index.js:509

==> ## click on drawer link to open the login dialog

############################################

############################################

I think, this problem happend because the browser's autofocusing login user form when modal is openning. Good luck

############################################

moog16 commented 5 years ago

Can we get some html or JS (codepen or glitch) to give us some insight?

abhiomkar commented 5 years ago

Since there is no activity, closing this issue.

The-Don-Himself commented 5 years ago

Kindly reopen this issue. I've been facing it as well ever since an update from 2 versions ago. Same component, the modal dialog. I think my scenario can help debug this, it happens on my live web app. https://campus-discounts.com/

I call the dialog using a button called social login.

If this button is on a normal page .i.e main content of the body, then there's no issue. For example here https://campus-discounts.com/join

If this button is on a sidebar (mine is on the top left), then the error occurs. For example, same page but open sidebar and click social login.

kfranqueiro commented 5 years ago

Thanks for this report! It sounds then like this happens in cases where you're opening a modal from another modal.

Does it happen if you close the modal drawer before opening the dialog?

The-Don-Himself commented 5 years ago

@kfranqueiro You're welcome for the report and also I believe your hunch is correct, I've just toyed with my dev instance and added a function to close the modal (sidebar) first and the error does not occur.

acdvorak commented 5 years ago

Material Dialogs are not intended to be nested; there should only be one dialog on screen at a time.

From the Design Guidelines:

Dialogs are purposefully interruptive, so they should be used sparingly.

We might want to document this limitation in the Dialog README.

The-Don-Himself commented 5 years ago

@acdvorak, I'm not sure though, how accurate that is because I've had no issues using other dialogs together like displaying messages via the snackbar while a drawer is open, or while in a modal. Perhaps the issue is that two major dialogs were used together? Furthermore, it may be worth nothing that this error message only pops up on the first attempt, on subsequent attempts no error is logged and the modals work together without issue. Actually, even with the error message everything still seems to function well. I don't know enough about the spec or core MDC code to contribute more than that. Meanwhile, I've already pushed to my live app, @kfranqueiro's proposed solution of closing the modal drawer first before opening the modal dialog so I won't be facing the issue any more. But other than this minor issue, great library so far guys 👍

Small Edit: If you do want to debug it as an issue, perhaps it's also worth noting that previous versions had no problems with two modals, I can only recall this issue from about 2 versions ago i.e https://github.com/material-components/material-components-web/releases/tag/v0.44.0 and https://github.com/material-components/material-components-web/releases/tag/v0.44.1

acdvorak commented 5 years ago

Glad to hear Ken's solution worked for you! 😀

I moved this issue into our backlog; we'll investigate it as time allows.

Thanks for the additional info and kind words @The-Don-Himself! 🎉

chacalounet commented 5 years ago

this code was the problem for my case dialog('MDCDialog:opening', () => { drawer.open = false; });

abhiomkar commented 5 years ago

There've been significant performance improvements to external dependency focus-trap that drawer & dialog uses.

Are you still able to reproduce this issue with latest version?

mcsonka-cxn commented 5 years ago

I can confirm that the issue is still reproducible. Two modals opened at the same time gives an error in the console.

simpslandyy commented 4 years ago

I recently had this problem, hopefully this will help someone: Note: I'm not using Material-UI, just a modal made from scratch, but hopefully it'll be a clue as to how to fix potential issues using trap-focus and MUI

Below I've listed the things that led up to the problem. To summarize, I had a component that on component mount would initialize a trap, and on state change (modal open/close) it would active/deactivate the trap.

    this._focusTrap = focusTrap(`.am-modal`, {
      initialFocus:  () => {
        return document.getElementsByClassName(`am-modal`)
      }
    });
 if (this.props.open) {
      this._focusTrap.activate();
 } else {
      this._focusTrap.deactivate();
 }

I didn't see a problem with this implementation until I used the component multiple times on a single page (this could also explain the nested case scenario). The focus trap wasn't targeting a specific .am-modal but multiple .am-modal.

The solution was to take a unique identifier as a prop for each modal used on the page such that on component mount, a trap would be initialized for a specific modal.

    this._focusTrap = focusTrap(`${this.props.id}`, {
      initialFocus:  () => {
        return document.getElementById(`${this.props.id}`)
      }
    });