Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
\Drupal\ckeditor5\SmartDefaultSettings::getEnabledCkeditor4Plugins() must be updated before we actually physically remove the CKE4 (ckeditor) module from core to not use the CKE4 plugin manager and/or CKE4 plugin interfaces, to ensure that sites running the D10 upgrade path actually work if they are still using CKE4 at that time.
Note that we could do this in Drupal 9 already.
Steps to reproduce
Proposed resolution
Stop relying on executing any code in core/modules/ckeditor during the CKE 4 → 5 upgrade path.
Remaining tasks
Review.
User interface changes
None.
API changes
None.
Data model changes
None.
Release notes snippet
The CKEditor 4 to 5 upgrade path no longer checks if a plugin is enabled before migrating its config. [Consequences here.] Sites should update to Drupal 9.4.4 or higher prior to updating to Drupal 10, and should ensure update.php is run. This will ensure disabled CKEditor 4 plugins do not have their config stored.
| Comment | File | Size | Author |
|---|---|---|---|
| #37 | 3270831-37-94x.patch | 9.08 KB | bnjmnm |
Issue fork drupal-3270831
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:
changes
Comments
Comment #2
wim leersLet's try to do this even before Drupal 10; the upgrade path in Drupal 9.3 & 9.4 should ideally not assume more access to CKEditor 4 code than we will be able to in Drupal 10.
Comment #3
wim leersComment #5
wim leersFailure:
→ that's for a configured CKEditor 4 plugin … that actually was disabled. So the equivalent CKEditor 5 configuration should not have been generated (this was the case in HEAD) or at least been removed prior to saving the CKEditor 5 text editor configuration (this is what we should do now).
Pushing a commit for that: f32329d309a9410f1181598a332acc5173576dfc.
Comment #6
wim leersComment #7
wim leersContemplating whether this should be tagged and added to #3238333: Roadmap to CKEditor 5 stable in Drupal 9 🤔
Comment #8
wim leersComment #9
xjm@Wim Leers I think it's a Standard Profile blocker rather than a stable blocker, unless these changes would alter the data model (the results of the upgrade path).
Comment #10
xjm(Rebased because I needed a patch version of it.) :)
Comment #11
wim leers#9 IMHO it's a because we should have the exact same upgrade path regardless of whether you're using Drupal 9.4 or 10.0. Otherwise it becomes a maintainability nightmare.
Comment #13
wim leersComment #14
wim leersPulled in the
9.4.xchanges since March 24 👍Comment #15
wim leersThere was one test failure due to #3273325: CKE5 and contrib: better "next action" description on upgrade path messages changing the structure for "expected messages". Fixed in 4c1a5f72b91fae7bfe45de3f9c23c96ad0baaaf0.
Ready for final review again!
Comment #16
bnjmnmCode looks good, but there's an aspect to this change I want to bring up.
So this changes the 4-5 migration a bit by assuming any CKE4 plugin with configuration is enabled, vs the current CK4-dependent method of it being possible to check
isEnabled(). That seems reasonable, particularly since SmartDefaultSettings provides a great deal of feedback regarding what is happening, it errs in the direction of no data loss. So overall this seems fine, but there's something I'd at least like clarified here.In D9, users would be able to disable any plugin they wish prior to saving the migrated text format. For the D10 upgrade path, it's possible for a plugin disabled in CKE4 becoming enabled in CKE5 due to the presence of config. There's good messaging for plugins enabled to allow specific elements, but the messaging here isn't quite as thorough. Should the upgrade messages, when it's from CKE4, include some emphasis that there may be CKE4 disabled plugins that are now enabled in CKE5? Something like "The following configurable plugins were migrated and enabled in CKEditor 5. If you would prefer any of these not be enabled go to admin/config/content/formats/yer-format". I'm unsure how often this would even be an issue, so the utility of that should be weighed against the noise it adds. I at least wanted to mention it.
Comment #17
wim leersCorrect.
The tricky bit is that it is impossible to know which CKEditor 4 plugins were actually enabled, because the CKEditor 4 admin UI does not guarantee that disabled CKE4 plugins have their plugin config removed.
That's possible, but likely also confusing.
I have a counterproposal: let's fix the bug in the CKE4 admin UI, and let's add a post-update hook that cleans up existing configuration for sites still on Drupal 9. That means that any site that first updates to Drupal 9.4.Z (with Z being the patch release in which this ships) or 9.5.0 prior to updating to 10.0.x would have only have CKE4 plugin config for actually enabled plugins!
I just scoured the CKEditor 4 issue queue and could not find an existing issue for this. I'd be happy to create that :)
Comment #18
bnjmnm#17
👍👍👍👍👍👍👍👍👍👍👍👍
This addresses my concern in a way that benefits every user without adding additional (unreasonable) "read every line within these long messages" pressure on what is already a complex process. Create the issue and I can RTBC. I don't believe that issue needs to block this one, but it would need to be a Stable Blocker (but seemingly a fairly simple one?)
Comment #19
wim leersDone: #3291744: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins.
Tired now, will add more details tomorrow (unless you beat me to it).
Comment #20
catchShould we account for ckeditor in #3290810: Remove updates added prior to 9.4.0 (9.4.4 for ckeditor) and add 9.4.0 database dumps? - i.e. require that sites with ckeditor4 enabled update to 9.4.version-with-the-new-update first?
Comment #21
wim leers@bnjmnm: #3291744: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins now has a patch that implements all that! 👍
@catch
That would be fantastic, I had no idea that was even possible!
Comment #22
bnjmnmSince #3291744: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins is officially a stable blocker I'm comfortable RTBCing the changes here.
Comment #23
wim leersYay! 🥳
Comment #27
xjmThanks @bnjmnm and @Wim Leers for the very clear discussion in #16 on, and @catch for the suggestion in #20; the above address all the questions/concerns I had for this issue.
I got a little ahead of myself committing it though; we should have a change record here. We'll also want to mention it in the 9.4.2 release notes since we will need this change in 9.4.x as soon as possible for the upgrade path.
Once we have the CR, I'll backport to 9.4.x as well.
Thanks everyone!
Comment #28
xjmComment #29
xjmComment #30
xjmUpdating tag for the next window.
Comment #31
bnjmnmComment #32
bnjmnmCR and release notes created. Still needs to be added to 9.4.4
Comment #33
xjmMaking the release note a bit more focused on the site owner's jobs here.
I think the CR and release note need a bit more of the info from #16 and #17.
Comment #34
bnjmnmRe #33
The concerns in #16 and #17 were addressed by #3291744: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins. With that issue in, the CKEditor 4->5 delivers the same results before/after the changes done here, provided they run db updates. It seems like it would create unnecessary noise to elaborate on something people won't encounter unless they neglect to run db updates or they're patching a pre-9.4.4 site with just the changes here and expecting it to work.
Comment #35
xjmAhh so the only disruption then is that
update.phpneeds to be run for a patch release, which usually we avoid. Sorry, got lost in the weeds with the two issues.Comment #36
xjmPolished the CR text a bit more (same content, just simplified the language and grammar a little).
Unfortunately, this does not cherry-pick cleanly to 9.4.x (possibly due to the fact that we committed the two issues in reverse order for this branch). Can we get a patch or MR for 9.4? Thanks!
Comment #37
bnjmnmHere's a 9.4 patch. Note that unlike the >=9.5 commits, there's no changes to SmartDefaultSettingsTest as those changes were effectively reverted a few days later upon committing #3291744: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins.
3291744 landing made it possible to remove the CKEditor 4 dependency without changing the overall CK4->5 migration process. This upgrade path has thorough test coverage, and the fact that the test doesn't need changing is additional confirmation that this is a nondisruptive change 😎.
Comment #39
xjmGreat, thanks @bnjmnm. Committed #37 to 9.4.x.