Make the condition on `this` more accurate. In principle it might run in... #97

Merged
merged 1 commit into from Dec 29, 2012

Projects

None yet

4 participants

Member

... a global context other than window.

Owner
majek commented Nov 29, 2012

Is this waiting for anything?

bump

i fail to see why this issue would have any sense of urgency attached to it... i doubt that this change could yield any practical benefit. as of today, SockJS only runs in browsers, and in browsers, window is the global object. if anything, it would seem that this is merely a style change, perhaps with an eye to the future, but low priority i think.

@majek majek added a commit that referenced this pull request Dec 29, 2012
@majek majek #97 - simplify tests 78160f8
@majek majek merged commit ec1632f into sockjs:master Dec 29, 2012
Owner
majek commented Dec 29, 2012

@fearphage Thanks for reminders I appreciate that!

@humanchimp Yeah, no urgency, but if there is a pull request hanging I should get back to it reasonably quickly - the fault is on my side.

@squaremo I applied the patch, although I didn't merge the tests. I think that running full network tests is too heavy for this small patch. Instead I propose a basic unit test that just checks for the type of returned object.

Owner
majek commented Dec 29, 2012

Gosh, instanceof is not supported by IE6. Is there a workaround?

Owner
majek commented Dec 29, 2012

Not true: http://msdn.microsoft.com/en-us/library/ie/zh0zb36z(v=vs.94).aspx Now I'm confused! If it works I could remove the if from tests/

@majek majek added a commit that referenced this pull request Dec 29, 2012
@majek majek #97 - okay, instanceof should be working in ie d0bc7ec
Member

@majek Yes fair enough about the test -- thanks for correcting

@squaremo squaremo deleted the squaremo:cons_without_new branch Dec 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment