Problem/Motivation

On July 29, we had a meeting with the CKEditor 5 developers.

On the subject of GHS (General HTML Support) news, @Reinmar mentioned that they've been integrating it with complicated features such as the Table plugin — to allow GHS to add more attributes to for example <table>.

We may need to do the same for our custom plugins, such as:

  1. drupalMedia.DrupalMedia for <drupal-media>, which currently only supports <drupal-media data-entity-type data-entity-uuid alt>
  2. drupalImage.DrupalImage, which overrides how <img> gets generated
  3. drupalEmphasis.DrupalEmphasis, which overrides <i> to <em>

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#16 3227822-16-d10.patch82 KBlauriii
#16 3227822-16-d9.patch82.15 KBlauriii

Issue fork drupal-3227822

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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Related issues: +#3246169: Embedded media are not linkable through UI; already linked embedded media are unlinked (data loss!), +#3246168: Images are not linkable through UI; already linked images are unlinked (data loss!)

#3246169: Embedded media are not linkable through UI; already linked embedded media are unlinked (data loss!) already added the foundations for this, but focused on only GHS for links wrapping <drupal-media>.

Both that issue and #3246168: Images are not linkable through UI; already linked images are unlinked (data loss!) added explicit test coverage for GHS + links.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Project: CKEditor 5 » Drupal core
Version: 1.0.x-dev » 9.3.x-dev
Component: Code » ckeditor5.module
Priority: Normal » Major
Issue tags: +Needs tests

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Version: 9.4.x-dev » 10.0.x-dev

bnjmnm opened merge request !1792

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Issue tags: -Needs tests

Added tests for adding an arbitrary attribute to <em>, <img>, and <drupal-media>. Only <drupal-media> fails

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

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Active » Needs review
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Needs work

Back to Needs work only for the functional test I requested on the MR.

This is looking great! 😊

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs work » Needs review
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

The thing I was asking about in the MR — my only remaining doubt — turned out to be exactly what #3231337: [drupalMedia] Remove manual dataDowncast from DrupalMediaEditing is about. @lauriii just added a pointer to that.

So: let's do this! 🚢 This will need a patch for all 3 branches to be tested — and while we do that, let's drop the drupalci.yml changes that this MR contains to speed up testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: GHS: ensure it works with our custom plugins, to allow adding additional attributes » [GHS] Ensure GHS works with our custom plugins, to allow adding additional attributes
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Status: Reviewed & tested by the community » Needs work

Looks fine aside from a \Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5Test::testImageCaption that was recently moved to testImageCaption but is being brought back here?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs work » Reviewed & tested by the community

That seems to have been added in one of the merges 😅 Not sure what happened there.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
FileSize
3227822-16-d9.patch82.15 KB
3227822-16-d10.patch82 KB

  • bnjmnm committed 24fac7d on 10.0.x 
    Issue #3227822 by lauriii, Wim Leers: [GHS] Ensure GHS works with our...

  • bnjmnm committed e53e947 on 9.4.x 
    Issue #3227822 by lauriii, Wim Leers: [GHS] Ensure GHS works with our...

  • bnjmnm committed 45c6373 on 9.3.x 
    Issue #3227822 by lauriii, Wim Leers: [GHS] Ensure GHS works with our...

bnjmnm closed merge request !1792

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Version: 10.0.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Everything looks good - the test coverage I was involved in creating was vetted by Wim & Lauri to be commit-worthy, and Wim reviewed Lauri's bugfix code, which I then deemed commit worthy.

Committed to 10.0.x / 9.4.x and cherry picked to 9.3.x since CKEditor 5 is experimental.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: +stable blocker

That was the last known item in the Ensure that sites can update from using CKE4 to CKE5 safely when using no contributed CKEditor modules bucket in the stable roadmap!

Status: Fixed » Closed (fixed)

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