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

IE10 nativeEvent object missing returnValue for input onChange events #6699

Closed
jslatts opened this issue May 4, 2016 · 12 comments
Closed

IE10 nativeEvent object missing returnValue for input onChange events #6699

jslatts opened this issue May 4, 2016 · 12 comments

Comments

@jslatts
Copy link

jslatts commented May 4, 2016

I just ran into this issue while testing the upgrade from 0.14.8 to 15.2 in IE 10. If preventDefault() or stopPropagation() is called inside the handler for an onChange event of an input control, a "Member not found" exception is thrown. Stepping through the code, it appears that the the nativeObject reference is broken. Accessing most of the properties of the MSEventObj will throw the same exception.

I am able to reproduce it with the minimal case below on a clean IE10 Windows7 test VM from Microsoft.

Repro Fiddle: https://jsfiddle.net/h6qfwa51/3/

@jslatts jslatts changed the title IE 10 nativeEvent object missing returnValue for input onChange events IE10 nativeEvent object missing returnValue for input onChange events May 4, 2016
@jimfb
Copy link
Contributor

jimfb commented May 4, 2016

Haven't looked into it, but I'll tentatively tag this as a regression since apparently it worked on 0.14.8.

@jslatts sounds like you've started looking into this already. Do you want to submit a PR for this?

@jslatts
Copy link
Author

jslatts commented May 5, 2016

@jimfb I will definitely take a crack at tracking down the issue and will submit a PR if I can figure it out. But this will be my first foray into the React codebase, so anyone who knows what they are doing should feel free to jump in. Also would be nice to have someone else confirm the issue.

@zpao
Copy link
Member

zpao commented May 5, 2016

Is this only happening with IE10? Does this only happen when using the development build? My guess is that it's related to the new warnings for synthetic events so that's where I would start investigating:

Object.defineProperty(this, 'nativeEvent', getPooledWarningPropertyDefinition('nativeEvent', null));

@jslatts
Copy link
Author

jslatts commented May 6, 2016

I'm only set up to test in IE10 at the moment. The issue happens on the 15.0.2 release distributed via FB CDN (which is used in my test case above).

The good news is that I located the commit and line of code that caused the bug.

It also appears to be fixed in master, so assuming that the commit with the fix lands in a future release, I don't think there is any work to do here.

@jimfb
Copy link
Contributor

jimfb commented May 6, 2016

@jslatts Would you mind verifying in http://jsfiddle.net/9kungxn4/ which is a build that contains the commit you mentioned.

@jslatts
Copy link
Author

jslatts commented May 6, 2016

@jimfb this fork of that test works correctly in IE10/Win7 and IE10/Win8 for me.

@jimfb
Copy link
Contributor

jimfb commented May 6, 2016

Excellent, I'll close this out! Thanks @jslatts!

@jimfb jimfb closed this as completed May 6, 2016
@zpao
Copy link
Member

zpao commented May 9, 2016

It looks like that change isn't likely to make it into 15 so you may be waiting a while for the fix. If there's something smaller that fixes your issue we might be able to take that.

@jslatts
Copy link
Author

jslatts commented May 9, 2016

@zpao smaller like just reverting this line of code:

!('documentMode' in document) || document.documentMode > 11

back to

!('documentMode' in document) || document.documentMode > 9

?

@tjcampbell
Copy link

@zpao @jslatts I just stumbled across this. Is there a workaround for the time being? I see the fix is only planned for 16.0. I can confirm simply reverting document.documentMode > 11 to document.documentMode > 9 as mentioned by @jslatts fixes the issue.

This must be breaking plenty of sites on IE10.

@jslatts
Copy link
Author

jslatts commented May 24, 2016

@tjcampbell I reverted back to 0.14.8 as a workaround. I would be happy to submit the PR with the one line change above it @zpao confirms they will accept it. I just don't know enough about the intent behind the change in the first place and possible side effects.

@tjcampbell
Copy link

Might help getting @jquense's opinion on this as he made the change. @zpao can we re-open this? I feel this is urgent enough to warrant a separate fix before #5746 is merged. This issue, #6614 and #6822 seem to be caused by #4051, making React 15 pretty unusable on IE.

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

4 participants