"Embed media" filter allows invalid config of default view mode not being part of view modes selectable

Created on 25 April 2022, over 2 years ago

Updated 28 July 2023, over 1 year ago

Problem/Motivation

Discovered in #3245720: [drupalMedia] Support choosing a view mode for → and split from #3276670: Some configurations of allowed view modes cause CKE to fail to initialize → .

The media embed filter settings validation logic is inadequate and needs to be hardened because it currently allows the user to save a configuration of a default view mode being a view mode that is not a selectable view mode. This has a consequence of making CKEditor 5 fail to initialize after saving.

AFAICT \Drupal\media\Form\EditorMediaDialog::buildForm() also only allows selecting the selected view modes, with no fallback behavior to automatically include the default view mode. Which makes no sense. In other words: the validation logic of \Drupal\media\Plugin\Filter\MediaEmbed::settingsForm() is inadequate. Therefore this is a bug in the media module, and deserves a separate issue there.

— from #3276670-5: Some configurations of allowed view modes cause CKE to fail to initialize →

A workaround will be added in #3276670: Some configurations of allowed view modes cause CKE to fail to initialize → , so the editor does not crash but the validation in the media module should be hardened in this issue to prevent this invalid configuration.

Steps to reproduce

Go to a text format's configuration form
Enable the "Embed media" filter
Select something as the Default view mode
Do not check that view mode in the "View modes selectable in the 'Edit media' dialog section
Return to editor (editor should fail to load, you should see error in console)

Proposed resolution

Remaining tasks

TBD
Remove the work-around that #3276670: Some configurations of allowed view modes cause CKE to fail to initialize → added, as also pointed out in #6.

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report

Status

Needs review

Version

11.0 🔥

Component

Media →

Last updated about 6 hours ago

Maintained by
🇳🇱Netherlands @seanB
🇩🇪Germany @chr.fritsch
🇺🇸United States @phenaproxima
🇪🇸Spain @marcoscano

Created by

🇺🇸United States hooroomoo

Live updates comments and jobs are added and updated live.

validation

Usability
Makes Drupal easier to use. Preferred over UX, D7UX, etc.

Site builder experience (SX)

Incomplete comments

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Comment over 1 year ago →
🇳🇱Netherlands Lendude Amsterdam
First stab at this.

Not sure if this is the best place to validate this.

Not sure about the wording of the error.

Left some of the test coverage in that might still be helpful

Added test coverage in a spot that feel a bit weird, but that whole tests feel a bit bit unfocussed so figured I'd start there

Do we need an upgrade path for this? I don't think so right? Because the workaround will have just added the default view mode to config already.
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.2 & MySQL 8
last update over 1 year ago
29,841 pass, 7 fail
Comment over 1 year ago →
🇳🇱Netherlands Lendude Amsterdam
duh...
Comment over 1 year ago →
System Message
The last submitted patch, 9: 3277049-8.patch, failed testing. View results →

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024