Update to CKEditor 5 v34.1.0 in core when this URL no longer 404s: https://github.com/ckeditor/ckeditor5/releases/tag/v34.1.0
v34.1 unblocks:
Note that this will also fix #3277405-3: Update @ckeditor/ckeditor5-list to v34.0.1.
core/package.json
cd core
yarn install
yarn run vendor-update
yarn run build:ckeditor5
yarn run build:ckeditor5-types
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #28 | is-this-the-magic.patch | 869 bytes | xjm |
| #27 | cke5-why-27-d10.patch | 2.5 MB | xjm |
| #23 | interdiff.txt | 1.35 KB | lauriii |
| #23 | 3277438-23-d95.patch | 2.58 MB | lauriii |
| #23 | 3277438-23-d93.patch | 2.58 MB | lauriii |
Comments
Comment #2
wim leersI've scanned Drupal's upstream CKE5 issues.
After the CKE5 v35 update, only two stable blockers will remain.
v35 will unblock:
The remaining stable blockers from the roadmap (#3238333: Roadmap to CKEditor 5 stable in Drupal 9):
v35's code freeze is planned for May 18th, 2022, followed by releasing npm packages on May 25th, 2022.
Comment #4
wim leersPer @Reinmar:
Comment #5
wim leersLooks like the PR that adds support for #3274635: [upstream] Use CKEditor 5's native <ol type> and <ul type> UX (https://github.com/ckeditor/ckeditor5/pull/11699) landed 2 hours ago: https://github.com/ckeditor/ckeditor5/commit/a6c677fa403ad0f907bab5c56a0.... So that will ship with v35/v34.1 too! 🤩
Comment #6
wim leershttps://www.drupal.org/project/drupal/issues/3277438
Comment #7
wim leersBecause #3266912: Review version constraints for production yarn dependencies did not land in
9.3.x, we need a different patch here for that branch. All 3 other branches that need to be updated can use the same patch (9.4.x,9.5.x,10.0.x) — it applies cleanly to all three 👍The only differences between the two patches are in
core/package.jsonand consequently also incore/yarn.lockComment #8
wim leers😬😬😬
Comment #9
wim leersAh … if I don't do
-3vbut just-v, I can reproduce it.I really wish DrupalCI applied patches with
-3(for 3-way merge). It'll fail if it cannot resolve conflicts with absolute confidence. Ah well 😔10.0.x-specific patch attached.Comment #10
nod_missing the update to the `ckeditor5` package :)
Comment #11
wim leersSo I forgot
in
devDependencies😬But … metadata detail doesn't even matter because it looks like this caused consistent failures in
ImageTestandMediaTestfunctional JS tests. Those will need to be investigated.Comment #12
nod_It does impact the
ckeditor-dll.jsfile so might be relevant?Comment #13
wim leersD'oh, right!
This was pretty painful — can't wait to not have to generate patches for four branches 🤪
Looks like I made not one mistake (#10) but two: I hadn't noticed previously that new plugin translations were available! 🙈
👆 These are all new! Mostly
uraka Urdu 👍Comment #14
nod_All good for me :)
Comment #18
wim leers#11:
… still happening. So: genuine failures. 😬
Comment #19
wim leersI got a message from CKEditor 5 developer https://github.com/niegowski:
Maybe that's causing the failures 🤞
Comment #20
bnjmnm#19 takes care of the failing Media tests, but not the fails in
\Drupal\Tests\ckeditor5\FunctionalJavascript\ImageTest::testLinkability. looking into that now...Comment #21
bnjmnmThe image test is because of a no-longer-needed workaround that is specifically targeted here #3277438: Update to CKEditor 5 v34.1.0. I'm going to close that issue as it has to solved for the update to be applied at all.
Comment #23
lauriiiComment #24
xjmJust getting this into the issue comments since the patch is 2.5 MB of mostly-automated updates. This is the test discussed in #21 and the only thing that needs actual code review.
Comment #25
lauriiiReviewed changes since #14 and all looks good. I realize that I authored #23 but I think the changes in #23 and #21 are simple enough that it shouldn't prevent from moving this issue to RTBC.
Comment #26
xjmSoo I'm not getting the same results as the patch. Steps for the 10.0.x patch:
cd coreemacs package.jsonreplace all 34.0.x with 34.1.x throughout the file.rm -rf node_modules; yarn installyarn run vendor-updateyarn run build:ckeditor5yarn run build:ckeditor5-typesI would expect the diff between this and #23 to only be the image test, but the actual diffstat is:
Comment #27
xjmIn #26 I just forgot to stage the new files; the actual diffstat against 23 is:
...whereas it should just be ImageTest. Not sure why
core.libraries.ymldoesn't match either.Here's the full patch I would get, minus ImageTest changes.
Comment #28
xjmOK here is the missing step 0: Apply attached one-line change.
Then follow steps in #26. Result:
...Which is what I expected, plus or minus a todo in the libraries file that should be manually removed.
Comment #29
xjmHere are the changes to the built assets in 10.0.x:
That is identical to the change in the interdiff from #28, in the built file. So that is reassuring.
Comment #30
xjmConfirmed the 9.5.x/9.4.x change is the same as #29.
Comment #35
xjmJust as validation for the RTBC in #25, @bnjmnm confirmed that he was about to upload an identical fix as #23 and practically crossposted with it.
Committed to 10.0.x, 9.5.x, 9.4.x, and 9.3.x. Phew! There is a new adventure almost every time we update CKE5.
Updating release tag because there was a sec release yesterday, so this will be a new fix next week on the June bugfix window.
Thanks everyone!
Comment #36
wim leers#21: hah — I even said that in the issue summary: it's the 3rd unblocked issue! 😄 But I forgot that it would break tests, because our tests were literally designed to break if the fix would land upstream 🙈 Thanks for sorting that out!
"The 4 issues from the issue summary that are unblocked:
That means
three2 fewer stable blockers! 🥳 (EDIT: 2, not 3, because #3273552: [upstream] codeBlock breaks when changing the `language-*` class was not a stable blocker.)