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.69k stars 351 forks source link

Not compatible with htmlparser2 >= 7 #541

Closed rolandmas closed 1 year ago

rolandmas commented 2 years ago

(As reported on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005867) Running the testsuite on Debian testing (which has version 7.2 of htmlparser2) fails. Could you adapt to sanitize-html to the current version of htmlparser2?

boutell commented 2 years ago

This is a bug in a debian package we didn't know existed 😄

sanitize-html itself, the npm package, depends explicitly on htmlparser2 6.x and works just fine.

I don't know how or why debian's wrapper of our npm package (which exists for a reason I'm not really clear on) ignores package.json and thus breaks the dependency, but this is an issue with the debian package.

OK that's a totally valid answer on my part... but, it is interesting that we're not on the latest htmlparser2. Yes we should take a look at upgrading. That's something that could also be done in a community PR, checking of course that the tests still pass.

However it might require a major version bump of sanitize-html because we expose parts of the htmlparser2 API directly, so if those have changed it would be a backwards incompatibility break.

On Mon, Feb 21, 2022 at 2:59 PM rolandmas @.***> wrote:

(As reported on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005867) Running the testsuite on Debian testing (which has version 7.2 of htmlparser2) fails. Could you adapt to sanitize-html to the current version of htmlparser2?

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/541, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27I6THAX2VPLXUFNTULU4KKR7ANCNFSM5O7PI2CA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

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

stianjensen commented 1 year ago

I guess this will be fixed now, with a new release

boutell commented 1 year ago

Ah yes closing, thanks.