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

Assigning some values breaks in IE8 #353

Closed
wants to merge 1 commit into from

Conversation

tracer99
Copy link

Libraries like MooTools fill in missing properties on the window object that exist in newer browsers.
However, some browsers like IE8 don't let you set those properties and this assignment causes an exception.

However, some browsers like IE8 don't let you set those properties and this assignment causes an exception in IE8
@guybedford
Copy link
Member

Thanks! Were you able to tell exactly which properties these were? We keep an ignored list at https://github.com/systemjs/systemjs/blob/master/lib/extension-global.js#L46 to track these, which is better for performance than introducing try-catch which deoptimizes the whole function.

@tracer99
Copy link
Author

I did. screenTop and screenLeft
However, these are needed for Firefox as FF is the browser that is missing these. So I'm unsure if globally excluding them will have a negative affect.

@guybedford
Copy link
Member

Awesome, no that's fine, we can add those to the ignore list. It just means the global auto-detection won't ever pick up if a global module calls itself window.screenTop = myGlobal.

@tracer99
Copy link
Author

Should I kill the pull request and make a new one to that list?

@guybedford
Copy link
Member

@tracer99 that would be great, happy to do it myself too when I'm on this project if you have better things to do though!

@guybedford
Copy link
Member

I've added these separately now. Thanks for reporting!

@guybedford guybedford closed this Mar 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants