[upstream] [GHS] CKEditor 5 removes empty inline elements

Very interesting! Thanks for doing this research! 🙏👏

CKEditor 5 should never strip explicitly allowed markup, not even more obscure markup like this. Bumping to and tagging .

Second critical GHS bug in a week: 🐛 [upstream] [GHS] CKEditor 5 does not retain custom HTML tags that are not defined by CKEditor 5 plugins whenever /.*/ is not allowed (e.g. when filter_html is enabled) Postponed .

But … frankly, the https://www.drupal.org/project/fontawesome → module should provide a filter instead. Because markup like

<fontawesome>drupal</fontawesome>

which the corresponding filter plugin then transforms to

<i class="fab fa-drupal"></i>

would be far clearer, and it'd make it much easier to do transformations on existing content, e.g. to update icons 😅

Wim Leers → credited Reinmar → .

Discussed this with @Reinmar from CKEditor 5!

Observations:

• This also doesn't work in CKEditor 4 without a custom plugin — I tried: Full HTML + CKEditor 4 + <p><i class="fab fa-drupal"></i> asdflksjdflaskdfj</p> pasted ⇒ <i class="fab fa-drupal"></i> gets stripped!
• The only reason it works for you in CKEditor 4 is because you have a custom plugin, and one that sets
  

  
    // Allow empty tags in the CKEditor since Font Awesome requires them.
    if ('editor' in drupalSettings && 'fontawesome' in drupalSettings.editor) {
      $.each(drupalSettings.editor.fontawesome.allowedEmptyTags, (_, tag) => {
        CKEDITOR.dtd.$removeEmpty[tag] = 0;
      });
    }
  

• In CKEditor 5 you also need a custom plugin to do this.

That being said, this should work. They confirmed this, and they are planning to work on this in the next few months! 👍 Just know that in the mean time, you could work around it by adding explicit support for this custom tag in a CKEditor 5 plugin in 📌 CKEditor 5 compatibility Needs review 😊 If that weren't the case, I'd have tagged this .

Wow, thanks for the update, Wim!! 🤗

But I just double-checked that this wasn't an issue in Drupal 9 CKE 4. So I've quickly set up a D 9.4, logged in, and created an article, putting <span></span> My span in the editor, after clicking "Source", and saved. Visiting the article, inspecting the markup and saw that the empty tag persisted.

BUT as soon as I went to edit the article again, clicking "Source" again, the empty <span></span> was gone. 🤯

mkdir my-drupal9-site && \
cd my-drupal9-site && \
ddev config --project-type=drupal9 --docroot=web --create-docroot && \
ddev start && \
yes | ddev composer create "drupal/recommended-project:9.4" && \
ddev composer require drush/drush && \
ddev drush -y site:install --account-name=admin --account-pass=admin && \
ddev drush uli | xargs open

To address the "filter" idea in #5 - this markup is not custom to the Fontawesome module, but rather the actual implementation of Fontawesome itself (see https://fontawesome.com/docs/web/setup/get-started). Changing the way the tags are added prevents folks from using any of the Font Awesome documentation or writing the code themselves site-agnostic.

Having the same problem here for something other than FontAwesome.
Please allow whatever we want to put in the HTML source, including unkwnow or empty tag, when we use full HTML.

#8: So … that does mean that plain CKEditor 4 exhibited the same behavior, right? That's why it also only works in CKEditor 4 with that custom plugin installed. The problem is that no equivalent API exists in CKEditor 5, so even writing that plugin is impossible today.

#9 fair point!

#10: that's definitely always been the goal.

@Wim Leers we are having the same issue with an a tag wrapped around a custom picture block:

CKeditor source view:

<div class="adblock">
  <a href="/link">
    <picture>
      <source media>
    </picture>
  </a>
</div>

CKeditor default view:

<div class="adblock">
  <picture>
    <source media="">
  </picture>
</div>

And indeed dataloss. We migrated to CKeditor 5 and just notcied all links were stripped from our adblocks.

#12: wow, that's a whole new kind of data loss, separate from this one! Created new issue at 🐛 [upstream] [GHS] CKEditor 5 removes s that wrap HTML elements not natively supported by CKEditor 5 Fixed and credited you 👍 Discussing with the CKE5 team later today.

Wim Leers → credited cilefen → .

Closed 💬 CKEditor 5 not supporting fa fa icon Closed: duplicate as a duplicate.

@Wim Leers thanks so much! I responded in the other issue as well along with our workaround/solution.

Confirming this issue still exists in Drupal 10.1.0

We tried:
<a href="#" class="button"><i class="fa fa-drupal"></i></a>
which is being erased, while
<a href="#" class="button"><i class="fa fa-drupal">XXXXX</i></a>
is kept.

https://github.com/ckeditor/ckeditor5/issues/9888 has not yet been fixed upstream, so it's also not in 📌 Update CKEditor 5 to 38.1.0 Fixed , so 10.1.1 won't fix it either.

I was preparing one of our sites to migrate to Drupal 10 but it looks like the issue is also there with Drupal 9.5.10 and CKEditor 5 enabled see attached screenshots.

https://github.com/ckeditor/ckeditor5/issues/9888 landed upstream 1 hour ago!

Better yet: so far, that upstream issue is looking likely to ship in a 38.x.0 minor release — meaning that we could update Drupal 10.1.x to a version that includes the fix! 😊

No guarantees though.

We need test coverage to prove this is broken today and will be fixed in 📌 Update CKEditor 5 to 39.0.0 Fixed .

The last submitted patch, 23: 3337298-23.patch, failed testing. View results →

Wim Leers → credited witeksocha → .

I discussed #23 in detail with @Reinmar and @witeksocha.

Conclusion:

1. For unrestricted text formats, #23 MUST NOT assume that all tags can have empty inline elements.
2. Instead, even for unrestricted text formats, which elements ought to support empty elements should be explicitly configured.
3. This matches the behavior in CKEditor 4, where it could not even be configured through the UI, it required custom code — see #7.

So, adjusting the test coverage slightly for that: even for unrestricted text formats, we should expect

<p>Before <i class="fab fa-drupal"></i>

to be transformed to

<p>Before and after.</p>

Green test result for https://git.drupalcode.org/project/drupal/-/merge_requests/4530/diffs?co... 🚀

See #3377562-22: Update CKEditor 5 to 39.0.0 → .

Since https://www.drupal.org/project/drupal/issues/3377562 📌 Update CKEditor 5 to 39.0.0 Fixed was merged into 11.x and hopefully soon 10.1.x

Automatically closed - issue fixed for 2 weeks with no activity.