Problem/Motivation

It was added in #3216244: Remove CKEditor5PluginContextualInterface and CKEditor5PluginContextualValidationInterface in favor of annotation, days after the validation constraint refactor landed. It added form-level validation.

But it only worked at the time thanks to a bug: $plugins = $this->pluginManager->getEnabledDefinitions($submitted_editor); did not actually return all enabled definitions, but …

It never worked, because it starts from

$plugins = $this->pluginManager->getEnabledDefinitions($submitted_editor);

… and then it goes on to check the reasons why that plugin was NOT enabled. But … it already is … and so … it never showed a validation error!

We just never noticed because this was introduced in a turbulent time: lots of changes were in flight, and progress was more important >2 months ago. Crucially, there was no test coverage 🤔😨

Steps to reproduce

The reason it makes sense for this be a form-level validation constraint only is because it actually was designed to ensure that all items that the user put in the toolbar will actually work based on their conditions.

  1. For example: placing the image button required the image upload status to be enabled, otherwise there's no point to having that button. But it would still result in a functional CKEditor 5 instance: it just won't have the image button there.
  2. For example: placing the media library button required the media filter to be enabled, otherwise there's no point to having that button. But it would still result in a functional CKEditor 5 instance: it just won't have the media library button there.

The problem with both is that the admin UI shows something different than the resulting CKEditor 5 instance.

Proposed resolution

Convert these to a validation constraint that validates conditions for plugins loaded by active toolbar items: if the toolbar item is active, it SHOULD show up.

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork ckeditor5-3231220

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:

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

Status: Active » Reviewed & tested by the community

It's green. It's adding test coverage. It found a flaw/oversight in our existing functional tests. It's fixing something we introduced months ago.

@bnjmnm is the other person who's been reviewing the validation constraints. But because we'd like to open the core merge request ASAP — preferably today — I'm going to go ahead and self-RTBC and merge this request.

  • Wim Leers committed 73cadf6 on 1.0.x
    Issue #3231220 by Wim Leers: Follow-up for #3216244: AdminUi::...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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