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

ChangeEventPlugin.js IE detection is broken on Chrome/Safari/FF etc #10030

Closed
alirussell opened this issue Jun 23, 2017 · 4 comments
Closed

ChangeEventPlugin.js IE detection is broken on Chrome/Safari/FF etc #10030

alirussell opened this issue Jun 23, 2017 · 4 comments

Comments

@alirussell
Copy link

alirussell commented Jun 23, 2017

Bug

What is the current behavior?

On our website we include a number of scripts from 3rd parties (Via segment etc) and one of them seems to be modifying document.documentMode inadvertently. This in combination with the release of react-dom 15.6.0 has broken our website completely because of some code in ChangeEventPlugin.js.

The error activeElement.detachEvent is not a function is written in the console every time you try and click into an input box.

Its specifically this line that is causing the issues: https://github.com/facebook/react/blob/v15.6.1/src/renderers/dom/client/eventPlugins/ChangeEventPlugin.js#L159

The 3rd party library is actually just setting document.documentMode = undefined, but this in itself is enough to break the line 'documentMode' in document because even though documentMode is undefined, it now exists as a key in document. This then means on all browsers react-dom believes its running in IE.

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/84v837e9/).

To reproduce on any app that uses react-dom 15.6.0/15.6.1 and includes input boxes, add the following at the top of the head tag (so it executes immediately):

<script>
document.documentMode = undefined;
</script>

An example can be found here: https://codepen.io/anon/pen/gRxMGm

What is the expected behavior?

In previous versions (And the 16.0 branch) of reactdom, the code is just !document.documentMode which works fine and will allow you to load relevant 3rd party libraries without experiencing any issues.

We have reached out to the vendor to fix the issue on their side, but I'm not sure how long this will take.

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

Doesn't work in React Dom 15.6.0 and 15.6.1, but works in 15.5.4 and 16.0+

From looking at the history of the code, it seems to have gone back and forth between the two different ways of checking documentMode, but this is the commit where it changed back to the code that causes an error: 68347c9#diff-d7d43033e0c4300df2b535a9df27b8edR164

@arthens
Copy link

arthens commented Jun 23, 2017

Came here to report the same problem. Nice analysis @alirussell

@aweary
Copy link
Contributor

aweary commented Jun 23, 2017

Thanks for the detailed report @alirussell! I've opened a PR (#10032) to address this 👍

While setting document.documentMode is a questionable choice, reverting this check makes sense since it still works as expected (and we're still using !document.documentMode elsewhere in the change event plugin).

I'm hoping we can get this in a patch release for 15.6.

@alirussell
Copy link
Author

Sounds good @aweary, Thanks! 👍

@aweary
Copy link
Contributor

aweary commented Aug 8, 2017

#10032 has been merged, this should be out in the next 15.6 path release.

@aweary aweary closed this as completed Aug 8, 2017
@nhunzaker nhunzaker mentioned this issue Aug 10, 2017
11 tasks
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