- Issue created by @DamienMcKenna
- πΊπΈUnited States DamienMcKenna NH, USA
I suspect the problem is that the "Limit allowed HTML tags and correct faulty HTML" filter removes the HTML structure that the plugin creates.
I think this will be another scenario where core's "Limit allowed HTML tags and correct faulty HTML" filter always removes the "style" attribute, due to its security implications. The Extended HTML Filter β module aims to replace core's filter with one that allows the "style" attribute, but it has problems, see β¨ Match the filter_html <> ckeditor5 integration in Drupal core Needs work for details.
- πΊπΈUnited States DamienMcKenna NH, USA
FYI I suspect other plugins might also be affected by this.
- π΅π±Poland dolszewski
Hi @damienmckenna
I've added a patch for this issue. It will not fix the problem completely but the
tag with class should not be removed anymore (probably you have to unselect and select again the "Limit allowed HTML tags and correct faulty HTML" filter.
In terms of style attributes, we can't do anything with that, Drupal does not allow adding style attributes ( https://www.drupal.org/project/drupal/issues/3381471 β¨ CKEditor 5: Restricting the style attribute in tag has restricted some modules to work as expected. Closed: works as designed ).
Any suggestion on how to change the style attribute into something else in the ckeditor5 page break plugin should be added to the official ckeditor5 repo on GitHub. - πΊπΈUnited States DamienMcKenna NH, USA
I think the best way forward would be to put some time into Extended HTML Filter β and make it a required dependency for the submodules that require the "style" attribute.
- Status changed to Needs work
5 months ago 11:28am 26 July 2024 - πΊπΈUnited States DamienMcKenna NH, USA
I tested patch #4 but it doesn't retain the "class" attribute, something is removing it. I think the definition also needs an entry for the span tag.
- π¬π§United Kingdom scott_euser
Thanks @DamienMcKenna, followed your rabbit hole of related issues. At the moment its a house of cards of patches to get this to work it seems. Most of the patches actually do no harm getting in, and facilitate things working once other bits get merged. I think in summary:
- Allow div class and span attributes - ckeditor5_plugin_pack module - π¬ Page break plugin looses formatting when editing content Needs work - ie, this issue, your patch in #7. Ready to merge I think
- Fix validation constraint so that * actually does not complain about diffs - drupal core - β¨ Match the filter_html <> ckeditor5 integration in Drupal core Needs work comment #17 but has no actual core issue. Workaround is to save the config manually
- Make extended html module match drupal core - extended_html_filter - β¨ Match the filter_html <> ckeditor5 integration in Drupal core Needs work - Probably ready to merge there, but again requires workaround until (2) is fixed.
- <* style> is not accepted - drupal core - π FilterHtml accepts <*> but does not support it, resulting in inaccurate ::getHtmlRestrictions() return value Needs work - Not yet ready to merge but contributed to by multiple core maintainers, so we can be confident its going in the right direction at least and has a viable path to merge
So based on that, probably merging this one (1) and merging (3) do no harm and at least would prep these two modules for when core actually supports it, making it easier for people to use things like page breaks without all of the above steps. Do you agree? If so, I'll mark this as RTBC for ckeditor5_plugin_pack maintainers to review.
- πΊπΈUnited States DamienMcKenna NH, USA
Thanks for digging into it, Scott. Yes, this is definitely a layered problem and bugs in core are affecting it.
- Status changed to RTBC
5 months ago 10:15am 30 July 2024 - π¬π§United Kingdom scott_euser
Okay I suggest we merge this one given it does no harm and once other related issues get fixed and merged we will eventually have working page breaks :)
- πΊπΈUnited States DamienMcKenna NH, USA
This was partially added in the new 1.2.x branch, but it missed the SPAN tag change from #7.
Dear maintainers,
Please work with the issue queue instead of committing changes elsewhere and then pushing the repository to d.o. Thank you. - Merge request !10Added 'span' to 'elements' for the pagebreak ckeditor5 plugin β (Merged) created by DamienMcKenna
- First commit to issue fork.
- π΅π±Poland salmonek
The fix has been released in previous (1.2.2) release.
Thank you for the contribution.@damienmckenna
We're using company repositories for tickets requested by management and bugs reported by QA since people across organization have access there and are familiar with it. In this module we will operate more on Drupal repositories as the internally induced work is mostly done. Automatically closed - issue fixed for 2 weeks with no activity.