search

apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance
MIT License
3.84k stars 353 forks source link

Array/String.includes does not work in IE11 #400

Closed pvelez-bp3 closed 4 years ago

pvelez-bp3 commented 4 years ago

In latest versions of the library you added the .includes function in several parts of the code. And since this experimental feature is not supported by IE11, it breaks the app there. Then, if I try to add the Polyfill provided by MDN, it starts causing troubles in places where we loop arrays/strings using for...in, because it will iterate over the added "includes" option to the array prototype.

TrySound commented 4 years ago

And since this experimental feature is not supported by IE11

Array/String.includes is not experimental feature. It's part of standard for long time. But IE11 is legacy here.

Then, if I try to add the Polyfill provided by MDN, it starts causing troubles in places where we loop arrays/strings using for...in

But array has a lot of other methods even in IE. How do you handle them? Wouldn't something be broken too if IE added support for new methods? hasOwnProperty guard should be always used in such cases.

boutell commented 4 years ago

Are you referring to the 2.0 beta versions of this library?

On Fri, Aug 14, 2020 at 8:34 AM Bogdan Chadkin notifications@github.com wrote:

And since this experimental feature is not supported by IE11

Array/String.includes is not experimental feature. It's part of standard for long time. But IE11 is legacy here.

Then, if I try to add the Polyfill provided by MDN, it starts causing troubles in places where we loop arrays/strings using for...in

But array has a lot of other methods even in IE. How do you handle them? Wouldn't something be broken too if IE added support for new methods? hasOwnProperty https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty guard should be always used in such cases.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/400#issuecomment-674052271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27PWD2VWCOOOGV3Z6JLSAUVLPANCNFSM4P7OFYXQ .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

pvelez-bp3 commented 4 years ago

Are you referring to the 2.0 beta versions of this library? … On Fri, Aug 14, 2020 at 8:34 AM Bogdan Chadkin @.***> wrote: And since this experimental feature is not supported by IE11 Array/String.includes is not experimental feature. It's part of standard for long time. But IE11 is legacy here. Then, if I try to add the Polyfill provided by MDN, it starts causing troubles in places where we loop arrays/strings using for...in But array has a lot of other methods even in IE. How do you handle them? Wouldn't something be broken too if IE added support for new methods? hasOwnProperty https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty guard should be always used in such cases. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#400 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27PWD2VWCOOOGV3Z6JLSAUVLPANCNFSM4P7OFYXQ . -- THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

We tried to upgrade to 1.27.3, when it started failing.

pvelez-bp3 commented 4 years ago

And since this experimental feature is not supported by IE11

Array/String.includes is not experimental feature. It's part of standard for long time. But IE11 is legacy here.

Then, if I try to add the Polyfill provided by MDN, it starts causing troubles in places where we loop arrays/strings using for...in

But array has a lot of other methods even in IE. How do you handle them? Wouldn't something be broken too if IE added support for new methods? hasOwnProperty guard should be always used in such cases.

You are right, .includes is no longer experimental, but for us, IE11 is a browser that, unfortunately for us, it still being used, so we have to support it. For the for... in we could handle ours, but there are some other libraries we include that started failing too after adding the polyfill.

abea commented 4 years ago

This was an easy fix since lodash is already in the 1.x version. You might want to upgrade to the 2.x version, though, and include it in your build to cover your specific needs.