IE10 WebWorkers #16

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Collaborator

Sebmaster commented Feb 25, 2013

This provides the ability to execute code as a WebWorker.

The performance of this version is abysmal though and definitly not mergeable.
Even though it is a webworker, it locks the UI thread; I'm not sure if something is absolutly wrong with my code though.

Does something in there stick out to someone/@adambom?

This starts the work for issue #12 (new issue since you can't append pull requests to existing issues).

Collaborator

Sebmaster commented Feb 26, 2013

So, @adambom either something was up with my browser/system yesterday, or the latest code changes fixed the UI lock in IE. Either way, I think it's fine now and works in my application with Chrome 25, IE10 and FF 20.

Have a look at it please; before you merge though, I'd like to squash the commits into a single one before you merge though (or you do that while merging in the master brach itself), so we don't clutter up the commit history of master.

Collaborator

Sebmaster commented Feb 26, 2013

Rebased on master because of the whitespace changes yesterday.

Collaborator

Sebmaster commented Feb 27, 2013

I squashed the commit together now since the partial comments didn't make much sense anyways. If you've verified it works it's ready to merge I guess.

Collaborator

adambom commented Feb 28, 2013

Here's what I'd like to do. Since the next major release is going to include a pretty major refactor to the RemoteRef API, I'd like to merge this on the "new-syntax" branch.

What we should do is create a method on P, similar to _require, called options. options takes an object as an argument and sets it as this._options.

One option, which we will document, is ie10Shim. Users can set that option prior to creating any RemoteRefs. Then any time we call the RemoteRef constructor we pass options as the third argument. In the RemoteRef constructor, where you're currently doing e.code === 18 && evaljs do e.code === 18 && options.ie10shim instead. Then we can get rid of hard coding the default url to eval.js

I think we should also throw a specific error message if we get the security error, but the ie10shim option is undefined.

Does that make sense/sound good?

Collaborator

Sebmaster commented Feb 28, 2013

Sounds good, are you going to merge new-syntax into master soon? If not, I may have to open a new pull request and rebase onto new-syntax.

Collaborator

adambom commented Feb 28, 2013

I'll probably tackle the merge this weekend.

Sebmaster closed this Apr 22, 2013

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