Allow arrays from different frames or contexts to be equal #767
Conversation
| // or `Array`s from different frames are. | ||
| var areArrays = className === '[object Array]'; | ||
| if (!areArrays) { | ||
| if (typeof a != 'object' || typeof b != 'object') { |
There was a problem hiding this comment.
This line appears to be a duplicate of line 126
|
Thanks for the PR. A couple issues:
For the test, it would be ideal if we could reproduce this issue without relying on Node. If this is not possible, you can add node-specific tests to the npmPackage directory. |
|
Thanks for taking a look! The two ways that I know to reproduce the issue are
More context: http://perfectionkills.com/instanceof-considered-harmful-or-how-to-write-a-robust-isarray/ I think my preference would be to add tests to |
|
That sounds good. I like having the specs be closer to where you would expect to find them for equality. |
|
OK - I've added tests for both the iframes case and the vm case. The iframes case requires appending an element to the DOM, which I clean up at the end of the test - but let me know if you don't like this. EDIT: as Travis shows, the iframes causes an issue on PhantomJS - preferences? I don't see the use case for having different iframes inside a Jasmine suite, so I'd be tempted to just remove those two tests. |
| var frame = document.createElement("iframe"); | ||
| document.body.appendChild(frame); | ||
| xObject = window.frames[window.frames.length-1].Object; | ||
| var obj = new Object(); |
There was a problem hiding this comment.
I think this should say new xObject() instead of just Object
|
Given that the fix is the same, let's just go with the |
|
OK, removed the iframe based tests. |
No description provided.