Problem/Motivation

It seems like GHS enabled markup is not supported in data-caption and leads to completely crashing CKEditor 5.

Steps to reproduce

  1. Create Full HTML text format with media embed, media library, source editing and image captioning enabled.
  2. Embed any media to content using the text format
  3. Link the embedded media
  4. Edit <drupal-media> in source editing to include data-caption with <a> element with GHS enabled attributes
  5. Return from source editing and confirm that CKEditor crashes

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 3268311-14.patch2.96 KBlauriii
#12 3268311-12-test-only.patch2.31 KBlauriii
#11 3268311-test-only.patch2.31 KBlauriii

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Parent issue: » #3238333: Roadmap to CKEditor 5 stable in Drupal 9
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: GHS enabled markup in data-caption crashes CKEditor 5 » [drupalMedia] GHS-enabled markup in data-caption crashes CKEditor 5

I think that means this falls under the drupalMedia plugin?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Added to roadmap: #3238333-54: Roadmap to CKEditor 5 stable in Drupal 9.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue summary: View changes
Status: Active » Postponed

Updated the testing steps. This could be another side effect of #3268311: [upstream] [drupalMedia] GHS-enabled markup in data-caption crashes CKEditor 5.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Found a simple way to reproduce this without <drupal-media>. Reported upstream: https://github.com/ckeditor/ckeditor5/issues/11450.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [drupalMedia] GHS-enabled markup in data-caption crashes CKEditor 5 » [upstream] [drupalMedia] GHS-enabled markup in data-caption crashes CKEditor 5
Issue tags: +Needs upstream bugfix
wim leers’s picture

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

This has been fixed upstream 6 hours ago: https://github.com/ckeditor/ckeditor5/pull/11488 🚀

Now we are just waiting for a new CKEditor 5 release to update to 🥳

@lauriii: Do we need explicit test coverage on our end? If so, this is unblocked.

wim leers’s picture

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

Circling back … it's probably more productive to do both pieces at the same time. Let's wait for this to land upstream.

wim leers’s picture

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

There should be a new CKEditor 5 release tomorrow. That means we should already start writing a test.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
StatusFileSize
new 3268311-test-only.patch2.31 KB

Here's a try on a failing test 🤞

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3268311-12-test-only.patch2.31 KB

Status: Needs review » Needs work

The last submitted patch, 12: 3268311-12-test-only.patch, failed testing. View results

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Closed (outdated)
Issue tags: -Needs tests
StatusFileSize
new 3268311-14.patch2.96 KB

I'm actually not sure there is a valid way to trigger this with <drupal-media> because all of the use cases would require nesting elements in an invalid way. This was simply happening because of #3268318: [drupalMedia] <a> with GHS allowed attributes downcast wraps data-caption with <a> was duplicating the elements first and then as a result causing this issue. All of the other use cases where that could be triggered would be testing exactly what is already being tested upstream.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Oh, yay! Removed this from #3238333: Roadmap to CKEditor 5 stable in Drupal 9 :) Glad to see #3274767: Update to CKEditor 5 v34.0.0 fixed this!