Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo operation on text input throws exception in IE9 #11609

Closed
nhunzaker opened this issue Nov 21, 2017 · 6 comments
Closed

Undo operation on text input throws exception in IE9 #11609

nhunzaker opened this issue Nov 21, 2017 · 6 comments

Comments

@nhunzaker
Copy link
Contributor

Do you want to request a feature or report a bug?

This is a bug report. It looks like IE9 is unmounting text nodes when undoing text input operations. If the display value of the input is rendered in another component, it raises an exception because IE9 does not allow changes to nodeValue on unmounted text nodes.

I think that's the issue, anyway. It is documented here:

https://connect.microsoft.com/IE/feedbackdetail/view/944330/invalid-argument-error-when-changing-nodevalue-of-a-text-node-removed-by-setting-innerhtml-on-an-ancestor

What is the current behavior?

  1. Open http://react-dom-test-fixtures.surge.sh/number-inputs in IE9
  2. Change the text in the first controlled text input
  3. Start debugging in the IE9 developer tools
  4. Press ctrl+z to undo your text change
  5. IE9 raises an exception when setting the nodeValue of the text label to the right of the input "invalid arguments":

What is the expected behavior?

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

This affects IE9 when using React 16. It is also an issue on master.


The issue springs up in ReactDOM.js:

https://github.com/facebook/react/blob/master/packages/react-dom/src/client/ReactDOM.js#L392-L398

I think there just needs to be a wrapper around the text node to see if it has a parent before updating. Maybe something like:

commitTextUpdate(
  textInstance: TextInstance,
  oldText: string,
  newText: string,
): void {
  // IE9 will raise an exception if modifying a detached text node
  // https://connect.microsoft.com/IE/feedbackdetail/view/944330/invalid-argument-error-when-changing-nodevalue-of-a-text-node-removed-by-setting-innerhtml-on-an-ancestor
  if (textInstance.parentNode) {
    textInstance.nodeValue = newText;
  }
}

But that feels like a band-aid solution. I'm curious what is causing the text node to unmount to begin-with.

@chrisdieckhaus
Copy link

Does this issue still need fixing?

@nhunzaker
Copy link
Contributor Author

To my knowledge, yes.

@yifeng-ruan
Copy link

I also faced this issue. do you fix it now?

@nhunzaker
Copy link
Contributor Author

Hey! Nope, and it looks like it's still an issue. I sent out a PR here:
#12505

@nhunzaker
Copy link
Contributor Author

Very curious! It looks like this behavior only occurs when you use onChange, not something like onKeyUp.

So something is happening in the ChangeEvent plugin that causes IE9 to revert text nodes when undoing.... I wonder.

@gaearon
Copy link
Collaborator

gaearon commented Mar 29, 2022

We are dropping support for IE in React 18, so this won't get a fix.
Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants