Problem/Motivation
When trying to save a CKEditor5 text format with the filter provided by this module enabled an error like this occurs:
The current CKEditor 5 build requires the following elements and attributes:
<br> <p> <h2> <h3> <h4> <h5> <h6> <strong> <em> <pre> <code class="language-*"> <s> <sub> <sup> <blockquote> <a href> <ul> <ol reversed start> <li> <hr> <table> <tr> <td rowspan colspan> <th rowspan colspan> <thead> <tbody> <tfoot> <caption> <img src alt height width data-entity-uuid data-entity-type data-caption data-align>
The following elements are missing:
<br> <p> <h2> <h3> <h4> <h5> <h6> <strong> <em> <pre> <code class="language-*"> <s> <sub> <sup> <blockquote> <a href> <ul> <ol reversed start> <li> <hr> <table> <tr> <td rowspan colspan> <th rowspan colspan> <thead> <tbody> <tfoot> <caption> <img src alt height width data-entity-uuid data-entity-type data-caption data-align>
Steps to reproduce
- Install a fresh Drupal 10 site with the standard profile
- Install this module
- Edit the "Full HTML" text format
- Tick "htmLawed HTML filter and purifier"
- Attempt to save
- You will see the error
This was mentioned on TypeError: Argument 1 passed to Drupal\ckeditor5\HTMLRestrictions::__construct() must be of the type array, object given but it was not solved by the fix from the linked core issue.
More details about why this is happening:
It seems like this is happening in Drupal\ckeditor5\Plugin\Validation\Constraint\FundamentalCompatibilityConstraintValidator::checkHtmlRestrictionsMatch().
Basically the filter is loaded here because it is of type FilterInterface::TYPE_HTML_RESTRICTOR:
$html_restrictor_filters = static::getFiltersInFormatOfType(
$text_editor->getFilterFormat(),
FilterInterface::TYPE_HTML_RESTRICTOR
);
however, since the filter doesn't actually override the getHTMLRestrictions() method it just returns false, which means that here:
$allowed = HTMLRestrictions::fromFilterPluginInstance($filter);
$diff_allowed = $allowed->diff($provided);
$diff_elements = $provided->diff($allowed);
$allowed is set to empty and this results in $diff_elements including all elements that the format provides. A validation violation is added here as a result:
if (!$diff_elements->allowsNothing()) {
$this->context->buildViolation($constraint->missingElementsMessage)
->setParameter('@list', implode(' ', $provided->toCKEditor5ElementsArray()))
->setParameter('@diff', implode(' ', $diff_elements->toCKEditor5ElementsArray()))
->atPath("filters.$filter_plugin_id")
->addViolation();
}
This causes the format configuration form to display the error when saving, likely because it thinks the filter forbids all elements from being saved.
Proposed resolution
As I see it there are two ways to fix this: change this filter's type (e.g. to Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE instead of Drupal\filter\Plugin\FilterInterface::TYPE_HTML_RESTRICTOR) or implement getHtmlRestrictions() so the filter plugin accurately reports the tags and attributes it supports. The latter is probably more "correct", but it seems to be quite difficult to do this in a way that will be compatible with other HTML restricting filters like filter_html.
So I'm going to attach an MR that switches the type instead. However, please consider that alternative before merging this in because I'm not sure that's the most appropriate solution.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3366282-error-message-the-current-ckeditor5-build.patch | 907 bytes | dylan donkersgoed |
Issue fork htmlawed-3366282
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
dylan donkersgoed commentedComment #4
dylan donkersgoed commentedHere's a patch.
Comment #5
dylan donkersgoed commentedComment #6
alpha2zee commentedThank you for looking into the issue to identify its cause and also note possible fixes along with their drawbacks.
htmLawed is functionally TYPE_HTML_RESTRICTOR, and while making it TYPE_TRANSFORM_IRREVERSIBLE will fix this CKEditor issue, I don't know what issues will arise from the change.
I think it is better to open an issue with CKEditor devs to change their code logic.
Comment #7
alpha2zee commentedAs noted elsewhere, the CKEditor5 issue doesn't occur in D10 but occurs in D9. It seems CKEditor5 devs are trying to get a fix for D9. I guess we just have to wait for that. For those who need an immediate fix, perhaps the suggested patch will work well.
Comment #8
dylan donkersgoed commentedThis issue does occur in D10. You can see it if you install a fresh D10 site with the standard profile and try to add this filter to the "Full HTML" format you run into the error.
While there are similarities with the linked issue (similar steps to reproduce, the error occurs in the same function) I don't think it's quite the same problem. This is a validation error when saving the format, not a fatal error. It occurs in D10 even though the fix is already applied in D10. And while I'm not 100% sure I don't think it requires other filters of the same type to be enabled.
I'm also not sure it's actually unintended behaviour in core. I'm not too familiar with the internals of core's CKEditor handling but it does kind of make sense to me that a plugin that declares it restricts HTML but then provides no information about what tags it restricts would encounter validation errors.
Comment #9
alpha2zee commentedThanks for correcting me. I was under the wrong impression, or may be I had tested a D10 version with a fix. The issue does occur in current and dev releases of D10. In my opinion, it is still primarily a core/CKEditor issue with handling of filter modules, as discussed elsewhere.
I cannot change the module filter type to something other than TYPE_HTML_RESTRICTOR.
As you noted, implementing getHTMLRestrictions() in the htmLawed module is difficult because htmLawed has a different filtering customization and may be a more complex filtering ability than Drupal/CKEditor. E.g., regex patterns to indicate permissibility of attribute values... correctly conveying this through getHTMLRestrictions() may not be possible.
Comment #10
alpha2zee commentedComment #11
frazacMaybe linked? https://www.drupal.org/project/twig/issues/3413999#comment-15604317