Problem/Motivation

Update to CKEditor5 v31.1.0 in core: https://github.com/ckeditor/ckeditor5/releases/tag/v31.1.0

Key changes we need (besides the many bugfixes we benefit from):

Proposed resolution

  1. core/package.json
  2. cd core
  3. yarn install
  4. yarn run vendor-update
  5. yarn run build:ckeditor5

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

CKEditor 5 asset library updated to version 31.1.0.

Issue fork drupal-3258250

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hooroomoo created an issue. See original summary.

hooroomoo’s picture

Assigned: Unassigned » hooroomoo

cilefen’s picture

Because updating ckeditor in the past has been a very specific process, and because this looks like the first update to ckeditor5 in this codebase, can you write a few words about how you built the library? Also, I don't see core/core.libraries.yml changing in this diff.

Wim Leers’s picture

Status: Active » Needs work

can you write a few words about how you built the library

Basically: update core/package.json, yarn install, yarn run vendor-update, yarn run build:ckeditor5. So: for CKE5 the process is 99% identical to any other JS package.

Also, I don't see core/core.libraries.yml changing in this diff.

Indeed! @nod_, looks like vendor-update didn't update core/libraries.yml?

Or … @hooroomoo, any chance you didn't run that yet maybe? 🤓

hooroomoo’s picture

So I tried again on a second branch and did the same steps I did the first time that are listed in #5 and I'm still not seeing the core/core.libraries.yml updated for me.

Having @nod_ take a look tomorrow.

hooroomoo’s picture

Status: Needs work » Needs review
Wim Leers’s picture

Issue summary: View changes
Priority: Normal » Major
Status: Needs review » Needs work
Issue tags: +blocker
Related issues: +#3258250: Update to CKEditor5 v31.1.0

Note: this blocks #3247683: Disable CKEditor 5's automatic link decorators (in Drupal filters should be used instead).

👍 because:

  1. Verified locally that the JS has been correctly updated.
  2. window.CKEDITOR_VERSION now returns '31.1.0'
  3. All automated tests are passing.
  4. Basic manual testing works fine too.
  5. The only remaining references to 31.1.0 are in code comments' @see links.

👎 because:

  1. Looks like yarn run vendor-update did now update core/core.libraries.yml thanks to @hooroomoo's bugfix … but it only updated the core/ckeditor5 library, not the dozens others: core/ckeditor5.editorClassic, core/ckeditor5.specialCharacters, core/ckeditor5.list, et cetera.

    This suggests that we should do #3249698: Simplify CKEditor 5 asset library metadata after all? 🤔 If we landed that first, then this MR would not need to manually update all those lines!

nod_’s picture

Issue tags: +JavaScript
Wim Leers’s picture

Title: Update to CKEditor5 v31.1.0 » [PP-1] Update to CKEditor5 v31.1.0
Related issues: +#3258371: fix yarn vendor-update command

The core/scripts/js/assets.js change will land in #3258371: fix yarn vendor-update command.

Wim Leers’s picture

Status: Needs work » Needs review
lauriii’s picture

Title: [PP-1] Update to CKEditor5 v31.1.0 » Update to CKEditor5 v31.1.0
Wim Leers’s picture

Version: 9.4.x-dev » 10.0.x-dev
Wim Leers’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

It's a holiday in the U.S., so:

  1. merged HEAD of 9.4.x into MR to avoid it doing what #3258371: fix yarn vendor-update command already did: git fetch && git merge origin/9.4.x, then commit the result (no conflicts to resolve).
  2. then created an equivalent 10.0.x MR (3258250-10), like so:
    $ git checkout -b '3258250-10' --track drupal-3258250/'3258250-10'
    Branch '3258250-10' set up to track remote branch '3258250-10' from 'drupal-3258250'.
    Switched to a new branch '3258250-10'
    git diff 9.4.x 3258250-update-to-ckeditor5 -- core/package.json | git apply -3v
    Checking patch core/package.json...
    Applied patch to 'core/package.json' cleanly.
    Applied patch core/package.json cleanly.
    

    This means I ported the crucial changes from the 9.4.x MR, and they applied cleanly. 👍

    Now I need to rebuild everything for 10.0.x:

    wim.leers at Acquia in ~/Work/d8 on 3258250-10*
    $ cd core
    $ yarn install
    yarn install v1.22.17
    [1/5] 🔍  Validating package.json...
    [2/5] 🔍  Resolving packages...
    [3/5] 🚚  Fetching packages...
    [4/5] 🔗  Linking dependencies...
    [5/5] 🔨  Building fresh packages...
    success Saved lockfile.
    ✨  Done in 22.19s.
    wim.leers at Acquia in ~/Work/d8/core on 3258250-10*
    $ yarn run vendor-update
    yarn run v1.22.17
    $ node ./scripts/js/assets.js
    Copy backbone/backbone.js to backbone/backbone.js
    Copy backbone/backbone-min.js to backbone/backbone-min.js
    Process map file backbone-min.map
    Copy css.escape/css.escape.js to css-escape/css.escape.js
    Copy es6-promise/dist/es6-promise.auto.min.js to es6-promise/es6-promise.auto.min.js
    Process map file dist/es6-promise.auto.min.map
    Copy farbtastic/marker.png to farbtastic/marker.png
    Copy farbtastic/mask.png to farbtastic/mask.png
    Copy farbtastic/wheel.png to farbtastic/wheel.png
    Copy farbtastic/farbtastic.css to farbtastic/farbtastic.css
    Copy farbtastic/farbtastic.min.js to farbtastic/farbtastic.js
    Copy jquery/dist/jquery.js to jquery/jquery.js
    Copy jquery/dist/jquery.min.js to jquery/jquery.min.js
    Process map file dist/jquery.min.map
    Copy jquery-form/dist/jquery.form.min.js to jquery-form/jquery.form.min.js
    Process map file dist/jquery.form.min.js.map
    Copy jquery-form/src/jquery.form.js to jquery-form/src/jquery.form.js
    Copy joyride/jquery.joyride-2.1.js to jquery-joyride/jquery.joyride-2.1.js
    Copy jquery-once/jquery.once.js to jquery-once/jquery.once.js
    Copy jquery-once/jquery.once.min.js to jquery-once/jquery.once.min.js
    Process map file jquery.once.min.js.map
    Copy js-cookie/dist/js.cookie.min.js to js-cookie/js.cookie.min.js
    Copy normalize.css/normalize.css to normalize-css/normalize.css
    Copy @drupal/once/dist/once.js to once/once.js
    Copy @drupal/once/dist/once.min.js to once/once.min.js
    Process map file dist/once.min.js.map
    Copy picturefill/dist/picturefill.min.js to picturefill/picturefill.min.js
    Copy @popperjs/core/dist/umd/popper.min.js to popperjs/popper.min.js
    Process map file dist/umd/popper.min.js.map
    Copy shepherd.js/dist/js/shepherd.min.js to shepherd/shepherd.min.js
    Process map file dist/js/shepherd.min.js.map
    Copy sortablejs/Sortable.min.js to sortable/Sortable.min.js
    Copy tabbable/dist/index.umd.min.js to tabbable/index.umd.min.js
    Process map file dist/index.umd.min.js.map
    Copy underscore/underscore-min.js to underscore/underscore-min.js
    Process map file underscore-min.js.map
    Copy loadjs/dist/loadjs.min.js to loadjs/loadjs.min.js
    Copy @ckeditor/ckeditor5-alignment/build/alignment.js to ckeditor5/alignment.js
    Copy @ckeditor/ckeditor5-basic-styles/build/basic-styles.js to ckeditor5/basic-styles.js
    Copy @ckeditor/ckeditor5-block-quote/build/block-quote.js to ckeditor5/block-quote.js
    Copy @ckeditor/ckeditor5-editor-classic/build/editor-classic.js to ckeditor5/editor-classic.js
    Copy @ckeditor/ckeditor5-editor-decoupled/build/editor-decoupled.js to ckeditor5/editor-decoupled.js
    Copy @ckeditor/ckeditor5-essentials/build/essentials.js to ckeditor5/essentials.js
    Copy @ckeditor/ckeditor5-heading/build/heading.js to ckeditor5/heading.js
    Copy @ckeditor/ckeditor5-horizontal-line/build/horizontal-line.js to ckeditor5/horizontal-line.js
    Copy @ckeditor/ckeditor5-image/build/image.js to ckeditor5/image.js
    Copy @ckeditor/ckeditor5-indent/build/indent.js to ckeditor5/indent.js
    Copy @ckeditor/ckeditor5-language/build/language.js to ckeditor5/language.js
    Copy @ckeditor/ckeditor5-link/build/link.js to ckeditor5/link.js
    Copy @ckeditor/ckeditor5-list/build/list.js to ckeditor5/list.js
    Copy @ckeditor/ckeditor5-paste-from-office/build/paste-from-office.js to ckeditor5/paste-from-office.js
    Copy @ckeditor/ckeditor5-remove-format/build/remove-format.js to ckeditor5/remove-format.js
    Copy @ckeditor/ckeditor5-source-editing/build/source-editing.js to ckeditor5/source-editing.js
    Copy @ckeditor/ckeditor5-table/build/table.js to ckeditor5/table.js
    Copy @ckeditor/ckeditor5-html-support/build/html-support.js to ckeditor5/html-support.js
    Copy @ckeditor/ckeditor5-special-characters/build/special-characters.js to ckeditor5/special-characters.js
    Copy ckeditor5/build/ckeditor5-dll.js to ckeditor5/ckeditor5-dll.js
    ✨  Done in 0.47s.
    wim.leers at Acquia in ~/Work/d8/core on 3258250-10*
    $ yarn run build:ckeditor5
    yarn run v1.22.17
    $ webpack --config ./modules/ckeditor5/webpack.config.js
    asset drupalEmphasis.js 1.15 KiB [compared for emit] [minimized] (name: path)
    orphan modules 1020 bytes [orphan] 2 modules
    runtime modules 396 bytes 2 modules
    built modules 1.21 KiB [built]
      ./modules/ckeditor5/js/ckeditor5_plugins/drupalEmphasis/src/index.js + 2 modules 1.13 KiB [built] [code generated]
      delegated ./core.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
      external "CKEditor5.dll" 42 bytes [built] [code generated]
    webpack 5.63.0 compiled successfully in 417 ms
    
    asset drupalHtmlEngine.js 2.31 KiB [compared for emit] [minimized] (name: path)
    orphan modules 6.01 KiB [orphan] 3 modules
    runtime modules 396 bytes 2 modules
    built modules 6.25 KiB [built]
      ./modules/ckeditor5/js/ckeditor5_plugins/drupalHtmlEngine/src/index.js + 3 modules 6.17 KiB [built] [code generated]
      delegated ./core.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
      external "CKEditor5.dll" 42 bytes [built] [code generated]
    webpack 5.63.0 compiled successfully in 487 ms
    
    asset drupalImage.js 8.58 KiB [compared for emit] [minimized] (name: path)
    orphan modules 636 KiB [orphan] 647 modules
    runtime modules 396 bytes 2 modules
    built modules 23.5 KiB [built]
      modules by path delegated ./*.dll 126 bytes
        delegated ./core.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
        delegated ./upload.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
        delegated ./utils.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
      ./modules/ckeditor5/js/ckeditor5_plugins/drupalImage/src/index.js + 7 modules 23.3 KiB [built] [code generated]
      external "CKEditor5.dll" 42 bytes [built] [code generated]
    webpack 5.63.0 compiled successfully in 1631 ms
    
    asset drupalMedia.js 16.1 KiB [compared for emit] [minimized] (name: path)
    orphan modules 663 KiB [orphan] 660 modules
    runtime modules 396 bytes 2 modules
    built modules 51.5 KiB [built]
      modules by path delegated ./*.dll 168 bytes
        delegated ./core.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
        delegated ./ui.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
        delegated ./widget.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
        delegated ./utils.js from dll-reference CKEditor5.dll 42 bytes [built] [code generated]
      ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js + 20 modules 51.3 KiB [built] [code generated]
      external "CKEditor5.dll" 42 bytes [built] [code generated]
    webpack 5.63.0 compiled successfully in 1756 ms
    ✨  Done in 3.29s.
    wim.leers at Acquia in ~/Work/d8/core on 3258250-10*
    

    and then commit & push it.

So, I didn't make any changes, just ran the same commands again. Just to be safe. Because I thought there would not be differences, but theoretically due to different package.jsons it'd have been possible:

$ git diff HEAD 3258250-update-to-ckeditor5 -- core/package.json
diff --git a/core/package.json b/core/package.json
index 9398f9761f..f6096afe2f 100644
--- a/core/package.json
+++ b/core/package.json
@@ -150,6 +150,7 @@
     "last 2 Edge major versions",
     "last 2 Opera versions",
     "last 2 iOS major versions",
+    "last 1 Explorer major version",
     "last 1 ChromeAndroid version",
     "last 1 UCAndroid version",
     "last 1 Samsung version",
wim.leers at Acquia in ~/Work/d8 on 3258250-10*

Fortunately, in practice, that's not the case … except that CKE5 in the 10.0.x branch was apparently missing one translation file:

$ git diff --stat HEAD 3258250-update-to-ckeditor5 -- core/assets/vendor/ckeditor5
 core/assets/vendor/ckeditor5/translations/uz.js | 1 +
 1 file changed, 1 insertion(+)

That's the only difference.

Most importantly, there is an empty diff for the yarn.lock file:

$ git diff 3258250-10 3258250-update-to-ckeditor5 -- core/yarn.lock
wim.leers at Acquia in ~/Work/d8 on 3258250-10*

Thanks to this adding that missing translation file, it should once again be possible to have a single MR/patch apply to all branches (9.3.x, 9.4.x and 10.0.x) in the future for CKEditor 5. 👍

I did not do any significant work on this, just mechanics, so I'm able to RTBC this.

Wim Leers’s picture

This was the sole failure for the 10.0.x test:

Testing Drupal\Tests\Core\Command\QuickStartTest
.SF..                                                               5 / 5 (100%)

Time: 00:18.457, Memory: 4.00 MB

There was 1 failure:

1) Drupal\Tests\Core\Command\QuickStartTest::testQuickStartInstallAndServerCommands
Failed asserting that 'Drupal development server started: http://127.0.0.1:8889>\n
This server is not meant for production use.\n
' contains "127.0.0.1:8889/user/reset/1/".

I expect the currently running test to pass. That failure cannot have been caused by this MR.

  • lauriii committed 3560d07 on 10.0.x
    Issue #3258250 by Wim Leers, hooroomoo: Update to CKEditor5 v31.1.0
    

  • lauriii committed 2bc77bc on 9.4.x
    Issue #3258250 by Wim Leers, hooroomoo: Update to CKEditor5 v31.1.0
    

lauriii’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 3560d07 and pushed to 10.0.x. Also committed the Drupal 9 MR to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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