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

Wrap IE specific files to properly set methods on window or global #628

Closed
wants to merge 6 commits into from

Conversation

GCheung55
Copy link
Contributor

Unwrapped code can cause issues with this if the code is appended to other code that has 'use strict' defined outside of a function scope.

To prevent any issues in reference to this, wrap the IE code in a self-executing function and make any changes to a reference to window or global.

@cjohansen
Copy link
Contributor

Have you tested this in IE? I seem to remember the function declarations being essential in making this work at all.

@GCheung55
Copy link
Contributor Author

I've only tested in IE8 but only manually making changes in the buster scaffolding file. I'm having difficulty running the test page in this repo because the dependencies are out of date. I just ran npm install and http-server in the root dir. I'm not sure what the issue is but a little help would be... Helpful :D

I found an article regarding setting the functions: http://www.adequatelygood.com/Replacing-setTimeout-Globally.html

Once the repo is updated I can test out the solution from the article.

@cjno
Copy link

cjno commented Dec 19, 2014

Hey, not sure I should be in this project. You might have me mixed up with someone else.

@GCheung55
Copy link
Contributor Author

@cjno sorry wrong person.
@cjohansen I'm trying to run tests in ie6 and getting an error that 'when' is undefined.

@cjno
Copy link

cjno commented Dec 19, 2014

@GCheung55 No worries, good luck and merry christmas

@cjohansen
Copy link
Contributor

...but the tests pass in IE7 and up? I'm not sure IE6 support is very relevant anymore.

@GCheung55
Copy link
Contributor Author

@cjohansen I did some more digging. The main issue is that the dev dependencies are out of date, as I described in #630. After I manually changed buster-test/lib/buster-test/test-context.js locally, things started working in IE8.

This comment may not bode well for the PR, but a caveat is that the IE specific files needs to execute before anything else.

That said, I would like you to still consider accepting this PR because of the findings in the http://www.adequatelygood.com/Replacing-setTimeout-Globally.html article, especially in the section "A Better Solution".

@cjohansen
Copy link
Contributor

That's awesome, thanks! 9ac6834

@GCheung55
Copy link
Contributor Author

Closing because the changes were added.

@GCheung55 GCheung55 closed this Jan 7, 2015
@GCheung55 GCheung55 mentioned this pull request Feb 18, 2015
20 tasks
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.

3 participants