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

IE8 does not have a setter for property "enctype" #2135

Merged
merged 1 commit into from
Feb 4, 2015
Merged

IE8 does not have a setter for property "enctype" #2135

merged 1 commit into from
Feb 4, 2015

Conversation

syranide
Copy link
Contributor

@syranide syranide commented Sep 3, 2014

f = document.createElement('form');
f.enctype = 'multipart/form-data';
console.log(f.enctype);
console.log(f.encoding);
console.log(f.getAttribute('enctype'));

Outputs (on IE8):

LOG: multipart/form-data
LOG: application/x-www-form-urlencoded
LOG: application/x-www-form-urlencoded

Whereas with f.encoding = 'multipart/form-data'; or f.setAttribute('enctype', 'multipart/form-data');:

LOG: multipart/form-data
LOG: multipart/form-data
LOG: multipart/form-data

Unlikely that anyone would ever bump into this, but it is super-weird if you would and it's pretty much for free so why not.

@syranide
Copy link
Contributor Author

syranide commented Sep 4, 2014

Switched it to use encoding instead, now both attribute and property access works. Works in all browsers I have my hands on, including FF3 (it should be more backwards compatible than enctype and as such safe, W3 HTML5 requires encoding to alias enctype).

@syranide syranide changed the title Property "enctype" is not understood by IE8 IE8 does not have a setter for property "enctype" Sep 4, 2014
@zpao
Copy link
Member

zpao commented Sep 23, 2014

cc @yungsters

@yungsters
Copy link
Contributor

Thanks for fixing this. In addition to having to set encoding, Internet Explorer will also have issues — either throws or silently fails — with setting encoding to an invalid value such as null. For what it's worth...

@syranide
Copy link
Contributor Author

@yungsters IE8 throws, but I figure it's not worth special-casing as you really really should use valid mimetypes (and everyone uses one of the two common, with one of them being default too).

@syranide
Copy link
Contributor Author

Perhaps we should introduce an THROWS_IN_OLDIE which we can apply to some of the less common properties which offends old IEs when given a non-supported value, which would simply do the assignment inside a try-catch.

@yungsters
Copy link
Contributor

Maybe. Can you add the following test case to your Test Plan (ine IE8):

React.renderComponent(<form enctype="multipart/form-data" />, ...);
React.renderComponent(<form />, ...);

@syranide
Copy link
Contributor Author

@yungsters

React.renderComponent(React.DOM.form({encType: "multipart/form-data"}), document.body);
> "multipart/form-data"
React.renderComponent(React.DOM.form(), document.body);
> "application/x-www-form-urlencoded"

React.renderComponent(React.DOM.form({encType: "multipart/form-data"}), document.body);
> "multipart/form-data"
document.body.firstChild.removeAttribute('enctype');
> "application/x-www-form-urlencoded"

It works because of getDefaultValueForProperty, I also tested with removeAttribute if/when #1510 is accepted (EDIT: tested in IE8 ofc).

@syranide
Copy link
Contributor Author

syranide commented Feb 3, 2015

@yungsters Pinging here as well :)

As for IE8 throwing when setting invalid values, it is a widespread issue with IE8 and I would say outside the scope of this PR.

@yungsters
Copy link
Contributor

Looks good to me.

syranide added a commit that referenced this pull request Feb 4, 2015
IE8 does not have a setter for property "enctype"
@syranide syranide merged commit 6672a7e into facebook:master Feb 4, 2015
@syranide syranide deleted the ie8enctype branch February 4, 2015 12:47
This pull request was closed.
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