2.3.0 breaks PhantomJS 1.x, IE 7 & 8

[ScottFreeCode]

Mocha is using Browserify to bundle the project's tests for the browser. Just the other day we started getting errors due to use of Function.prototype.bind in PhantomJS 1 and due to use of Object.keys (in the safe-buffer dependency) in Internet Explorer 7 and 8. (I strongly suspect that Function.prototype.bind would be an issue in IE 7 &/ 8 as well except that they fail on the Object.keys issue first.) Specifically using readable-stream@2.2.11 fixes both issues.

(Between the fact that we can work around this as long as nothing is forcing 2.3 into the browser bundle and the fact that we're testing unusually old browsers, I'd understand if this is considered a non-issue; but I wanted to bring it to your attention for what it's worth.)

[mcollina]

Thanks for spotting those. I'd work on removing that bind(), it should not be there.

https://github.com/feross/safe-buffer/blob/master/index.js#L8-L12 can be easily refactored. Can you send a PR for that?

[ScottFreeCode]

I can't make any promises that I'll find the time, but yeah, these should both be straightforward to amend; I'll try to get something sent along before next week if I can get a moment to spare on it!

[calvinmetcalf]

opened a pull on safe-buffer

[ScottFreeCode]

Wow, y'all are fast! Thanks!!

[ScottFreeCode]

Phantom is working now (:tada:), but IE 7 and 8 are having still more trouble -- something about this line if I'm reading it right... probably Object.defineProperty? I was hoping to check that fixing the bind call works precisely to look out for additional issues like this, but at this point I'm not sure I could be much help, as defineProperty can be a lot less trivial to replace than bind -- depending on what it's being used for, anyway.

Let me know if you'd like me to open a new issue about this one.

[mcollina]

@ScottFreeCode are you sure it was working on IE 7 & 8 before readable-stream 2.3?

[ScottFreeCode]

are you sure it was working on IE 7 & 8 before readable-stream 2.3?

Yeah -- besides the fact that it was working until the day after 2.3 came out, mochajs/mocha#2898 also passes its IE7/8 tests by reverting to 2.2.11:
https://travis-ci.org/mochajs/mocha/jobs/245229191
https://travis-ci.org/mochajs/mocha/jobs/245229192

Depending on what the rest of the Mocha team thinks, we might end up just using that, though -- it wouldn't be the only older-than-latest dependency Mocha sticks to for stability purposes. Or we might end up doing a major version bump and dropping old IE support, since this isn't the only annoyance we've run into with them.

In the meantime, I've opened PR #301 to see if we can find any remaining issues and fix them in one place, assuming it's still worth the trouble, rather than continuing to go back and forth.

Thanks very much for being so helpful with this, by the way!