CKEditor breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified

Hello,

The problem also occurs in version 10.1.6 and without adding any tags to the "Manually editable HTML tags" setting of the plugin.

Just by adding the "Source" button the text formatting interface is damaged.

Wim Leers → credited Adrianm6254 → .

Wim Leers → credited ericgsmith → .

Wim Leers → credited joshuami → .

Wim Leers → credited Luke.Leber → .

Merging 🐛 Change to edit source behavior in 10.2 leads to data loss on edit Active and 🐛 Several elements are missing from ckeditor5 Basic HTML Active as duplicates of this, since this was the first issue created for this regression.

Already crediting all contributors of those issues here, next up: transfering relevant info.

Added another related possibly duplicate issue

Interesting! #3410303 points to 🐛 Upgrade filter system to HTML5 Fixed as the culprit.

But the other issues pointed at 🐛 Fix → native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by Fixed !

Now this definitely needs a deeper investigation.

@Wim Leers you asked for a couple of files from my other issue, so I'll copy them here. Hope this helps. I'll also include what my Style Plugin & Source Editing settings are showing on the old site (D10.1.5.) and the new site (D10.2.0).

Source Editing:

Original (D10.1.5)
    <cite> <span> <dl class> <dt class> <dd class> <iframe allow allowfullscreen src width height frameborder> <blockquote cite> <ul type> <ol type> <h2 id class> <h3 id class> <h4 id class> <h5 id class> <h6 id class> <a hreflang accesskey id rel target title class> <img data-entity-substitution class> <p class>
    
Current (D10.2.0)
    <cite> <span> <dl class> <dt class> <dd class> <iframe allow allowfullscreen src width height frameborder>
    
Style Plugins:

Original (D10.1.5)
    a.button|Button
    p.note|Note
    p.large|Larger text
    h1.node__title|Page title
    img.border|Add a border
    img.display-left|Float left
    img.display-right|Float right
    p.h2|H2
    p.h3|H3
    p.h4|H4

Current (D10.2.0)
    a.button|Button
    p.note|Note
    p.large|Larger text
    h1.node__title|Page title
    img.border|Add a border
    img.display-left|Float left
    img.display-right|Float right
    p.h2|H2
    p.h3|H3
    p.h4|H4

@Wim Leers, I think #3396628 may be a separate issue.

What I was experiencing was definitely related to the changes in SourceEditingRedundantTagsConstraintValidator.php as part of #3396628. At https://git.drupalcode.org/project/drupal/-/commit/ec24a58c6dd7e7d3a1ead..., the validation is comparing elements enabled in source editing to all other enabled plugins. That check prevents someone from adding an attribute like class to anything that might be covered by another plugin that also enables aspects of that element.

The error message I was getting that prevented saving a valid config is The following @element_type(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: %overlapping_tags.

Here's an example:

We had a couple of CKEditor 4 plugins that we wrote to support adding phone numbers and email addresses with an enforced format and classes that allowed them to be editable and styled. That code required the following to be enabled in source editing: <span class="cke-phone cke-email">. (Note: this is a temporary need to avoid data loss until we can write a proper CKEditor 5 solution.

When we attempt to save the text format after the 10.2 upgrade, we get the error message:

The following attribute(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Style (<span class="cke-phone cke-email">).

That is directly coming from $enabled_plugin_elements_optional, but the only other plugin enabled that affects span is the Language selector. Clearly, we are not going to get the ability to save those classes from a completely unrelated plugin, but the overlap filter is preventing span from being in source editing.

I propose we remove the overlap optional filtering so that source editing can truly allow the developer to say "I know I don't have another plugin for this so I want to allow it in source just to prevent data loss until I can write something way more complex and robust (possibly never 😆)".

To put this another way, source editing should allow us to define exceptions that are not covered by another plugin rather than forcing us to write a plugin from scratch.

@joshuami I’m facing exactly the same issue. I managed to workaround some missing classes and prevent data loss by adding some styles to the Styles plugin ie span.my-class. But that doesn’t cover all cases, for instance the

, where it complains with the same constraint validation you’ve described.

Coming here from 🐛 Several elements are missing from ckeditor5 Basic HTML Active .

My site's content is missing basic HTML tags after updating to 10.2.0 -- most noticeably bold and links. Turning off the Limit HTML Tags filter restored the missing tags but as others have noted, that is not a good long-term solution.

When I turned the Limit HTML Tags filter back on again, I was presented with the following error message:

The current CKEditor 5 build requires the following elements and attributes:
<br> <p> <h2> <h3> <h4> <h5> <h6> <* dir="ltr rtl" lang> <cite> <dl> <dt> <dd> <span> <figure> <figcaption> <iframe src height width allowfullscreen frameborder> <strong> <em> <code> <blockquote> <a href data-entity-type data-entity-uuid data-entity-substitution> <ul> <ol reversed start> <li> <img src alt height width data-entity-uuid data-entity-type data-caption data-align> <drupal-media data-entity-type data-entity-uuid alt data-caption data-align>
The following elements are missing:
<strong> <em> <code> <blockquote> <a href data-entity-type data-entity-uuid data-entity-substitution> <ul> <ol reversed start> <li> <img src alt height width data-entity-uuid data-entity-type data-caption data-align> <drupal-media data-entity-type data-entity-uuid alt data-caption data-align>

Of course, the field where I would add those elements back in is read-only...

With CKEditor 5 this is a read-only field. The allowed HTML tags and attributes are determined by the CKEditor 5 configuration. Manually removing tags would break enabled functionality, and any manually added tags would be removed by CKEditor 5 on render.

I do have source editing enabled (as well as the CodeMirror plugin), and everything worked without issue prior to updating core.

In further tests, it seems this only happens if one defines HTML tags that already exist or variations of them with custom attributes or attribute values allowed. If a tag added here is a tag that is not added by any other plugin, it seems to work fine.

Might this be a duplicate of 🐛 FilterHtml data loss when iframe and/or textarea is allowed Active ? I noticed both Adrianm6254 and jesss have an iframe reference in their respective configs.

Could be, I am also using iframe as the HTML tag allowed, alongside some div tags with custom classes

From what I'm seeing on my site, there is no actual data loss. It's simply that when rendering the field, the text filter is stripping out tags that should be allowed. If you go to edit the content, all those "missing" tags are still there.

That issue is fixed in 10.2.1 (released today) so if you can still reproduce this after updating then it must be a separate issue.

Ok I have used a wrong term. I mean data loss on render, not in the database. Sorry for the confusion.

I will spin up local instance of our system and try it out and report back

#3410303: FilterHtml data loss when iframe and/or textarea is allowed indeed fixes this issue

@KlemenDev, this is not a duplicate and the issue summary is not fixed by 3410303 🐛 FilterHtml data loss when iframe and/or textarea is allowed Active . The problem is that you can't save a valid configuration for source editing after 10.2.

Unless we want to reopen issue #3396628 which I mention in comment 12 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review , we still have a validation issue which forces resolution of errors on the text format creating a state that will cause data loss upon editing existing content.

For any element that has a valid plugin, the current validations requires you remove the element and its classes from source editing. This is a problem, because not every plugin covers every attribute that may be needed in an existing site.

This is currently a hard blocker for upgrading a site from CKEditor 4, which allowed pretty much any source editing pattern that you desired, to CKEditor 5, which now requires new plugin for every possible pattern in source editing.

Hello @joshuami, I have the same idea.
I still haven't seen clearly which tags the problem is most evident in, but as above refered I have <iframe> and other element types for instance <ul class>.
For now, in the concerning project I've detected the same issue, since we already use ckeditor5, I've implemented a hook_config_schema_info_alter() to temporarly remove the constraint mentioned on #12 and #13, starting from https://git.drupalcode.org/project/drupal/-/commit/ec24a58c6dd7e7d3a1ead.... But again, it's far from ideal.

I think that the solution you indicated in #12, in the final part, could work if it is fine-tuned.
For example, in my case, this constraint would make sense to be checked right after the CKEditor5Plugin Style, but before other plugins that enable the same tag, with a different but unspecified attribute (like class="").

I am encountering this error on 10.2.1, or at least a closely related one. I tried to remove allowing dl, dt, and dd tags after the 10.2.1 update, and I get this error:

The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: .

So it's not citing any plugins that are already supported and should not be added, but still throwing that error anyway.

I've encountered a problem in 10.2.1 whereby and tags are always deleted by the editor but are present when viewing the node before an edit. My problem is fixed by disabling source editing.

Hi @Phil Stringer, indeed, should be related with SourceEditingRedundantTagsConstraintValidator.php as mentioned by #12.

To add to the cases:

We have these tags in our output (from D6 migration) and we want to preserve allowing them but they can’t be added to the Allowed Tags anymore (I'm ok with that actually, as long as I can find a way to add them back in somewhere) and are not accepted in the Source Code allowed tags because other buttons define them

<iframe src allowfullscreen width height frameborder>
<div class>
<ul type class>
<ol type class>
<img title class>
<table class>

We worked around this with a creative MODULE.ckeditor5.yml plugin definitions

MODULE_ckeditor5_allowHtml:
  ckeditor5:
    plugins: [htmlSupport.GeneralHtmlSupport]
    config:
      htmlSupport:
        allow:
          - name: table
            classes: true
          - name: ol
            classes: true
            attributes:
              - key: type
            ...ETC...
  drupal:
    label: Arbitrary HTML support
    elements: 
      - <table class>
      - <ol class type>
            ...ETC...
    conditions: [ckeditor5_table]

But that is because there were buttons and added iframe support with a module for that (but regretting that already)

The following attribute(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Style (

While I don't recommend this module (read the project page near the end to see what this means) I am putting it here to show that it's a problem that people are trying to solve.
https://www.drupal.org/project/ckeditor5_allowed_html →

Updating the issue title to indicate that this was not introduced in 10.2, but rather 10.1.6.

@mark_fullmer, can you point to the commit that introduced the bug for you? I was fairly certain it was introduced in 3396628 🐛 Fix → native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by Fixed , which added the validation that compared source editing to the currently enabled plugins—among some other changes related to lists.

I'm not able to reproduce the error in a site running 10.1.6 and CKEditor 5.

🐛 Change to edit source behavior in 10.2 leads to data loss on edit Active was closed as a duplicate / merged into this issue, but I don't think the issue summary currently accurately describes to the same level of detail exactly what the issue is that multiple users are still reporting in this comments.

The original author of this issue closed it as resolved by 🐛 FilterHtml data loss when iframe and/or textarea is allowed Active so I think we potentially have a lot of confusion in this issue, potentially 2 unrelated issues.

Are there any objections to updating the issue summary to more clearly highlight what is reported in comment 12 onwards? That isto highlight that the issue is to do with the cha gw in validation logic?

The alternative would be reopening the issue that was marked as duplicate, but there are a lot of comments in this issue that seem to being relating to that issue.

I also agree with above comment that this is a 10.2 regression and I'm not sure why the title was changed.

Yep, y'all are right. I tested again and this was not present in 10.1.6. Changing the title back.

For those needing to work around this issue here is a patch to revert the change that tightened the validation from 🐛 Fix → native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by Fixed as noted comment #34 by @joshuami

Tests would fail as this doesn't revert the additional tests related to the ol start functionality which introduced this change.

Not expecting a review so leaving as active - this is just if people need to revert to the behaviour of Drupal 10.1.

+1

Comment #35 is exactly correct. There are two issues at play, there was a data loss issue that was fixed in https://www.drupal.org/project/drupal/issues/3410303 🐛 FilterHtml data loss when iframe and/or textarea is allowed Active , but there still exists a separate issue (fixed/reverted by the patch in comment #38), which is that saving a valid CKEditor "Source editing" configuration in the admin interface will cause a validation error. You can still technically edit it through drush, but it's not ideal.

Patch 38 appears to work

I was running into the issue mentioned in #12, and the patch in #38 fixes it for now.

Patch #38 works for me. I don't like patching core, but is that really the answer for now?

I created a Google Doc to explain exactly what was happening with me. If it's OK I link to it here:

https://docs.google.com/document/d/1H3p6rH-oi0kSfeqc79b-BbE6wLLOf83r8Eju...

If that is not OK I will put it all here, but it's a lot.

For those looking for an intermediate solution, instead of a rollback of the aforementioned commit, with a custom implementation as mentioned above:

/**
* Implements hook_config_schema_info_alter().
*/
function MODULE_config_schema_info_alter(&$definitions) {
if (isset($definitions['ckeditor5.plugin.ckeditor5_sourceEditing'])) {
unset($definitions['ckeditor5.plugin.ckeditor5_sourceEditing']['mapping']['allowed_tags']['sequence']['constraints']['SourceEditingRedundantTags']);
}
}

It seems we have consensus since I asked in #35 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review that some of the original issue details were related to the new resolved 🐛 FilterHtml data loss when iframe and/or textarea is allowed Active so I have update to include some of the use cases mentioned here and make it clearer (hopefully) what the recent comments have been.

I have added the propose resolution from one of the closed duplicates - while I'm in favor of reverting the validation, there hasn't been much discussion on making the validation "opt out" or perhaps just smarter about what is being prevented.

Adding another potential use case - https://www.drupal.org/project/drupal/issues/3274635#comment-15457719 ✨ [upstream] Use CKEditor 5's native and UX Needs work

I've updated the issue's steps to reproduce; previously, they did not capture all of the elements required to trigger this scenario. Specifically, the CKEditor 5 toolbar needs to be configured to use both the "Source" and the "Styles" plugins, and at least one style needs to be defined, and finally, one manually editable tag attribute needs to be added for an element that is already allowed by the CKEditor toolbar.

My take: if this is the new expectation of how the "Limit allowed HTML tags and correct faulty HTML" filter and the "Manually editable HTML tags" should interact, as of Drupal 10.2, then I think there are three potential resolutions:

1. Drupal core provides a setting to opt out of this locked behavior to accommodate the use cases mentioned in the issue description.
2. Sites with those use cases apply the equivalent of the alter hook in #45 to opt out themselves.
3. Sites use a different HTML limiter, such as https://www.drupal.org/project/htmlpurifier → , in place of Drupal core's "Limit allowed HTML tags and correct faulty HTML" filter.

I confirm that the updated steps in the issue description reproduce the error.

Also confirming that the updated steps in the issue description reproduce the error.

I very much appreciated the summary from mark_fulmer in #53 I am going with option 2 (#45) -- more testing to do but so far that seems to work great.

Thanks @Mark Fullmer for the suggested solutions in #53.

I would vote for option 1. For a better user experience, in the error message where said

The following @element_type(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: %overlapping_tags.'

In that message, If we could provide a link to where an admin user who has the permission to ignore the constraint conflicts without custom coding/hook required, that would give a better user experience.

Apart from the proposed resolutions from #53, another one could be:
4. Accept the wildcard in the class attribute name, such as 'class*', for example, in the 'Manually editable HTML tags' box, if you specify '' then all classes for an A tag will be allowed.

The MR 7887 introduces functionality to support wildcard characters '*' in the class attribute names within the 'Manually editable HTML tags' setting.

If you the patch to do a local test, you can get the patch from

https://git.drupalcode.org/project/drupal/-/merge_requests/7887.patch

This MR is aiming for 11.x branch.

Mingsong → changed the visibility of the branch 3410100-10.2-regression-ckeditor to hidden.

Mingsong → changed the visibility of the branch 10.2.x to hidden.

MR 7888 is aiming for 10.2.x branch.

The patch for 10.2.x

https://git.drupalcode.org/project/drupal/-/merge_requests/7888.patch

Those two MRs are ready for review.

I recently updated to version 10.2.6 from 10.1.something and I'm getting the error but with whitespace where it should tell me what the offending attribute is, so I can't do anything to resolve it:
The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: . I don't know what's caused this, but it made me wish extra hard that I could switch off this annoying validation... therefore very thankful for #38, cheers for that!

Attribute name missing in the error message is another problem. More details see

https://www.drupal.org/project/drupal/issues/3445375 🐛 Supported attribute(s) not showing in CKEditor 5 Source Editing plugin error message Active

I'm having the same issue as in #65 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review . The patch in #63 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review did NOT solve the issue.

#65 is different issue from this one.

Agreed, setting back to needs review.

My apologies, I overlooked comment #68 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review

EthanT → closed merge request !8323

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Changed from version 11 to 10.2

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies. Also, 10.2 is in security mode now.

Just to note for those using ^10.3; #38 still seems to apply fine (10.3.1).

Confirmed that the changes in MR !7887 fix the problem described in the steps to reproduce in the issue summary but only if <blockquote class*> is used for the "Manually editable tags" value in step 6 (instead of <blockquote class>).

Why only class? This applies to any attribute a plugin is defining. If a plugin defines <div id> you now can no longer add ID's to any source edited divs.

I believe we either need an optout of the constraint as suggested in #53 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review or we need to re-evaluate why this constraint is needed in the first place.

Updating issue summary with the proposed options that have been discussed in the issue.

From what I can gather option 1 has the most support - I would like to get some form of feedback from ckeditor5 subsytem maintainers before progressing.

My client is having this issue.

Basically it is manifesting itself on the table element. Basically we have written a plugin for tables which allow us to add classes to the table element. This worked perfectly under D10.1, but then broken when we migrated to 10.2. The problem is that when we edit content with a table element, all the classes are stripped from the table. So before we save any content we need to re-apply the styles again.

I have tested all the patches and none of them are working.

#75 didn't really do anything. I had to reroll it so it would work with 10.2.6, but didn't resolve the issue.
#38 Fixed the issue when configuring the source editing, but didn't solve the issue with editing content that has tables with classes. It did resolve the issue of fixing up the classes in the filter html

I also tried MR!7888 but this didn't do anything as well, but then again this MR doesn't really change much.

As workaround we defined the style tag on another core plugin:

ckeditor5_removeFormat:
  ckeditor5:
    plugins: [removeFormat.RemoveFormat]
  drupal:
    label: Remove Format
    library: core/ckeditor5.removeFormat
    admin_library: ckeditor5/internal.admin.removeFormat
    toolbar_items:
      removeFormat:
        label: Remove Format
    elements:
     - <span>

I am running into this, too, and have seen it for a long time now. If this isn't easily fixable, shouldn't core maintainers roll back the commit that caused this problem? This is a majorly critical issue because it makes it impossible to edit text formats, which is, needless to say, really important.

Here's another use case: We want to allow our editors to add "data-*" attributes to divs when in source editing mode. We use the uswds_ckeditor_integration module, which provides several editor plugins, of which we use several, but not the "USWDS Grid" plugin, which happens to specify "

Here's how I understand this issue. It seems that this problem is specific to the class attribute. It seems that the root cause to the problem is that ckeditor5_style defines <$any-html5-element class> as it's elements, which then triggers the validation if anyone tries to add a class attribute to a HTML5 element, because theoretically, they could add that using the style plugin. The plugin documents that the class is dynamically restricted to a subset of class attributes, but this is not taken into account in the validation.

Requiring the use of the style plugin might be problematic because it a) requires someone to know and specify every class that may be used and it b) exposes them through the user interface (which might not be always desired). Because of this, people are specifying class as an attribute in the source editor plugin.

If this is a somewhat correct understanding of the problem, it seems that an ideal solution would be to special case the class attribute on the style plugin somehow. This way we could allow adding class attribute to elements even though it's an attribute that's supported by the style plugin.

It seems that #59 is proposing something pretty close to this. However, it seems to be adding a new syntax for this. I'm not sure that we need a new syntax for this; in my mind we would just special case the class attribute to not trigger this validation.

@lauriii, while the patch in #59 is focused on class and the style widget, the text-format-save blocking error can occur for any attribute that is defined by a module and in source editing. It could be an id or a data-attribute if there is a module that defines those attributes for the element in question.

The validation introduced in 3396628 🐛 Fix → native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by Fixed triggers on any "overlap". While the class issue is probably the most common, it's not the only scenario that can cause that error to throw and prevent the text format from being saved.

morvaim → changed the visibility of the branch 10.4.x to hidden.

The validation is indeed triggered on any "overlap" but is that a problem? If an attribute or tag is already enabled by another plugin then that attribute or tag can be used in Source editing mode when editing a content. So it can make sense to not add it as a duplicate to the "Manually editable HTML tags" list if it is already allowed and can be used. Class attribute is an exception in this regard because although the Style plugin adds it in its definition as an allowed attribute, in reality it is not allowed but only those specific classes are allowed which are added to the "Styles" list. This is why it is not "valid" to make the validation fail when someone tries to add the class attribute to a tag in "Manually editable HTML tags" in my opinion.