Problem/Motivation

Reported by @glynster at #3337298-12: [upstream] [GHS] CKEditor 5 removes empty inline elements:

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.

Steps to reproduce

Copy/paste that first snippet into https://ckeditor.com/docs/ckeditor5/latest/features/html/general-html-su...'s source view
turn off source view
return to source view

The wrapping <a> is indeed lost!

Proposed resolution

Wait for https://github.com/ckeditor/ckeditor5/issues/13803 to get fixed upstream.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

Wim Leers created an issue. See original summary.

Wim Leers credited glynster.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Postponed

Wim Leers credited Reinmar.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Just discussed with @Reinmar — this is on their radar, and they have enough information for them to act on this. This is likely complicated by the fact that the Link plugin needs to know if the wrapped content is a block or inline element, and that is non-trivial for GHS-powered elements.

glynster’s picture

glynster commented

@Wim Leers thanks for looking into this further. We tried all sorts of workarounds and we hit a deadend. What helped us was to use the following module ckeditor5_embedded_content. Then we created our own plugins as needed. This has actually been pretty awesome as any extra markup and so forth all resides within the template. For our case this was needed as we had hundreds of articles with ad blocks in them. The custom code was there in ckedtor 4 and as soon as we switched and saved any of them, the links vanished!

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
commented
Related issues: +#3272732: Drupal 10 & CKEditor 5 readiness

Noting here that this blocks entities embedded via entity_embed from being linked to other content. That was possible in CKE4 thanks to #2511404: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Contributed project blocker

Thanks for pointing that out, @fenstrat!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

Upstream issue was created: https://github.com/ckeditor/ckeditor5/issues/13803

Wim Leers credited witeksocha.

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

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

weseze’s picture

weseze commented

Adding another use case that is probably caused by the same issue here.

For our ckeditor4 -> 5 migration testproject we noticed all our custom anchors are disappearing.
We are using https://www.drupal.org/project/anchor_link in ckeditor4 for this. There is currently no support for ckeditor5 for this module.
But in our migration we had hoped to simply leave this module out and allow content editors to add and edit anchors in ckeditor source code: (since it is rarely used anyway)

...html...
<a id="my_anchor_point"></a>
...more html...

Our config allows a-tag and id attributes on them.
But they are still filtered out...

Updating to a cke5 comatible version of the "anchor_link" module would likely solve the issue (once it is released)
But I see no reason that empty a-tags with an id would be filtered out. This is a valid use case to make anchor-points in content.

witeksocha’s picture

witeksocha
Primary language Polish
Location Kraków
commented

@weseze most likely it's this upstream: https://github.com/ckeditor/ckeditor5/issues/9888, and related Drupal issue: https://www.drupal.org/project/drupal/issues/3337298. We are taking a look at this on the CKEditor side.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Fixed upstream 10 hours ago! 🥳 https://github.com/ckeditor/ckeditor5/commit/371ae0bd057c0120fbf3e1f7581...

neclimdul’s picture

neclimdul
He/Him
commented

My understanding of the explanation was it provided the tools to fix it and there may be some additional work for Drupal to fully support it for things like entity embedding. But exciting!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Update patch now live at #3370989: Update CKEditor 5 to 38.1.0! 🤩

Wim Leers credited lauriii.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Postponed » Fixed
Issue tags: -Needs upstream bugfix

#3370989: Update CKEditor 5 to 38.1.0 landed, the fix is shipping with 10.1.1! 🥳

Crediting @lauriii for the testing he did at #3370989-8: Update CKEditor 5 to 38.1.0.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Fixed » Needs work

The original example does work but entity embedding is still very broken so this isn't really solved for me. What's the next steps to be able to get that working?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Fixed
Related issues: +#3168966: Add ability to insert Media inline in CKEditor widget, +#3075527: FilterAutoP breaks links around the <drupal-media> tag

@neclimdul \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testLinkability() proves that this actually does work. \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testLinkedMediaArbitraryHtml() proves this even works in more complex cases.

You must be doing something even more complex, or there must be some contrib/custom logic causing this to break.

Or maybe you're trying to do #3168966: Add ability to insert Media inline in CKEditor widget? Or perhaps it's #3075527: FilterAutoP breaks links around the <drupal-media> tag that you're reporting? If it's neither of those two issues, please create a new bug report with exact steps to reproduce starting from a vanilla Drupal 10 site. I promise I'll investigate as soon as I have concrete steps to reproduce! 😊

Status: Fixed » Closed (fixed)

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

acbramley’s picture

acbramley commented

Unfortunately this still doesn't seem to work for entity_embed as detailed in #7

On 10.1.2 with
Embed patched with #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 (MR2 commit e4a16215)
Entity Embed patched with #3272732: Drupal 10 & CKEditor 5 readiness (MR12 commit c9970f19)

@Wim Leers while those tests prove the markup may work, the functionality doesn't seem to work in the actual WYSIWYG.

When I embed an entity, in my case Media (yes we need to migrate to media library but that's another story), the Link button is greyed out when selected on the embedded entity.

However, I now see that this issue was about <a> tags being stripped, not about supporting the link button with embedded entities. Perhaps we need a new issue?