search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
720 stars 820 forks source link

Fix wysiwyg sanitisation #11201

Closed GuySartorelli closed 2 months ago

GuySartorelli commented 2 months ago

Description

There are multiple commits here since we're doing several related but separate changes:

  1. Reimplement the original bugfix (just cherry-picked the commit directly)
  2. Refactoring a sub-optimal test to use dataproviders and to test valid_elements and extended_valid_elements separately
  3. Don't skip validation of HTML - without this, an empty valid_elements set would result in skipping validation entirely.
  4. Set the default valid_elements settings for any new TinyMCE config instances
  5. Wasn't in the ACs but seems prudent - make the default TinyMCE settings configurable at a global level.

Issues

GuySartorelli commented 2 months ago

Looks like you've uncovered a completely new bug 🥳

To test, in a fresh installation without any of these PRs and without your configuration above, add a link to a page and save. Then, add only the second config you have there (i.e. add $h2 and $c2, leaving the original "Content" field as it was) and refresh the page. TinyMCE will have stripped the link out of your original content.

If you can reproduce that with a fresh installation, it's a separate bug and should be handled in a separate card (and probably fixed in 5.2)

emteknetnz commented 2 months ago

Yup validated it's an existing issue, have raised card