Problem/Motivation

The editor module was introduced in #1833716: WYSIWYG: Introduce "Text editors" as part of filter format configuration, to layer Text Editor integration on top of our Text Format concept provided by the filter module.

A major reason for doing this was because we wanted it to be entirely optional: rich text/WYSIWYG editors were frowned upon at the time, especially because they generated poor HTML markup.

Times have changed.

Drupal 8 shipped with CKEditor 4, and everything was nicely decoupled. Custom text editors can be implemented. And they are. We should keep the ability to have different text editors for sure!

But … the split between the filter and editor modules, that is more questionable. It means a lot of extra complexity.

On the form side:

  1. editor module must alter the filter_format form to inject its things. It generates a subform (and subform state) for this, so it can make the text editor configurable and allow the Editor plugin to inject its form.
  2. and in turn, a specific text editor that provides extensibility (such as core's CKEditor module) must generate a subform (and subform state) for every plugin that it accepts.

On the configuration side:

  1. FilterFormat and Editor are two distinct config entity types, and each Editor is tightly coupled to a FilterFormat
  2. … but it's the end user's responsibility to keep these in sync!
  3. If/when we add config entity validation (see #3231342: [PP-2] Introduce ConfigEntityForm to standardize use of validation constraints for config entities), this will become worse, because we'll need to always validate a FilterFormat + Editor pair!

Proposed resolution

Merge them. A single new TextFormat config entity that can optionally contain a text editor — text editors are and should remain optional.

Remaining tasks

Postponed on #3425349: [PP-1] Deprecate \Drupal\ckeditor5\Plugin\Editor\CKEditor5::validatePair() and `type: ckeditor5_valid_pair__format_and_editor`

User interface changes

API changes

Data model changes

Release notes snippet

Comments

Wim Leers created an issue. See original summary.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

catch’s picture

catch
he/him
Primary language English
commented

This seems reasonable to me.

Would editor module still exist to provide the image upload/link/untransformed routes and etc. and we just remove the configuration handling from it?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

IMHO: no, the module would be gone entirely.

We'd have to keep the routes around for BC reasons of course, but there's no reason that the editor.* routes could not exist in core/modules/text_format/text_format.routing.yml — we have no validation at all around that anyway 🤓

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

This would also make #3225033: Whenever "Enable image uploads" is enabled for a text editor, the editor_file_reference filter should be enabled simpler to fix.

catch’s picture

catch
he/him
Primary language English
commented

OK this seems like a good plan, but it might end up being one for 10.1.x or later since it'll have a lot of deprecations we won't want to commit to 9.5.x

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Yep, agreed! And … there's no way in hell this would happen in time anyway 😅

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I had not seen this issue before but I came to the same conclusion while reviewing #3270734: Update Editor + CKEditor 5 module to not use CKEditor 4 in tests with Wim.

Do we need the complexity of merging the two existing config entities into a new config entity, or do we just extend FilterFormat to include an optional editor? editor.module could even still exist if we want to continue separation of concerns - some sites may not provide a WYSIWYG editor at all, but still handle HTML that needs to be filtered. Editor config could be considered a third party setting for a filter format?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Do we need the complexity of merging the two existing config entities into a new config entity, or do we just extend FilterFormat to include an optional editor?

This is definitely how I envisioned this happening!

editor.module could even still exist if we want to continue separation of concerns - some sites may not provide a WYSIWYG editor at all, but still handle HTML that needs to be filtered.

Not sure yet if that's necessary or desirable. But I'm 70% convinced we should not do that. A lot of complexity lies in form alterations that editor.module has to do. Removing those would be valuable.

Using a text editor would definitely remain optional.

Editor config could be considered a third party setting for a filter format?

I think it'd be simpler to instead have an optional editor entry in the filter_format config entity 🤓

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2830210: Allow tokens to be used in Text Editor/CKEditor 5 image upload directory path

Something interesting just came up that is very closely related to this issue: #2830210-30: Allow tokens to be used in Text Editor/CKEditor 5 image upload directory path.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

aaronmchale’s picture

aaronmchale
Primary language English
Location Edinburgh, Scotland
commented

Just stopping by to note that, if this is done, we figured out a BC-safe reusable pattern for removing/moving admin paths around. Which we originally did for moving around the Boock types and Custom block library.

Please do feel free to reuse the same pattern.

See #3333383: Create a redirect for the new Block types path.

Oh and just ignore me if there aren't actually any paths to redirect here, but just dropping this in incase it's useful.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Oh and just ignore me if there aren't actually any paths to redirect here, but just dropping this in incase it's useful.

There aren't 😄 Because editor has no admin UI of its own; it already is altering the format admin UI!

aaronmchale’s picture

aaronmchale
Primary language English
Location Edinburgh, Scotland
commented

There aren't 😄 Because editor has no admin UI of its own; it already is altering the format admin UI!

Excellent! One less thing to worry about then :D

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Note: this should also do #3064957: getJSSettings() is not in lowerCamel format.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [META] Discuss: merge the Editor config entity into the FilterFormat config entity » [PP-1] [META] Discuss: merge the Editor config entity into the FilterFormat config entity
Status: Active » Postponed
Related issues: +#3425349: [PP-1] Deprecate \Drupal\ckeditor5\Plugin\Editor\CKEditor5::validatePair() and `type: ckeditor5_valid_pair__format_and_editor`

We should first land #3425349: [PP-1] Deprecate \Drupal\ckeditor5\Plugin\Editor\CKEditor5::validatePair() and `type: ckeditor5_valid_pair__format_and_editor`, which I'm working on right now 👍

quietone’s picture

quietone commented
Issue summary: View changes

Adding postponed to remaining tasks according to remaining tasks.

catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-1] [META] Discuss: merge the Editor config entity into the FilterFormat config entity » [PP-2] [META] Discuss: merge the Editor config entity into the FilterFormat config entity

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.