Preparation for moving CKEditor 5 to core

We don't want Drush-related things in Drupal core.

Rebased the patch, no longer applied after #3227853: Remove dependency on Vue.js.

(Not yet updating the shell script because more changes inbound.)

This removes the work-around test coverage that #3228355: PHP warning when saving text format with CKE5: $form_state->getTriggeringElement() unexpectedly returning NULL introduced, and which will be fixed in Drupal core in #3228956: PHP notices are not detected for headless browser-triggered requests in FunctionalJavascript tests.

Change package from CKEditor 5 to Core (Experimental) and use version: VERSION.

Also removing the Drush command.

EDIT: Never mind, I had already added that to the script…

#3231364-2: Add CKEditor 5 module to Drupal core failed gloriously on cspell :D

Adding "modules/ckeditor5/js/build/**" to core/.cspell.json.
Adding modules/ckeditor5/js/build to the list of directories that should not be checked for ES6ification.

Just moving it to core/assets/vendor instead. First step: patching ckeditor5.libraries.yml and core.libraries.yml. Note that this is not just cut and paste: significant change is necessary for things to still work.

Update core/package.json to find the build script in core/assets/vendor/ckeditor5 now.

I can't get

yarn run v1.22.5
$ cross-env BABEL_ENV=legacy node ./scripts/js/babel-es6-build.js --check --file /var/www/html/core/modules/ckeditor5/js/ckeditor5.admin.es6.js
[22:21:31] '/var/www/html/core/modules/ckeditor5/js/ckeditor5.admin.es6.js' is being checked.
Done in 1.19s.

/var/www/html/core/modules/ckeditor5/js/ckeditor5.admin.es6.js
  309:19  error  Unexpected lexical declaration in case block  no-case-declarations
  341:19  error  Unexpected lexical declaration in case block  no-case-declarations
  357:19  error  Unexpected lexical declaration in case block  no-case-declarations
  373:19  error  Unexpected lexical declaration in case block  no-case-declarations
  412:19  error  Unexpected lexical declaration in case block  no-case-declarations
  480:32  error  'Observable' was used before it was defined   no-use-before-define

✖ 6 problems (6 errors, 0 warnings)

figured out.

I cannot reproduce this locally. It seems that the auto-ES6-ification failed to do the correct thing here. I see similar cases elsewhere in Drupal core, and there it did get generated correctly.

So temporarily ignoring this file instead :D

We still need to update the CKE5 patch to update webpack.config.js to point to core/assets/vendor/ckeditor5/….

Finally, I do not know enough about webpack to know if it's safe for me to move its configuration to a different location:

Checking core/webpack.config.js

FAILURE core/webpack.config.js does not have a corresponding core/webpack.config.es6.js

But the commit script already has a similar exemption for core/postcss.config.js, so I'll just follow suit.

ALSO: found one spot I missed in #16…

Missed some things in the move to core/assets…

… and also for the core patch.

Add core/modules/ckeditor5/modules/ckeditor5_dev/js/ckeditor5_dev.es6.js to the eslint ignore list.

The last straggler: layercake.js not needing to be ES6ified because it's external code.

Using #27.

Fix the failing ComposerIntegrationTest: we were not yet updating composer.lock per https://www.drupal.org/about/core/policies/core-dependencies-policies/ma....

Update expectations in CKEditor5PluginManagerTest for the move of assets out of the CKE5 module and into core/assets/vendor/ckeditor5/.

In splitting up asset libraries between CKE5 module and core/assets/vendor, the CSS file for the language plugin ended up in both sides. Which caused a test fail at #3231364-25: Add CKEditor 5 module to Drupal core. This fixes that.

Update core patch for #3231427: Changing basic HTML format to CKEditor 5 results in CKEditorError: plugincollection-plugin-not-found {"plugin":null} on fresh install.

Update CKE5 patch for [#32321427]; it no longer applied.

This makes ResolvedLibraryDefinitionsFilesMatchTest pass tests.

Apply the latest patches.

… and add the composer.lock changes to the patch, which #29 should already have done.

EDIT: oh, nope, looks like we should've always been adding this one!

+++ b/src/Plugin/Editor/CKEditor5.php
@@ -816,7 +816,7 @@ class CKEditor5 extends EditorBase implements ContainerFactoryPluginInterface {
-      $plugin_libraries[] = 'ckeditor5/ckeditor5.translations.' . $ui_langcode;
+      $plugin_libraries[] = 'core/ckeditor5.translations.' . $ui_langcode;

D'oh — of course :D

Quoting #3231364-32: Add CKEditor 5 module to Drupal core:

We will instead from now post multiple patches: ckeditor5-CID.patch (includes everything) ckeditor5_assets_only-CID-do-not-commit.patch (assets only) and ckeditor5_module_only-CID-do-not-commit.patch.

Updated patches for #3227875: Add ckeditor5-remove-format to allow removing formatting from pasted content + #3227890: Add ckeditor5-special-characters to allow inserting special characters for users that do not know the native picker.

In the next round, we should also apply #3231324-4: Use core icons where possible after moving to core.

• Core patch rebased on top of 9da1866da41f0bfcdb90e1f8901ce8919549f189 in 9.3.x. and updated to account for #3227871: Add ckeditor5-paste-from-office to allow pasting from Microsoft Office & Google Docs.
• CKE5 patch: updated for #3227871: Add ckeditor5-paste-from-office to allow pasting from Microsoft Office & Google Docs.
• Script: updated for #3229433: Post-devue js directory refactoring.

D'oh, I forgot to include #3231324: Use core icons where possible after moving to core 😬 Will do so in the next reroll!

rm README.md

ckeditor5.ckeditor5.yml has this at the top:
# @see this module's README.md for details on defining CKEditor 5 plugins in

So it's a WTF that in the core patch this README doesn't exist. Maybe we should retain the README in the core patch? Or else change the above to reference something else?

Implemented #3243869: Should ckeditor5_dev module be excluded from the core patch and be a contrib project only?.

@lauriii I just realized you created the core/yarn.lock section of the -core.patch patches here? I've been updating that patch but have not been touching that area! 😅

Updating the core patch to now target 2073a9468d7220f5d2d0b7b4368cc3fcdb9f43c9 of the module, i.e. #3245942: Update CKEditor 5 to 31.0.0 — the core/yarn.lock needed to be updated.

Thanks to @lauriii for the mentoring on how to do this! (Basically: copy relevant parts from this module's package.json into core/package.json, then cd core && yarn install.)

Okay, #3231364-67: Add CKEditor 5 module to Drupal core failed to apply … and in fixing that I realized that we can keep the "core" patch here smaller, the script is taking care of regenerating the core/yarn.lock anyway 🥳

I wrote in #43 we should incorporate #3231324: Use core icons where possible after moving to core's patch, but that still hasn't happened. Writing it again to avoid forgetting again. Once it's in here, we can mark that issue fixed.

Massive changes to the core patch due to #3247246: Attribute value encoding not compatible with Xss::filter() and #3247711: Simplify and accelerate builds: update our use of the CKEditor 5 DLL manifest!

The former also brought a new dev dependency: jsdom.

Missed in #53: #3247246: Attribute value encoding not compatible with Xss::filter() introduced an asset library definitions change. Updating the core patch once more…

Rebased the CKE5 patch. Also for #3247246: Attribute value encoding not compatible with Xss::filter() in particular.

And now incorporating #52: #3231324: Use core icons where possible after moving to core.

Ugh, the patch generated by #56 failed because of the new Nightwatch test…

Let's see how this fares: #3044496-342: [ignore] bnjmnm patch testing.

Better, but not there yet:

Checking core/modules/ckeditor5/tests/src/Nightwatch/Tests/drupalHtmlBuilderTest.es6.js

yarn run v1.22.5
$ cross-env BABEL_ENV=legacy node ./scripts/js/babel-es6-build.js --check --file /var/www/html/core/modules/ckeditor5/tests/src/Nightwatch/Tests/drupalHtmlBuilderTest.es6.js
[16:37:06] '/var/www/html/core/modules/ckeditor5/tests/src/Nightwatch/Tests/drupalHtmlBuilderTest.es6.js' is being checked.
Done in 0.77s.

/var/www/html/core/modules/ckeditor5/tests/src/Nightwatch/Tests/drupalHtmlBuilderTest.es6.js
   4:19  error  'jsdom' should be listed in the project's dependencies, not devDependencies  import/no-extraneous-dependencies
  10:27  error  eval can be harmful                                                          no-eval

✖ 2 problems (2 errors, 0 warnings)

For the first one, I think we should do https://stackoverflow.com/a/55863857, but let's go with the simplest possible fix for now…

Fingers crossed for #3044496-343: [ignore] bnjmnm patch testing…

Two failures on the latest patch:

 Error: Cannot read test source location: /var/www/html/core/modules/ckeditor5/js/ckeditor5_plugins/drupalEngine/src/drupalhtmlbuilder.js

The patch is adding it at core/assets/vendor/ckeditor5/ckeditor5_plugins/drupalHtmlEngine/src/drupalhtmlbuilder.js
Looks like drupalHtmlBuilderTest.es6.js is using a relative path that works for contrib but not post-move.

1) Drupal\Tests\ckeditor5\Kernel\CKEditor5PluginManagerTest::testEnabledPlugins
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
     0 => 'ckeditor5/drupal.ckeditor5'
-    1 => 'core/ckeditor5.basic'
-    2 => 'core/ckeditor5.internal'
-    3 => 'core/ckeditor5.pasteFromOffice'
-    4 => 'core/drupal.ckeditor5.emphasis'
-    5 => 'ckeditor5/drupal.ckeditor5.media'
-    6 => 'ckeditor5_test/layercake'
+    1 => 'ckeditor5/drupal.ckeditor5.media'
+    2 => 'ckeditor5_test/layercake'
+    3 => 'core/ckeditor5.basic'
+    4 => 'core/ckeditor5.internal'
+    5 => 'core/ckeditor5.pasteFromOffice'
+    6 => 'core/drupal.ckeditor5.emphasis'
 )

Same ones, just different order.

I misread the green branch test result as a passing test 🙈🙈🙈

Tried to address both things in #59.

Fingers crossed for #3044496-344: [ignore] bnjmnm patch testing…

Closer, but not there yet.

Eyes on #3044496-345: [ignore] bnjmnm patch testing.

So close! The plugin manager test is now passing. Still something slightly wrong with file paths in the Nightwatch test, fixing that…

Eyes on #3044496-346: [ignore] bnjmnm patch testing.

+++ b/tests/src/Nightwatch/Tests/drupalHtmlBuilderTest.js
@@ -9,7 +9,8 @@ const { JSDOM } = require('jsdom');
 // eslint-disable-next-line no-eval
-const DrupalHtmlBuilder = eval(`(${fs.readFileSync(path.resolve(__dirname, '../../../../../../assets/vendor/ckeditor5/ckeditor5_plugins/drupalEngine/src/drupalhtmlbuilder.js')).toString()})`.replace('export default', ''));
+
+const DrupalHtmlBuilder = eval(`(${fs.readFileSync(path.resolve(__dirname, '../../../../../../assets/vendor/ckeditor5/ckeditor5_plugins/drupalHtmlEngine/src/drupalhtmlbuilder.js')).toString()})`.replace('export default', ''));

The no-eval is now failing because of the blank line

A most unwelcome and unfortunate accidental change 😬

→ #3044496-347: [ignore] bnjmnm patch testing (that issue is now timing out 😅 → see live progress at https://dispatcher.drupalci.org/view/D8%20Core%20Patches/job/drupal_patc...)

00:55:47.874 [Tests/drupalHtmlBuilderTest.es6]
00:55:47.883 ✔ should return empty string when empty DocumentFragment is passed
00:55:47.889 ✔ should create text from single text node
00:55:47.892 ✔ should return correct HTML from fragment with paragraph
00:55:47.896 ✔ should return correct HTML from fragment with multiple child nodes
00:55:47.899 ✔ should return correct HTML from fragment with comment
00:55:47.904 ✔ should return correct HTML from fragment with attributes
00:55:47.906 ✔ should return correct HTML from fragment with self closing tag
00:55:47.908 ✔ attribute values should be escaped
00:55:48.011 
00:55:48.011 [Tests/drupalHtmlBuilderTest]
00:55:48.013 ✔ should return empty string when empty DocumentFragment is passed
00:55:48.015 ✔ should create text from single text node
00:55:48.017 ✔ should return correct HTML from fragment with paragraph
00:55:48.020 ✔ should return correct HTML from fragment with multiple child nodes
00:55:48.022 ✔ should return correct HTML from fragment with comment
00:55:48.025 ✔ should return correct HTML from fragment with attributes
00:55:48.026 ✔ should return correct HTML from fragment with self closing tag
00:55:48.028 ✔ attribute values should be escaped

🥳

Address #3231364-75: Add CKEditor 5 module to Drupal core, points 2 and 3.

Relative to 7e3cf3bacce08f39c3c69b1081cefc2a9a15cf8b in CKE5, d63941bc1af2e1a31debad1f202e46351eaa149d in core.

This should address the failures for #68.

It also required https://git.drupalcode.org/project/ckeditor5/-/commit/b2dfb1c35102cddebe...

Being tested at #2488258-163: [ignore] Patch testing issue.

Swapped the CKE5 and core patch URLs 😇

#2488258-164: [ignore] Patch testing issue

Updated script to use #72.

#2488258-166: [ignore] Patch testing issue