Problem/Motivation
#3396133: CKEditor 5 JS error when editing certain content raised confusion around the fact that the CKEditor 5 support that was added in #3272732: Drupal 10 & CKEditor 5 readiness works only on Drupal >=10, and not also on Drupal 9.5.
(The tests specifically are skipped on <10: https://git.drupalcode.org/project/entity_embed/-/blame/ac6f2fe13b18f5f55c43badd6f46195776e2351c/tests/src/FunctionalJavascript/CKEditor5IntegrationTest.php#L71-73)
But … it was actually only ever tested against Drupal 10.1 😅 Combined with the fact that Drupal core minors update CKEditor 5 to new major versions, and based on past experience we know that sometimes a CKEditor 5 major release in a new Drupal core minor results in contrib modules needing to re-build their JS and tag a new release, that is then compatible only with that minor.
Steps to reproduce
N/A
Proposed resolution
- ✅
Automatically run tests on the current minor (10.1)— already the case in HEAD - Automatically run tests on the previous minor (10.0) — this would ensure we know whether we need distinct releases
- Automatically run tests on the next minor (10.2) — this would ensure we know about upcoming need for a new release of Entity Embed for that core branch
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork entity_embed-3396568
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:
Comments
Comment #3
wim leersResults:
10.0.x(which uses CKEditor 5 v35.4.0): https://git.drupalcode.org/project/entity_embed/-/jobs/22287010.1.x(which uses CKEditor 5 v38.0.1 in 10.0.0 and v38.1.0 in 10.1.0 and v39.0.1 in 10.1.3: https://git.drupalcode.org/project/entity_embed/-/jobs/22285410.2.x(which uses CKEditor 5 v39, and soon v40 — see #3393557: [upstream] Update CKEditor 5 to 40.0.0): https://git.drupalcode.org/project/entity_embed/-/jobs/222855This that this actually would be a pretty strong reason to start the following branching strategy:
2.0.xof Entity Embed for Drupal 10.0.x, but recompile for10.0.x)2.1.xfor Drupal 10.1.x (straight branch from8.x-1.x)8.x-1.xof Entity Embed for Drupal 9.5.x (revert #3272732: Drupal 10 & CKEditor 5 readiness)2.2.xfor Drupal 10.2.x or 10.3.x or …ℹ️ This is similar to Editor Advanced Link, which has
2.1.xfor Drupal 10.0.x and 2.2.x for Drupal 10.1.x — although they managed to make 2.2.x also work on 10.0.x. But that's not always possible.@Dave Reid, I briefly mentioned the above last Friday, but I'm not sure you fully realized this. I suspect you won't like this 😅 But unfortunately it's out of my hands — it's a policy thing on the CKEditor 5 side.
They are working on doing fewer breaking changes — which is why they didn't go from https://github.com/ckeditor/ckeditor5/releases/tag/v38.0.0 directly to https://github.com/ckeditor/ckeditor5/releases/tag/v39.0.0, but did https://github.com/ckeditor/ckeditor5/releases/tag/v38.1.0 and tried doing https://github.com/ckeditor/ckeditor5/releases/tag/v38.2.0-alpha.1, but eventually never tagged a 38.2.0, and instead had to go to https://github.com/ckeditor/ckeditor5/releases/tag/v39.0.0.
But they have still got some ways on maturing into avoiding breaking changes.
Comment #4
wim leersI just wrote "2.0.x for Drupal 10.0.x", but I think it's equally reasonable for Entity Embed to require Drupal 10.1.x, and just say that that is the first and only version to support CKEditor 5.
P.S.: the self-RTBC here is only for the added test jobs. But still, it's failing on
10.0.x, so really … this need discussion with @Dave Reid to determine how he wants to proceed here.Comment #6
dave reidI think I've got it covered by skipping the CKEditor5 test on 10.0, using the new GitLab CI variables for OPT_IN_TEST_PREVIOUS_MAJOR, OPT_IN_TEST_PREVIOUS_MINOR, and OPT_IN_TEST_NEXT_MINOR.
Comment #7
dave reidEverything is passing now. Merging.
Comment #9
dave reidComment #10
wim leers👍 Looking good! 😊