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:
drupalMedia.DrupalMediafor<drupal-media>, which currently only supports<drupal-media data-entity-type data-entity-uuid alt>drupalImage.DrupalImage, which overrides how<img>gets generateddrupalEmphasis.DrupalEmphasis, which overrides<i>to<em>
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 3227822-16-d10.patch | 82 KB | lauriii |
| #16 | 3227822-16-d9.patch | 82.15 KB | lauriii |
Issue fork drupal-3227822
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:
- 3227822-ghs-ensure-it
changes, plain diff MR !1792
Comments
Comment #2
wim leers#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.
Comment #3
wim leersComment #5
bnjmnmComment #7
bnjmnmAdded tests for adding an arbitrary attribute to
<em>,<img>, and<drupal-media>. Only<drupal-media>failsComment #9
lauriiiComment #10
wim leersBack to only for the functional test I requested on the MR.
This is looking great! 😊
Comment #11
lauriiiComment #12
wim leersThe 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.ymlchanges that this MR contains to speed up testing.Comment #13
wim leersComment #14
bnjmnmLooks fine aside from a
\Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5Test::testImageCaptionthat was recently moved totestImageCaptionbut is being brought back here?Comment #15
lauriiiThat seems to have been added in one of the merges 😅 Not sure what happened there.
Comment #16
lauriiiComment #21
bnjmnmEverything 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.
Comment #22
wim leersThat was the last known item in the bucket in the stable roadmap!