[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 Critical and tagging data loss.

Second critical GHS bug in a week: #3335991: [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).

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 😅

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;
      });
    }
  

  — source

• 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 #3274028: CKEditor 5 compatibility 😊 If that weren't the case, I'd have tagged this Contributed project blocker.

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 #3349893: [upstream] [GHS] CKEditor 5 removes <a>s that wrap HTML elements not natively supported by CKEditor 5 and credited you 👍 Discussing with the CKE5 team later today.

Closed #3348139: CKEditor 5 not supporting fa fa icon 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 #3370989: Update CKEditor 5 to 38.1.0, 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 #3377562: Update CKEditor 5 to 39.0.0.

Since markup like <i class="fab fa-drupal"></i> or <span class="icon my-icon"></span> can only be created through the SourceEditing CKEditor 5 plugin, I added the necessary test coverage there.

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 was merged into 11.x and hopefully soon 10.1.x

I'm experiencing this issue with 'ul' tag <ul id="divisionitemlist" class="text0">&#8203;</ul> and related the issue here, any suggestions n fixing the issue much appreciated.

Is the update to ckeditor only supposed to address FontAwesome and/or icon elements? I'm still having trouble adding empty inline elements. From the comments above, it sounds like this is still a bug?

Steps to reproduce:
-install Drupal Core 10.4.6 (which has CKEditor v44, according to core.libraries.yml)
-create node; body field using Full HTML text format
-click "Source" to edit source HTML
-add <span class="foobar"></span>
-toggle the Source button back and forth
-span element has now disappeared