Problem/Motivation

Test module editor_private_test depends on ckeditor which is on its way out of core.
Let's replace it with the up and coming star ckeditor5

Steps to reproduce

Proposed resolution

Replace ckeditor with ckeditor5 in 

core/modules/editor/tests/editor_private_test/editor_private_test.info.yml

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3306715

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:

About issue forks

Comments

Spokje created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Priority: Normal » Critical
Parent issue: » #3270437: [meta] Tasks to deprecate the CKEditor 4 module

Marking this as critical since the parent issue is marked as such.

Spokje opened merge request !2702

spokje’s picture

spokje
Primary language Dutch
commented
Issue summary: View changes
spokje’s picture

spokje
Primary language Dutch
commented
Title: Replace ckeditor with ckeditor5 in core/modules/editor/tests/editor_private_test/editor_private_test.info.yml » Replace ckeditor with ckeditor5 in editor_private_test.info.yml
spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3270734: Update Editor + CKEditor 5 module to not use CKEditor 4 in tests

I wonder why #3270734: Update Editor + CKEditor 5 module to not use CKEditor 4 in tests managed to not make this change… 🤔

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

Ah, I think I found why! https://git.drupalcode.org/project/drupal/-/commit/5b0cd33 removed the dependency on ckeditor and ckeditor5 altogether and it seems like this was simply an unnecessary dependency?

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Drupal 10 beta blocker
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Replace ckeditor with ckeditor5 in editor_private_test.info.yml » Replace ckeditor with editor_test in editor_private_test.info.yml
Component: ckeditor.module » editor.module
Status: Needs work » Reviewed & tested by the community

Being slightly bold 🤓

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
Drupal\Tests\editor\Functional\EditorPrivateFileReferenceFilterTest::testEditorPrivateFileReferenceFilter
Drupal\Core\Config\UnmetDependenciesException: Configuration objects provided by editor_private_test have unmet dependencies: editor.editor.private_images (editor)
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers

😬🙃

On it.

spokje’s picture

spokje
Primary language Dutch
commented

Just when I thought self-RTBC-ing was going to become big in 2022... 😈

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned
Status: Needs work » Needs review

The test failure says:

Drupal\Core\Config\UnmetDependenciesException: Configuration objects provided by editor_private_test have unmet dependencies: editor.editor.private_images (editor)

Kinda cryptic, right?

Would you have guessed that this means that the problem is that the editor module is not installed? I definitely would not have reached that conclusion! Without xdebug I could not have figured this out.

So this has been wrong in editor_test.info.yml since day 1, a decade ago! 🙈

Hoping for an RTBC from @Spokje this time 🤞

spokje’s picture

spokje
Primary language Dutch
commented

Green enough for me -> RTBC

And a bonus follow-up issue to restructure the core/modules/editor/tests/modules directory: #3307310: Restructure core/modules/editor/tests/modules test modules

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3307310: Restructure core/modules/editor/tests/modules test modules

Hah, +1 to eliminating wonkiness.

  • bnjmnm committed a5029ec on 10.1.x 
    Issue #3306715 by Spokje, Wim Leers: Replace ckeditor with editor_test...

  • bnjmnm committed 3be1b18 on 10.0.x 
    Issue #3306715 by Spokje, Wim Leers: Replace ckeditor with editor_test...

  • bnjmnm committed 6fd79d3 on 9.5.x 
    Issue #3306715 by Spokje, Wim Leers: Replace ckeditor with editor_test...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Reviewed & tested by the community » Fixed

The switch to 10 keeps surfacing stuff like this! Nice to see the cleanup happening, thanks @Spokje for catching this & getting it in the issue queue.

Committed to 10.1.x and cherry picked to 10.0.x and 9.5.x.

Spokje closed merge request !2702

Status: Fixed » Closed (fixed)

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