Synchronize editor status with paired text format status

1. +++ b/core/modules/editor/src/EventSubscriber/FilterFormatSubscriber.php
   @@ -0,0 +1,54 @@
   +    if (!preg_match('`^filter\.format\.([a-z0-9_]+)$`', $config->getName(), $found) || !$event->isChanged('status')) {
   

   This is sad :(

   Can't we use hook_ENTITY_TYPE_presave()?

2. +++ b/core/modules/editor/src/Tests/EditorFilterIntegration.php
   @@ -0,0 +1,56 @@
   +  /*
   

   Nit: one missing asterisk :P

I would say that the sad part is that we cannot detect the configurable type in a subscriber like this. This makes the config events dispatching/subscribing so useless :(

Oh, yes, absolutely. Your patch is great, the API forcing us to do this is sad. I wonder if we're just missing something. But in any case, a cleaner solution exists, using hooks. And I suspect that's exactly why things work the way they do :)

Many thanks for working on this!

I have only nits:

1. +++ b/core/modules/editor/editor.module
   @@ -509,3 +511,30 @@ function _editor_parse_file_uuids($text) {
   +function editor_filter_format_presave(ConfigEntityInterface $format) {
   

   You can typehint to \Drupal\filter\Entity\FilterFormat :)

2. +++ b/core/modules/editor/editor.module
   @@ -509,3 +511,30 @@ function _editor_parse_file_uuids($text) {
   +  // The text format being created cannot have an editor yet.
   

   s/an editor/a text editor/

3. +++ b/core/modules/editor/editor.module
   @@ -509,3 +511,30 @@ function _editor_parse_file_uuids($text) {
   +  /** @var \Drupal\Core\Config\Entity\ConfigEntityInterface $original */
   

   Same typehint.

4. +++ b/core/modules/editor/editor.module
   @@ -509,3 +511,30 @@ function _editor_parse_file_uuids($text) {
   +  // If the text format status is the same, exit here.
   

   s/exit here/return early/

5. +++ b/core/modules/editor/editor.module
   @@ -509,3 +511,30 @@ function _editor_parse_file_uuids($text) {
   +  if (($status = $format->status()) == $original->status()) {
   

   Let's use strict equality.

6. +++ b/core/modules/editor/editor.module
   @@ -509,3 +511,30 @@ function _editor_parse_file_uuids($text) {
   +  /** @var \Drupal\Core\Config\Entity\ConfigEntityInterface $editor */
   

   \Drupal\editor\Entity\Editor

7. +++ b/core/modules/editor/src/Tests/EditorFilterIntegration.php
   @@ -0,0 +1,56 @@
   +    $editor = Editor::create(['format' => $format->id(), 'editor' => 'unicorn']);
   +    $editor->save();
   

   We don't use $editor anywhere. So can be simplified to Editor::create(…)->save() AFAICT :)

I was going to RTBC this, but then I realized that we probably also want to test the inverse direction: when re-enabling a text format, the text editor should be re-enabled also.

Other than that, this looks perfect :)

Wonderful! This is a great patch :)

Without a beta evaluation, this likely won't go in for some time though. But marking RTBC already, to signal that it is indeed ready.

This sounds like a bug actually, and maybe a bit of a data integrity issue even. What happens in HEAD if a text format with or without existing content is disabled and then someone tries to use the editor for it? What happens if it's disabled and then re-enabled? What happens if one or the other is disabled via a config deployment? Some manual testing of both HEAD and the patch would be good to see what the implications are and if there's a bigger bug lurking here. Anyway, recategorizing the issue so it gets more visibility.

Also, @alexpott and I agreed that this would probably need an upgrade path, particularly for existing sites where text formats had already been disabled, to ensure that the existing configuration is updated and consistent.

A nice integration of editor with text format would have been if Editor would write its settings directly into format as third-party settings. No more headache :)

Editor module predates the concept of third-party settings.

Upgrade path looks great to me. Just one nit that can be fixed on commit.

+++ b/core/modules/editor/src/Tests/Update/EditorUpdateTest.php
@@ -0,0 +1,75 @@
+    $editor_basic_html =  $config_factory->get('editor.editor.basic_html');

Nit: two spaces instead of one.

Heh, thanks :) It's fine to keep the patch at RTBC if you're only making whitespace changes ;) :)

This has test coverage and upgrade path and fixes a bug so permitted during beta. Committed 40fa14b and pushed to 8.0.x. Thanks!