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

Support for IE8 when document.domain is set #306

Merged
merged 1 commit into from
Jun 6, 2015
Merged

Support for IE8 when document.domain is set #306

merged 1 commit into from
Jun 6, 2015

Conversation

danjamin
Copy link
Contributor

@danjamin danjamin commented Jun 6, 2015

In relation to issue #150 -- took commit 4f738ac and applied it to es5-sham

Basically, the problem is that in IE < 9, when document.domain is set, using the es5-sham causes a "Permission denied" error. What this fix does, is uses a different approach to the Object.create polyfill when this situation arises. It leaves the original iframe approach intact for all other cases.

For those of us still supporting IE8 😩, and where document.domain is necessary for an API being used via iframes (x-origin reasons), this fix should allow es5-sham to be used without error.

SUPPORTS_ACTIVEX = false;
}

return SUPPORTS_ACTIVEX && document.domain;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this if (!document.domain) { return false; } on the first line of the function, to avoid the try/catch when it's not set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed 👍

@ljharb
Copy link
Member

ljharb commented Jun 6, 2015

This is great - I suspect this can't be tested for without making a separate HTML file, since document.domain can't be unset once set?

@ljharb ljharb added the ES5 sham label Jun 6, 2015
@danjamin
Copy link
Contributor Author

danjamin commented Jun 6, 2015

@ljharb agreed that testing this specific edge case might be a bit tricky given the IE8 environment and document.domain needing to be set

@ljharb
Copy link
Member

ljharb commented Jun 6, 2015

@danjamin It would be super helpful if you could set up a jsfiddle, that used rawgit.com to point to your PR's test file, and set document.domain so I can use it to verify in various browser versions :-) if not, i'll wait to merge it til I have time to set that up.

@danjamin
Copy link
Contributor Author

danjamin commented Jun 6, 2015

@ljharb I'll give it a shot -- will ping you when it's ready

@ljharb
Copy link
Member

ljharb commented Jun 6, 2015

Thanks!

@danjamin
Copy link
Contributor Author

danjamin commented Jun 6, 2015

@ljharb turns out jsfiddle doesn't let you adjust the document.domain; so I just created a quick gh-pages for my fork; you can visit it here http://danjamin.github.io/es5-shim/

it has 2 pages (/broken and /fixed) -- if you visit both in IE8 you will see that the broken one has the "Permission Denied" error where as the fixed one does not error on the Object.create(null) invocation.

The only real difference between /broken and /fixed is that one points at es5-shim/master of es5-sham.js and the other at the version in the branch in my fork being pull requested.

@ljharb
Copy link
Member

ljharb commented Jun 6, 2015

Awesome, that's good enough for me! https://github.com/danjamin/es5-shim/commit/c471214c1c8c4f6a1042c2ef53a0e69cf2ac5ad0 confirms it for me in my testing.

ljharb added a commit that referenced this pull request Jun 6, 2015
Support for IE8 when document.domain is set
@ljharb ljharb merged commit 7b1321b into es-shims:master Jun 6, 2015
@danjamin danjamin deleted the document-domain-ie8 branch June 6, 2015 17:23
@danjamin
Copy link
Contributor Author

danjamin commented Jun 6, 2015

Cool -- glad I could help 😃

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

Successfully merging this pull request may close these issues.

2 participants