Undefined error in contentHasFocus method

[saradacp]

Summary:

Got the following error in the browser console when using react modal Uncaught TypeError: Cannot read property 'contains' of undefined at Object.contentHasFocus (ModalPortal.js:171) at Object.focusContent (ModalPortal.js:101) at Object.componentDidUpdate (ModalPortal.js:66) at measureLifeCyclePerf (ReactCompositeComponent.js:75) at ReactCompositeComponent.js:729 at CallbackQueue.notifyAll (CallbackQueue.js:76) at ReactReconcileTransaction.close (ReactReconcileTransaction.js:80) at ReactReconcileTransaction.closeAll (Transaction.js:206) at ReactReconcileTransaction.perform (Transaction.js:153) at ReactUpdatesFlushTransaction.perform (Transaction.js:140)

Steps to reproduce:

1. When populating contents in the modal dynamically

Expected behavior:

Should be able to populate contents without any errors.

Additional notes:

A small undefined check in the react-modal.js resolved the issue. Will make a PR with the fix.

[claydiffrient]

What version of react-modal did you experience this with?

[saradacp]

1.6.5

[saradacp]

@claydiffrient : This fix is critical for us. The exception above might be a peculiar case but the code should work fine if this.content is undefined. Would you be able to fix it? Even if you run the test some of the tests are skipped because of this error. Please let me know if you want to talk over phone for more details. Thanks!

[diasbruno]

@saradacp I cannot reproduce this behavior.

If you can provide more details about what's happening it would be helpful.

Some ideas:

• What happens when this content is empty/clean?
• Are this content elements of a form?
• Is there something trying to focus other element while the modal is opened/opening?

[diasbruno]

@saradacp Another option for a temporary fix for this issue is to fork the project and make a branch with the fix. Then you can include your fork (and branch) in you package.json, until we find out what's going on.

[saradacp]

@diasbruno : Thanks for the advice. Will do that temporarily. The error is happening because the content is undefined It's failing in (ModalPortal.js Line ~189) contentHasFocus () { return document.activeElement === this.content || this.content.contains(document.activeElement); } In the above code when setting the active element this.content is undefined hence failing at this point. In my case, I have multiple modals and we close one modal while opening the other and the contents are fetched from an API. Is it possible to add a null check before calling contains? Thanks

[diasbruno]

If it don't drive the focus management crazy, it's the fix. :)

I believe that this line is suspicious ModalPortal.js#L216. A good place to see what's happening to this.content.

I'll try to make a test scenario for this case.

[diasbruno]

For some reason, the value of this.content gets defined, then null(??) and then this.content is defined again.

I cannot reproduce this error on none of the browsers. What was the browser that throws this error?

I guess checking for this.content != null can be safe.

[saradacp]

@diasbruno : I see that you made a few code changes but don't see the null check though. Did you get chance to publish the changes? I'm seeing the issue in chrome browser. Here is my use case:

1. I open a modal on a button click ---> this modal has a hyperlink content inside
2. clicking on that hyperlink would open another modal. ---> The second modal has a button inside
3. clicking on that button suppose to open the modal#1 again but here the code throws error
   Uncaught TypeError: Cannot read property 'contains' of undefined at Object.contentHasFocus (ModalPortal.js:171) at Object.focusContent (ModalPortal.js:101) at Object.componentDidUpdate (ModalPortal.js:66) at measureLifeCyclePerf (ReactCompositeComponent.js:75) at ReactCompositeComponent.js:729 at CallbackQueue.notifyAll (CallbackQueue.js:76) at ReactReconcileTransaction.close (ReactReconcileTransaction.js:80) at ReactReconcileTransaction.closeAll (Transaction.js:206) at ReactReconcileTransaction.perform (Transaction.js:153) at ReactUpdatesFlushTransaction.perform (Transaction.js:140)

[diasbruno]

Thanks, @saradacp. I'm still not sure if that is the case. The scenario you post is kinda odd, but I'll try to reproduce this problem and see what is going on.

[saradacp]

@diasbruno : Thank you for your continuous support. I tried to add the null check but the tests are failing. Attaching the code diff and failed test cases for reference. We have release in 2 weeks and desperately waiting for the fix :(

react-modal-test-error.txt

react_modal_diff.txt

[diasbruno]

The problem seems to be on contentHasFocus() which tries to call this.content.contains(...). So, probably this.content can be null or a DOMElement that doesn't has the method .contains().

On the images I posted, it's expected that this.content has the <div /> created for the modal.

The best way to find out is writing a console.log(this.content) before the return statement from contentHasFocus.

Before calling contains() we need to check if:

• this.content is null, than the function should return false.
• this.content is any other DOMElement other than the <div />, contains() should not be called. and return false.

[saradacp]

@diasbruno : I 'm getting the modal content as undefined in contentHasFocus.@173. Could you add a null check and publish the code?

[diasbruno]

This should help you, but this is not a proper fix since we can't reproduce this behavior. So, you'll have to publish a patch on your fork.

contentHasFocus () {
  return document.activeElement === this.content || 
    (this.content && this.content.contains(document.activeElement));
}

[saradacp]

@diasbruno: Having && in contentHasFocus actually resolves my issue and it does not have any side effects. Not sure why this can't be fixed. Unfortunately we can't fork this using company github and we have a release in 10days. Would greatly appreciate if you could fix this. Thanks

[saradacp]

@diasbruno :

This is my modal invocation. We use close/open the modal using showFindAClub redux state variable. Once this contains of undefined error happens the modal stops expected behavior. Please advise!

[constantx]

Running onto this same isssue

[diasbruno]

@constantx did you found a way to fix this issue? I still don't have a clue about this one. Maybe it's the way it does the unmount (?).

[lindskogen]

@diasbruno:

For some reason, the value of this.content gets defined, then null(??) and then this.content is defined again.

This is actually expected behaviour for refs (if defined with an inline function), see: https://facebook.github.io/react/docs/refs-and-the-dom.html#caveats

[diasbruno]

That's weird. I was suspecting that we could have something like a reference to something that can no longer be reached.

[diasbruno]

And it's really hard bug to reproduce this bug.

[diasbruno]

I also think that it can be related to componentWillUnmount before componentDidMount and it mess up everything (but this is a guess).

[tstirrat15]

I've been seeing this issue as well through Sentry.

Browser: Chrome Mobile 44.0.2403 OS: Android 6.0.1

[diasbruno]

@tstirrat15 can you provide more info about this? ...or steps to reproduce.

[tstirrat15]

No idea on steps to reproduce... it just came in on our error tracker. Here's a stack trace, though, if that would help:

TypeError: Cannot read property 'contains' of undefined
  at contains(~/react-modal/lib/components/ModalPortal.js:168:0)
  at contentHasFocus(~/react-modal/lib/components/ModalPortal.js:107:0)
  at focusContent(~/react-modal/lib/components/ModalPortal.js:67:0)
  at call(~/react-dom/lib/CallbackQueue.js:76:0)
  at notifyAll(~/react-dom/lib/ReactReconcileTransaction.js:80:0)
  at call(~/react-dom/lib/Transaction.js:206:0)
  at closeAll(~/react-dom/lib/Transaction.js:153:0)
  at call(~/react-dom/lib/Transaction.js:140:0)
  at call(~/react-dom/lib/ReactUpdates.js:89:0)
  at perform(~/react-dom/lib/ReactUpdates.js:172:0)
  at flushBatchedUpdates(~/react-dom/lib/ReactUpdates.js:47:0)
  at call(~/react-dom/lib/Transaction.js:206:0)
  at closeAll(~/react-dom/lib/Transaction.js:153:0)
  at call(~/react-dom/lib/ReactUpdates.js:89:0)
  at perform(~/react-dom/lib/ReactUpdates.js:172:0)
  at call(~/react-dom/lib/Transaction.js:206:0)
  at closeAll(~/react-dom/lib/Transaction.js:153:0)
  at perform(~/react-dom/lib/ReactDefaultBatchingStrategy.js:62:0)
  at batchedUpdates(~/react-dom/lib/ReactUpdates.js:97:0)
  at batchedUpdates(~/react-dom/lib/ReactEventListener.js:147:0)
  at apply(~/raven-js/src/raven.js:298:0)

[diasbruno]

The problem with this issue is that it's hard to reproduce. ¬¬

[tstirrat15]

Yeah.. I feel you. It's not a stop-and-fix for us, because it's isolated to a very small number of users/cases. I just wanted to provide what information I could.

[diasbruno]

Can you describe the behaviour/activity of your component?

[tstirrat15]

This is the way we're wrapping it:

function Modal({
  modalOpen,
  closeModal,
  label,
  children,
}) {
  return (
    <ReactModal
      isOpen={modalOpen}
      contentLabel={label}
      onRequestClose={closeModal}
      className={s.content}
      overlayClassName={s.overlay}
    >
      {children}
      <CloseButton
        onClick={closeModal}
      />
    </ReactModal>
  );
}

Where:

• modalOpen is redux state that indicates whether the modal is open
• closeModal is the redux action creator to close the modal

We're using it for a review process, where the user can suggest changes to a transaction before it's approved. You click a button to open the modal and make changes, and then the modal is either closed by submitting the form or by esc/clicking out/clicking a close button/etc.

[diasbruno]

hmm, ok, I'll use this info to make a test project later. Thank you, @tstirrat15!

[diasbruno]

@tstirrat15 which version are you using? react-modal.

[diasbruno]

Finally, got it!!!! It seems that this.focusAfterRender on ModalPortal.js#L80 can be in a wrong state.

Steps to reproduce

• Replace ModalPortal.js#L80 with ìf (true) {`.
• Mount a closed Modal.
• Do something that will ask the ModalPortal to fire componentDidUpdate.

Obs

• Error is triggered.

[tstirrat15]

Nice! Thank you, @diasbruno! Is this available in the most recent release?

[diasbruno]

@tstirrat15 yes, available on v2.0.6. There are some extra info on PR #424.

[diasbruno]

@tstirrat15 if sentry still receives errors, please feel free to reopen this issue.