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

ReactJS support for IE6/IE7 (reimplemented) #2169

Closed
wants to merge 6 commits into from

Conversation

mtsyganov
Copy link

After discussion in PL2133 I have made a lighter implementation of ChangeEventPlugin to support IE6/IE7 so it dont breaks existing behaviour of all other browsers.

Following changes were made to support old IE browsers IE6 and IE7

  1. Algorithm to distinguish user event value changes and JS value changes on INPUT and TEXTAREA in ChangeEventPlugin was implemented for IE6/IE7 by using datetime diffs
  2. String[0] replaced by String.charAt(0) in setInnerHTML
  3. Checking renderNode.hasAttribute extended by IE6/IE7 compatible attribute check

To run react.js apps with IE6/IE7 you have to use es5-shims, console-polyfill and queryselector-polyfill

1. Algorithm to distinguish user event value changes and JS value changes on INPUT and TEXTAREA in ChangeEventPlugin was implemented for IE6/IE7 by using datetime diffs
2. String[0] replaced by String.charAt(0) in setInnerHTML
3. Checking renderNode.hasAttribute extended by IE6/IE7 compatible attribute check
To run react.js apps with IE6/IE7 you have to use es5-shims, console-polyfill and queryselector-polyfill from bower
@syranide
Copy link
Contributor

The algorithm for 1 doesn't seem very robust, I'm pretty sure (but not certain) that it could be made without arbitrary thresholds for time.

I don't have access to IE6/IE7, but I really doubt that typeof renderNode[RESULT_INDEX_ATTR] !== 'undefined' is identical to renderNode.hasAttribute(RESULT_INDEX_ATTR). There are lots of (non-standard) attributes that doesn't have properties.

Also, does IE6/IE7 support value-less attributes?

@mtsyganov
Copy link
Author

@syranide

  1. Give me please your ideas. I have tried with VBScript, but had no success. My previous implementation worked with additional node attribute, but I had to change some more places in react.
  2. You can get access easy with VM lite. There is an automatic installation of windows xp. It creates a virtualbox machine and installs automatic windows xp with IE6.
    hasAttribute shim for IE7 is not my idea. I have taken it from stackoverflow.
  3. why to you ask?

@mtsyganov
Copy link
Author

@syranide I have checked renderNode.getAttributeNode(RESULT_INDEX_ATTR) !== null It works also. Should I replace typeof with getAttributeNode?

@mtsyganov
Copy link
Author

@syranide thank you for feedback. I have made some improvements.

The changes I have made are very very small. The benefit is, that with these changes you can now run reactjs also on very old browsers. Without these changes you can't run on IE6/IE7 even examples from the homepage of react. And changes I made don't have influence on working functionality.

@zpao
Copy link
Member

zpao commented Sep 25, 2014

I'm going to wontfix this based on what I said before. We will undoubtedly break this and that just means unknown pain down the line. The changes now are small but it's unclear what they will become. And honestly, the day I can make a case for dropping IE8 support is the day I make a case for removing a bunch of code from React.

But like I said before, I totally support you or somebody else maintaining a fork and keeping it up to date. We'll even point people who ask towards it. But we won't be maintaining it ourselves.

@mtsyganov
Copy link
Author

@zpao ok. Thank you for your attention and your feedback. I have published my fork react-ie on npm. Till today the package collected over 300 downloads with out any marketing. It's not much, because IE6/IE7 are dead, but we see there is a demand.
It would be great, if you can tell about this IE6/IE7 on the official react.js blog.

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

Successfully merging this pull request may close these issues.

3 participants