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

onBeforeInput fires after Edge updates the DOM #8876

Closed
colinjeanne opened this issue Jan 27, 2017 · 2 comments
Closed

onBeforeInput fires after Edge updates the DOM #8876

colinjeanne opened this issue Jan 27, 2017 · 2 comments

Comments

@colinjeanne
Copy link

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

What is the current behavior?
In Edge the onBeforeInput event is fired after Edge has already made changes to the DOM negating the purpose of the event and breaking the assumptions of components built on this event, such as Draft.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).
https://jsfiddle.net/max_winderbaum/yv38s3xe/3/ demonstrates the issue in general but to see the effects on other components consider these steps

  1. Go to https://facebook.github.io/draft-js
  2. Type hello world
  3. Bold hello
  4. Underline the space between the words
  5. Select both hello and the space and press a
  6. The entire control's text is replaced with a
  7. Hit undo several times to observe further broken states until finally React fails with a NotFoundError in removeChild

This issue is opened in Draft because the core issue is that onBeforeInput is misbehaving on Edge and so breaks Draft's assumptions.

What is the expected behavior?

onBeforeInput should fire before any changes occur in the DOM

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

This repros in the latest version of Draft.

The root cause here is the same root cause as in #7269, although that issue no longer repros. The fix referenced in that issue, #7270, fixes this issue as well.

@colinjeanne
Copy link
Author

This does not repro in Edge/16.16299

@gaearon
Copy link
Collaborator

gaearon commented Jan 6, 2018

Thanks for letting us know. We’ll close then.

@gaearon gaearon closed this as completed Jan 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants