IE8: prototyping array results in problems elsewhere

[braaam]

The IE8 only array prototype addition causes all arrays to have a new member in IE8, the indexof function.

When iterating through arrays elsewhere in my javascript applications, it can break, because the code never expects there to be properties that aren't the array's own properties. There would need to be a manual hasOwnProperty check everywhere to fix this.

I think interact.js shouldn't do these kind of manupilations to native types, even if it's just to make things in IE8 work. Better drop IE8 compatibility if it's hard to support it IMO.

I fixed it by calling a local indexof function and replacting all calls to .indexof to use this local one. Not a great solution as well, but at least it's predictable. I could do a pull request if you're ok with using a none native function for all browsers.

[taye]

Using a very simple non native indexOf would be fine. The way the method is used internally, it could just be a straight forward for loop + comparison. A pull request would be more than welcome.

I'll try later to also avoid changing prototypes for Element#matchesSelector, Event#preventDefault and Event#stopPropagation.

[taye]

I have some free time now so I think I'll redo all of those methods myself if you haven't already started.

[taye]

I've made the changes in a new branch. Could you try this version to check if the issue you were having is fixed: https://rawgit.com/taye/interact.js/dont-modify-native-prototypes/interact.js?

Edit: corrected the URL

[braaam]

Hi,

Thanks for the fix. Sorry for slow response, gonna test it this afternoon.

[braaam]

There is still a problem when a string is fed to the isElement function. I've changed the first comparison which fixes it.

    function isElement (o) {
        return (typeof o === 'object') && (
            typeof Element === "object" ? o instanceof Element : //DOM2
            o && typeof o === "object" && o !== null && o.nodeType === 1 && typeof o.nodeName==="string"
        );
    }

[taye]

I've added your change. Can you confirm that it works correctly? https://rawgit.com/taye/interact.js/dont-modify-native-prototypes/interact.js

[braaam]

Yes, it's fixed, thanks!

[taye]

Awesome. Thanks for reporting the issue.