Problem/Motivation

Discovered in #3245720: [drupalMedia] Support choosing a view mode for <drupal-media> 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

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

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#9 3277049-8.patch5.29 KBlendude

Comments

hooroomoo created an issue. See original summary.

hooroomoo’s picture

Issue summary: View changes
hooroomoo’s picture

Issue summary: View changes
hooroomoo’s picture

Issue summary: View changes
wim leers’s picture

Title: Embed media allows invalid config of default view mode not being part of view modes selectable » "Embed media" filter allows invalid config of default view mode not being part of view modes selectable
Issue summary: View changes
Issue tags: +validation, +Usability, +Site builder experience (SX)

Thanks for the write-up!

Improved markup/clarified a few things.

catch’s picture

This issue should also remove the workaround and test coverage added in

core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/Media.php
core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php

(see https://git.drupalcode.org/project/drupal/-/merge_requests/2161/diffs#di...)

wim leers’s picture

Issue summary: View changes

Yep! Documented explicitly now.

lendude’s picture

Status: Active » Needs review

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 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.
lendude’s picture

Version: 10.0.x-dev » 11.x-dev
StatusFileSize
new5.29 KB

duh...

Status: Needs review » Needs work

The last submitted patch, 9: 3277049-8.patch, failed testing. View results

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.