micc83 / fontIconPicker

🌈 jQuery fontIconPicker v2 is a small (3.22kb gzipped) jQuery plugin which allows you to include a simple icon picker with search and pagination inside your administration forms.
http://micc83.github.io/fontIconPicker/
MIT License
266 stars 86 forks source link

Fix for in loops to guard against added prototype methods #36

Closed marmite22 closed 6 years ago

marmite22 commented 7 years ago

We use your plug in and this was causing an error in our codebase because someone had extended Arrays with a custom prototype :(

See http://www.jameswiseman.com/blog/2011/04/07/jslint-messages-the-body-of-a-for-in-should-be-wrapped-in-an-if-statement/

I'm afraid I don't know what method/build process you use to minify the JS so I've not updated the .min.js file.

swashata commented 7 years ago

I maybe a bit out of loop here, but isn't extending prototype is forbidden with jQuery? https://bugs.jquery.com/ticket/2721

marmite22 commented 7 years ago

I agree we definitely shouldn't be doing it! Someone has though somewhere in our code base - they will be beaten. I've just added some checks to guard against it and make fontIconPicker more robust.

swashata commented 7 years ago

But fip is a jQuery plugin which shouldn't check against any practice the jQuery explicitly tells not to do. Let's cc @micc83 and hear from him.

swashata commented 6 years ago

Closing issue, since the idea doesn't go well with the framework.