Repository navigation
2.3.0 breaks PhantomJS 1.x, IE 7 & 8 #293
Description
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.)
Activity
mcollina commentedon Jun 21, 2017
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 commentedon Jun 21, 2017
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 commentedon Jun 21, 2017
opened a pull on safe-buffer
ScottFreeCode commentedon Jun 21, 2017
Wow, y'all are fast! Thanks!!
ScottFreeCode commentedon Jun 22, 2017
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 thebindcall works precisely to look out for additional issues like this, but at this point I'm not sure I could be much help, asdefinePropertycan be a lot less trivial to replace thanbind-- 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 commentedon Jun 22, 2017
@ScottFreeCode are you sure it was working on IE 7 & 8 before readable-stream 2.3?
ScottFreeCode commentedon Jun 23, 2017
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!