Problem/Motivation

It is not necessary to storage information about disabled filters in the config system, because they describe state of system where the config is saved and pollute the config system.

It is also impossible to release a dependency on a module or plugin by disabling it from the filter.

Steps to reproduce

  1. Install Standard.
  2. Navigate to /admin/config/development/configuration/single/export and export the Full HTML text format
  3. Observe that filter_html is present (but has status: false) even though it is not enabled

Proposed resolution

Filter disabled filters before config save.

Remaining tasks

Tests?

User interface changes

API changes

CommentFileSizeAuthor
#26 2385047-26.patch7.61 KBwim leers
#24 interdiff_23-24.txt2.74 KBcodebymikey
#24 drupal-filter-remove-disabled-filters-2385047-24.patch3.35 KBcodebymikey
#24 interdiff_23-24-9.2.x.txt1.22 KBcodebymikey
#24 drupal-filter-9.2.x-remove-disabled-filters-2385047-24.patch3.26 KBcodebymikey
#23 drupal-filter-9.2.x-remove-disabled-filters-2385047-23.patch2.3 KBcodebymikey
#23 drupal-filter-remove-disabled-filters-2385047-23.patch2.39 KBcodebymikey
#5 remove-disabled-filters-2385047-5.patch570 byteskarlshea
#1 remove-disabled-filters-2385047.patch653 byteswebflo

Issue fork drupal-2385047

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

webflo’s picture

webflo commented
StatusFileSize
new remove-disabled-filters-2385047.patch653 bytes

Status: Needs review » Needs work

The last submitted patch, 1: remove-disabled-filters-2385047.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented
Issue summary: View changes
StatusFileSize
new remove-disabled-filters-2385047-5.patch570 bytes

Reroll on 8.2.x, the array_filter needed to happen after parent::preSave().

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: remove-disabled-filters-2385047-5.patch, failed testing.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
catch’s picture

catch
he/him
Primary language English
commented

I just checked a couple of config exports, and I couldn't find a text format with a disabled filter, the exports only had the filters that were explicitly enabled - how do you end up with a disabled filter in configuration?

I did notice that each filter has status: true, which isn't in the config schema. So if we still save disabled filters in some situations, we should also remove this key as part of removing them from the exports since it'll no longer be necessary.

catch’s picture

catch
he/him
Primary language English
commented
Title: Remove disabled filters from config » Remove disabled filters from text format configuration
Issue tags: -Configurables, -CMI +Needs issue summary update, +Needs tests, +Needs steps to reproduce
quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: -Needs steps to reproduce +Bug Smash Initiative
Related issues: +#2502637: Disabled text formats can't be seen in the GUI

Adding steps to reproduce and a related issue.

In the related issue the filter will not be removed, it can be re-enabled. That seems far better to me than deleting it.

What is the reasoning for deleting the filter?

quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: +Needs steps to reproduce

I was completely wrong on this, ignore my previous comment. I am removing changes to the IS and restoring tag.

catch’s picture

catch
he/him
Primary language English
commented
Category: Bug report » Task

Agreed with trying to clean this up, but it's a task rather than a functional bug.

codebymikey made their first commit to this issue’s fork.

codebymikey opened merge request !2026

codebymikey’s picture

codebymikey commented
StatusFileSize
new drupal-filter-remove-disabled-filters-2385047-23.patch2.39 KB
new drupal-filter-9.2.x-remove-disabled-filters-2385047-23.patch2.3 KB
codebymikey’s picture

codebymikey commented
StatusFileSize
new drupal-filter-9.2.x-remove-disabled-filters-2385047-24.patch3.26 KB
new interdiff_23-24-9.2.x.txt1.22 KB
new drupal-filter-remove-disabled-filters-2385047-24.patch3.35 KB
new interdiff_23-24.txt2.74 KB

Ensure that the filters are also sorted by weight so they're consistently stored.

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

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should 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.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Version: 9.4.x-dev » 9.5.x-dev
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs issue summary update, -Needs tests, -Needs steps to reproduce
StatusFileSize
new 2385047-26.patch7.61 KB

Tests are present already.

Added steps to reproduce.

I'd RTBC the MR, but it needs to be rebased on top of 9.5.x … but then again we'll also need this to get committed to 10.0.x. So I just converted @codebymikey's MR to a patch that applies to both 9.5.x and 10.0.x 👍

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs update path 
+++ b/core/modules/filter/src/Entity/FilterFormat.php
@@ -206,6 +206,13 @@ public function preSave(EntityStorageInterface $storage) {
+    // Remove disabled filters.
+    $this->filters = array_filter($this->filters, static function ($filter) {
+      return !empty($filter['status']);
+    });
+    // Sort the filters by weight for consistency.
+    uasort($this->filters, 'Drupal\Component\Utility\SortArray::sortByWeightElement');

This will need an update function to process existing configuration to remove disabled filters.

Plus sorting of configuration should ideally by done by schema and sorting by the key is better than by weight as sorting by weight produces impossible to read diffs when only weight is changing.

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.

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.

eric_a’s picture

eric_a commented
Issue tags: -Bug Smash Initiative
Related issues: +#3017054: Consistently sort filter formats to simplify config exports

This now needs a small cleanup after #3017054: Consistently sort filter formats to simplify config exports.
As for the needed update function:filter_post_update_sort_filters() from that issue must be perfect example code.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Priority: Normal » Major
Issue tags: +Configuration schema, +validation, +blocker, +Needs update path tests

This will help make #3421946: [PP-2] Make FilterFormat config entities fully validatable simpler.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

AFAICT this should also change 

# Filter with module and status.
filter:
  type: mapping
  label: 'Filter'
  mapping:
    id:
      type: string
      label: 'ID'
      constraints:
        PluginExists:
          manager: plugin.manager.filter
          interface: 'Drupal\filter\Plugin\FilterInterface'
    provider:
      type: string
      label: 'Provider'
    status:
      type: boolean
      label: 'Status'
    weight:
      type: integer
      label: 'Weight'
    settings:
      type: filter_settings.[%parent.id]

to:

filter:
  type: mapping
  label: 'Filter'
  mapping:
    id:
      type: string
      label: 'ID'
      constraints:
        PluginExists:
          manager: plugin.manager.filter
          interface: 'Drupal\filter\Plugin\FilterInterface'
    provider:
      type: string
      label: 'Provider'
    weight:
      type: integer
      label: 'Weight'
    settings:
      type: filter_settings.[%parent.id]

… because all entries in filter.format.*:filters will have status: true.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Also: \Drupal\ckeditor5\Plugin\CKEditor5PluginManager::isPluginDisabled() can be simplified thanks to this change:

diff --git a/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php b/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php
index 5f900126b1..5b1391a925 100644
--- a/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php
+++ b/core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php
@@ -483,10 +485,7 @@ protected function isPluginDisabled(CKEditor5PluginInterface $plugin, EditorInte
         case 'filter':
           $filters = $editor->getFilterFormat()->filters();
           assert($filters instanceof FilterPluginCollection);
-          if (!$filters->has($required_value) || !$filters->get($required_value)->status) {
-            return TRUE;
-          }
-          break;
+          return !$filters->has($required_value);
 
         case 'requiresConfiguration':
           $intersection = array_intersect($plugin->getConfiguration(), $required_value);
wim leers’s picture

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

bbrala made their first commit to this issue’s fork.

bbrala changed the visibility of the branch 2385047-remove-disabled-filters-11.x to hidden.

bbrala changed the visibility of the branch 2385047-remove-disabled-filters-10.2.x to hidden.

bbrala’s picture

bbrala
Location Netherlands
commented

This issue confuses me, there was a patch, but looking at the current MR i see quite a few things from there missing. This issue needs some investigating to see if we are not missing loads.

Not unexpeceted since its so old :)

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.