Problem/Motivation

When modifying "Manually editable HTML tags", there is an error message said

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

The supported attribute's name is missing from the error message.

An expected error message is supported to provide the specific attribute name that is overlapped with plugins. Such as,

The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Alignment (<p class="text-align-center">).

Steps to reproduce

  1. Login in as an admin user.
  2. Create a text format using CKEditor 5 with Source editing plugin and Text alignment plugin.
  3. In the 'Manually editable HTML tags' text box, add the following tags and attributes.
    <p class="text-align-center">
    
  4. Save the text format without any changes.
  5. The error occured.

Proposed resolution

I believe this bug is caused by Drupal\ckeditor5\HTMLRestrictions::applyOperation() function.

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi...

As the function description suggested, it is supposed to support wildcard within an element name. Therefore, the wildcard $text-container should be resolved into a concrete tag, so that the Alignment plugin should be chosen in the error message, for example. But actually the result is opposite.
Actually this function doesn't resolve the wildcard tag for some plugins, such as the Alignment plugin.

At line 1143, the comment inside the function resolveWildcards() suggest that,
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi...

// Do not resolve to all tags supported by the wildcard tag, but only
// those which are explicitly supported. Because wildcard tags only
// allow declaring support for additional attributes and attribute
// values on already supported tags.

But the problem is that, the Alignment plugin only has a wildcard tag of '$text-container' , nothing else. So this logic won't work for this plugin or any other plugin which only has wildcard tags.

So if the logic is changed to

// Do not resolve to all tags supported by the wildcard tag, but only
// those which are explicitly supported by this set of restrictions
// or specified in the $supported_wildcard_tags array.
// Because wildcard tags only
// allow declaring support for additional attributes and attribute
// values on already supported tags.

          if (isset($r->elements[$wildcard_tag]) || isset($supported_wildcard_tags[$wildcard_tag])) {
            $naively_resolved_wildcard_elements[$wildcard_tag] = $tag_config;
          }

It will work for all plugins.

Remaining tasks

  1. PHPUnit test to reproduce this issue.
  2. Merge requests to fix this issue.

User interface changes

Before:
Before

After:
After

API changes

TBD

Data model changes

TBD

Release notes snippet

TBD

Issue fork drupal-3445375

Command icon 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

Mingsong created an issue. See original summary.

mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes

mingsong’s picture

mingsong’s picture

Issue summary: View changes
mingsong’s picture

It seems that something related to <$text-container at line 455 in ckeditor5.ckeditor5.yml file.

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi...

The $text-container variable seems wan't replaced with the actual tag name, which is <p> tag in this case.

mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes
mingsong’s picture

Issue summary: View changes
mingsong’s picture

Version: 10.2.x-dev » 11.0.x-dev
mingsong’s picture

Another example is

<h2 class="text-align-center">

which trigger another error message:

The following attribute(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these attributes: .

In which, the plugin name is missing from the message.

mingsong’s picture

I believe this bug is caused by Drupal\ckeditor5\HTMLRestrictions::applyOperation() function.

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi...

As the function description suggested, it is supposed to support wildcard within an element name. Therefore, the wildcard $text-container should be resolved into a concrete tag, so that the Alignment plugin should be chosen in the error message, for example. But actually the result is opposite.
Actually this function doesn't resolve the wildcard tag for some plugins, such as the Alignment plugin.

At line 1143, the comment inside the function resolveWildcards() suggest that,
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/ckedi...

// Do not resolve to all tags supported by the wildcard tag, but only
// those which are explicitly supported. Because wildcard tags only
// allow declaring support for additional attributes and attribute
// values on already supported tags.

But the problem is that, the Alignment plugin only has a wildcard tag of '$text-container' , nothing else. So this logic won't work for this plugin or any other plugin which only has wildcard tags.

So if the logic is changed to

// Do not resolve to all tags supported by the wildcard tag, but only
// those which are explicitly supported by this set of restrictions
// or specified in the $supported_wildcard_tags array.
// Because wildcard tags only
// allow declaring support for additional attributes and attribute
// values on already supported tags.

          if (isset($r->elements[$wildcard_tag]) || isset($supported_wildcard_tags[$wildcard_tag])) {
            $naively_resolved_wildcard_elements[$wildcard_tag] = $tag_config;
          }

It will work for all plugins.

mingsong’s picture

Status: Active » Needs review

10.2 branch is ready for review.

mingsong’s picture

Merge requests are ready for review.

mingsong’s picture

Issue summary: View changes
lmoeni’s picture

I ran into this problem recently in 10.1/10.2 while editing my text format. I tested the 10.2 patch with 10.2.6 and it works perfectly.
Thanks!

thatguy’s picture

Tested the MR 7924, works well and fixes the issue

smustgrave’s picture

Status: Needs review » Needs work

Can the issue summary be completed please.

Proposed solution needs to be filled in

if a UI issue before/after screenshots should be included

Thanks!

mingsong’s picture

Issue summary: View changes
StatusFileSize
new238.4 KB
new225.24 KB
mingsong’s picture

Status: Needs work » Needs review

Thanks @Stephen.

The summary has been updated as required.

smustgrave changed the visibility of the branch 11.x to hidden.

smustgrave’s picture

Status: Needs review » Needs work

Thanks for updating issue summary, updates look good.

Left some comments on the MR but a lot of the comment updates, period additions, etc seem to be out of scope of this issue. Code sniffer wasn't failing before without these changes

Test coverage is great.

mingsong’s picture

Status: Needs work » Needs review

Thanks @Stehpen for the review.

I correct those out of scope comments.

Regarding other changes, I believe they are necessary as they are part of the fix for this bug.

It is ready for review again.

smustgrave’s picture

Status: Needs review » Needs work

Only posted a few threads but believe there are still a dozen or so out of scope changes adding comments, punctuation, etc. Would say that could be a follow up issues (maybe novice maybe not).

mingsong’s picture

Without those changes, those PHPUnit tests would fail.

quietone’s picture

Version: 11.0.x-dev » 11.x-dev

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

redneko’s picture

Hi, I am experiencing this issue. It would be great if it could be made release ready.

Until I applied this as a patch it was impossible for me to complete some of my work, because without the information missing from the error message I couldn't progress

robloach’s picture

Attached is the patch against Drupal 10.4.x, with the non-consequential parts removed to avoid the conflicts.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

chamilsanjeewa’s picture

The attached patch targets Drupal 11.3.6. Unchanged context has been stripped out to prevent apply conflicts.

s_leu made their first commit to this issue’s fork.

s_leu’s picture

Status: Needs work » Needs review

I re-rolled the MR on the latest changes in main and also pushed a new branch for back-porting or patching this into 11.4.x.

Also tried to address some more feedback and reduce the changes to only the required ones.