🇺🇸United States @joshuami

Thanks, @alexpott. Added a merge request with the repeated function. Let me know if I got the naming convention correct. I assumed it should be "update_10301" as the first update required after Drupal 10.3, but maybe that should be "update_10300". 🤷

Quick note for anyone needing to run that command on an Acquia environment, I found that you need to escape the parenthesis when running the command through acli, acli remote:drush [application].[environment] -- php:eval '_scheduler_content_moderation_integration_add_fields\(\)' worked where application is the project code and environment is the CD, dev, stage, or prod.

It's a bit of a bummer to have to run it for every environment up the chain, but at least it unblocks the upgrade without persistent errors.

We really need another release that adds scheduler_content_moderation_integration_update_9004() calling the same _scheduler_content_moderation_integration_add_fields() function. Or is that too messy and we should just rename the 9001 update and move it to the 9004 position. 🤔

Any preferences from the maintainers? I'm happy to create the MR.

@sanket.tale, I didn't see the issue on views with menu settings set by a view. It was in views off of an entity type were the the tabs were added via a mymodule.links.tasks.yml that defined the tabs using the menu api per https://www.drupal.org/docs/drupal-apis/menu-api/providing-module-define... → .

This is similar to how the media module in core adds the views.view.media.yml to the system.admin_content base route:

entity.media.collection:
  title: Media
  route_name: entity.media.collection
  base_route: system.admin_content
  weight: 10

If you create the multiple views and add them via a custom module in mymodule.links.tasks.yml, I think you should be able to reproduce, but I haven't tried this on a view that has both primary and secondary menu tabs like /admin/content.

Updating issue summary with a GIF to demo the issue.

joshuami → created an issue.

joshuami → created an issue.

I've tested MR #10 and the dev branch with success as well. There are no tests in this module and the functionality is all there with multiple manual tests, so I guess this is RTBC. I tend to agree that getting a stable release would be ideal.

I do worry that the next release may break backwards compatibility with the contrib ckeditor module or possibly the paid CKEditor 4 version provided by CKSource.

I noticed that I cannot remove the composer dependency on drupal/ckeditor with the patch from this merge request. I don't see why not in the module's composer.json—which is a bit odd.

I also was curious about the ckeditor5_plugin_pack approach to CKEditor 5 templates. I guess the good news is that its implementation is very close to MR #10. The configuration of templates is nearly the same with a small difference in how the SVG is added via a field rather than as a file path.

The plugin widget has a neat little search feature, but I'm not a fan of the concatenated template descriptions. It does not show the full description in any way that I can find.

Until those clean up items get added to the ckeditor5_plugin_pack, I think there is still a place for this module, but afterwards, I would imagine most folks are going to settle on the module maintained by CKSource. 🤔

This issue occurs with the s3fs module as well and I can confirm that the patch correctly handles the error for that contrib module as well. Updating the issue summary to reflect that a core fix would solve the issue for more than just stage file proxy.

Would something like this need tests? The patch basically shifts the error to the error logs rather than disrupting page render. That seems reasonable.

@Falco010, there is a core patch for 'disable-refocus' => TRUE. I believe I've triggered that cursor refocus in more cases than just CKEditor5 as a part of using the media library on a entity reference field.

See #3415961 🐛 [drupalMedia] Using the Insert Media button causes the window to scroll to the bottom of the page Needs work .

Here is my attempt at the requested review from @mandclu. This issue is definitely a bit confusing to follow with both the patches and the merge request.

The latest patch in #146 applies cleanly to the latest release, 8.x-1.4. In my tests, it works well.

The merge request is against the 8.x-2.x dev branch. Which doesn't have a release and is not the default dev branch. I have not been able to get it to successfully install with composer.

The merge request seems to make some different choices than the latest patch other than the base branch.

• The format of the README.md is a little different (minor).
• The patch declares core_version_requirement: ^9.3 || ^10 || ^11 while the merge request shows core_version_requirement: ^8 || ^9 || ^10.
• The patch includes an update hook.
• The patch adds a permission called insert ckeditor templates. The merge request adds a permission called use text format advanced.
• The patch adds some language management that the merge request does not.
• The merge request adds an upcast and downcast in /js/ckeditor5_plugins/ckeditor_templates/src/ckeditorTemplatesCommand.js that adds the widget type around handles to add a new line before or after the inserted template.
• The merge request includes code formatting changes in /src/Plugin/CKEditorPlugin/CkeditorTemplates.php.
• The patch properly types attributes in /src/Entity/CKEditorTemplates.php but the merge request has a save() function in that same file that clears cache definitions when a template is saved.
• There are a few spots in the patch that set the form fields as text_format where the merge request opts for textarea and string.

@mandclu (or another maintainer), should that merge-request be against the 8.x-1.x branch? And if so, how would we resolve which of the slightly different approaches should make it into the accepted merge request.

The sweet spot for me would likely be a new merge request against 8.x-1.x that is mostly the patch from #146 with the addtion of the type around widgets getting added to the templates if those work well. I've not been able to test them as I personally haven't found a way to get the merge request to apply.

Thoughts?

itmaybejj → credited joshuami → .

I think UI-only enforcement would be sufficient.

That is a good question. I think just the CKEditor plugin would meet the need. Adjusting the media_embed filter for all possible configurations would get complicated quickly. And, there might be a need to support legacy content that used one of the disabled features.

A disallowed patterns filter would a pretty slick feature as well. It would be nice to strip empty elements like headings and paragraphs that editors always seem to be leaving behind. I'll have to think about writing that up as a separate issue. 😁

That should be literally impossible 😳

I was equally confused by that result.

To make sure it wasn't some oddity in the moderately complex site on which I was working, I tested the patch on a site closer to the standard install. The only additions I made were to add two new display modes for images that were not used by the other default media entity types.

Before applying the patch, an embedded document showed only the "Default" view mode:

After applying the patch, an embedded document showed all enabled view modes even though "Half width" and "Narrow" were not configured display modes for documents.

You're leaving me hanging there! 🤣 Where were you going with that? 😄

😆 At least you can tell it wasn't generated by AI.

That was a few days ago, so I can only guess at what I was going to say, but it would have been something like this. 🤔

It would be nice to be able to choose whether or not caption, link, display mode, and even alignment appear per each type of media. As a developer, I can put all sorts of things into a media entity display mode. Here are some examples of where I might want to prevent one of the default toolbar items.

Caption: I may choose to provide a caption rather than have CKEditor provide the caption functionality because I want to include license and attribution meta data in the caption.

Link: I might want to prevent editors from adding a link around an image to discourage the creation of gaudy image "buttons" that break from the style guide. I would definitely want to hide the link functionality from documents, audio, and video because it would break the functionality and lead to more support requests. (Sorry, that's two examples 😜)

Display mode: If there is only one display mode for a media type, I might want to hide that display mode to reduce the visual complexity for my editors.

Alignment: I might want to hide alignment from videos because I want my editors to always embed full-width videos and the align left and right forces a 50% width by default.

I can probably train editors to work around all of these scenarios, but if I could hide the feature I don't want them to use, they will be much happier and it lowers the chance of confusion.

I ran across this issue with a document media type that is configured to show translations on some display modes via an entity views attach. (I know it sounds complicated, but it was a pretty simple solution for a complex translation need. 😆) If the document was embedded, and then immediately translated, the preview does not update for a bit. I didn't time it, but the 5 minutes mentioned above sounds about right.

So I tried the patch in #9. It did invalidate the cache for the preview upon applying the patch, but it didn't really fix the caching issue on subsequent tweaks to the display mode.

The patch did have an unintended consequence after applying it. It showed all display modes as an option even if the media type did not have that display mode configured. I really don't understand how setting the cachetag checksum could have had that affect, but I thought I would report it for whomever might pick this up.

@jacobupal, I think you might be on to something, but the approach in patch #70 in CKEditor 5 support for Content Templates ✨ CKEditor 5 support for Content Templates Needs review creates a command called "ckeditorTemplates" in /js/ckeditor5_plugins/ckeditor_templates/src/ckeditorTemplatesCommand.js that is called in /js/ckeditor5_plugins/ckeditor_templates/src/ckeditorTemplatesEditing.js. That command is inserting a section around every template when embedded:

+          <section class="ckeditor-template-wrapper">
+            <div class="ckeditor-template-content">
+              ${htmlCode}
+            </div>
+          </section>

That section getting converted into a widgetEditable with the widgetTypeAround handles. I didn't test that patch in particular, but that's what I'm seeing in the code. It kinda looks like they were following the tutorial at https://ckeditor.com/docs/ckeditor5/latest/tutorials/widgets/implementin....

That said, it would be awesome if we could target any element + attribute combination to make it an editable block widget when in focus. A horizontal rule gets upcast into a widget, why can't we do that with other element + attribute combinations. It might fix some issues like the ability to edit a <cite> within a <blockquote>.

@Wim Leers, thanks for asking the clarifying questions.

Audio might have a caption, but it would be rare.

Okay, "rare" may have been an overstatement. 😄

Originally, I was thinking of documents as not really something you would semantically mark up as a figure, but you might have a point. Reading through the HTML 5 definition of flow content (https://www.w3.org/TR/2011/WD-html5-author-20110809/content-models.html#...), I guess it would be a correct usage to wrap a document media link in a figure and provide a figcaption as the description.

I've certainly implemented a description field on documents and used the display mode template to output a description with an aria-describedby on the document. There wasn't a way to add a caption to a document in earlier releases of media library with CKEditor 4, so I hadn't really given it thought to using it.

Generally, a little more configurability for what appears in the drupalmedia toolbar would be helpful. There are so many use cases were the

Updated the issue summary to focus on the link button as that is the larger issue for creating patterns that will break media players and document download links.

It's worth noting that nearly all of Media and Media Library's test coverage and development have been focused on images. The result is that new features for images tend to be added to all media types (whether they are appropriate or not). There are many more areas where adding or removing embed options would make other media types such as video, audio, documents, maps, charts (and more) significantly more usable and accessible. Overriding this experience was much easier in CKEditor 4 with the entity embed dialog (modal) as it allowed for Drupal form alters per media type. With CKEditor 5, this all has to be considered as part of the plugin architecture for the balloon toolbar that appears on each embedded media item.

joshuami → created an issue.

In a project using this patch, we just discovered an edge case that wasn't covered when scheduling media. It only occurs when creating new media, but the publish status sort weight changes back to 53 when the error is triggered before the media entity is saved. I can't reproduce this for content entities, so it may be an issue upstream in core media. I'll try an dig into this a little more as time permits.

@mark_fullmer, can you point to the commit that introduced the bug for you? I was fairly certain it was introduced in 3396628 🐛 Fix → native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by Fixed , which added the validation that compared source editing to the currently enabled plugins—among some other changes related to lists.

I'm not able to reproduce the error in a site running 10.1.6 and CKEditor 5.

I have been able to duplicate this issue on a site that used a combination of ckeditor_bs_grid and ckeditor_templates. Any template added to a grid where the template included divs—such as adding a Bootstrap card component—resulted in data loss on edit. For some reason, the duplicate ck-widget classes would trigger the removal of the div with col class immediately following the row.

It's an odd enough behavior that I can't quite track down the source. I'm adding a comment in case anyone else is seeing similar.

@KlemenDev, this is not a duplicate and the issue summary is not fixed by 3410303 🐛 FilterHtml data loss when iframe and/or textarea is allowed Active . The problem is that you can't save a valid configuration for source editing after 10.2.

Unless we want to reopen issue #3396628 which I mention in comment 12 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review , we still have a validation issue which forces resolution of errors on the text format creating a state that will cause data loss upon editing existing content.

For any element that has a valid plugin, the current validations requires you remove the element and its classes from source editing. This is a problem, because not every plugin covers every attribute that may be needed in an existing site.

This is currently a hard blocker for upgrading a site from CKEditor 4, which allowed pretty much any source editing pattern that you desired, to CKEditor 5, which now requires new plugin for every possible pattern in source editing.

@Wim Leers, I think #3396628 may be a separate issue.

What I was experiencing was definitely related to the changes in SourceEditingRedundantTagsConstraintValidator.php as part of #3396628. At https://git.drupalcode.org/project/drupal/-/commit/ec24a58c6dd7e7d3a1ead..., the validation is comparing elements enabled in source editing to all other enabled plugins. That check prevents someone from adding an attribute like class to anything that might be covered by another plugin that also enables aspects of that element.

The error message I was getting that prevented saving a valid config is The following @element_type(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: %overlapping_tags.

Here's an example:

We had a couple of CKEditor 4 plugins that we wrote to support adding phone numbers and email addresses with an enforced format and classes that allowed them to be editable and styled. That code required the following to be enabled in source editing: <span class="cke-phone cke-email">. (Note: this is a temporary need to avoid data loss until we can write a proper CKEditor 5 solution.

When we attempt to save the text format after the 10.2 upgrade, we get the error message:

The following attribute(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Style (<span class="cke-phone cke-email">).

That is directly coming from $enabled_plugin_elements_optional, but the only other plugin enabled that affects span is the Language selector. Clearly, we are not going to get the ability to save those classes from a completely unrelated plugin, but the overlap filter is preventing span from being in source editing.

I propose we remove the overlap optional filtering so that source editing can truly allow the developer to say "I know I don't have another plugin for this so I want to allow it in source just to prevent data loss until I can write something way more complex and robust (possibly never 😆)".

To put this another way, source editing should allow us to define exceptions that are not covered by another plugin rather than forcing us to write a plugin from scratch.

Wim Leers → credited joshuami → .

joshuami → changed the visibility of the branch 3396628-tighten-sourceeditingredundanttagsconstraintvalidator to hidden.

Added some clarification to the issue summary. The text filter won't cause data loss until saving the format after resolving all errors to allow saving.

Hmm. I was adding <span lang dir> to source editing. Just adding <span> does work. Thanks for having me double check @ericgsmith.

I still think it might strip the declaration upon saving after an upgrade because of the new rules around plugin requirements, but at least you can sort of use the language selector again.

Alright... another use case with data loss. The language selector no longer works with any core configuration.

If you try and add language to the toolbar, it gives you an error that you need another plugin to add <span>, but if you try to enable span via source editing, it throws an error that it is already enabled via language.

Has anyone used the patch with Drupal 10.2? I'm curious whether the change introduced in 3396628 🐛 Fix → native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by Fixed impacts the ability to create and use a template with <div> and classes.

joshuami → created an issue.

lostcarpark → credited joshuami → .

And here we go. New patch with new line numbers.

It's still needed. Just needs a reroll. #3359796: Skip form displays that don't exist. → added some lines above the function the patch was updating. I'll see if I can get a new patch posted today or tomorrow.

While troubleshooting a new site with media entity download where path aliases were not working—I came across this issue. It seemed odd as several of our other D8+ (all D9 or D10 now) were working with the path alias instead of the /media/id/download. It turns out that we nearly always use subpathauto, which solves the issue of having a media alias path with media entity download quite well. Posting in case this unblocks anyone else looking for a solution in this space. 😅

@kevinquillen, I think the only way to get it to work at the moment is to composer require drupal/simplesamlphp_auth:^4.0@dev. That worked for us, but I'm keen to see the full release sooner than later as I don't like pinning to a dev commit when I can avoid it.

Oh, I didn't think about v4. I've been so focused on Radix 5 and Bootstrap 5 for the last couple of builds that I forgot what it used to look like. 😆

Interestingly, Bootstrap 4.6 does have root variables, but looking at https://git.drupalcode.org/project/radix/-/blob/8.x-4.x/src/kits/default..., I don't see the _root.scss getting called. I think that might be a miss in Radix 4. 🤔

In addition to documentation, it might be worth adding a CKEditor5 stylesheet to Radix and refactoring the way the dependencies are called in to allow the root variables.

I really think most themes (or at least generated subthemes) should come with a ckeditor5-stylesheet built in now. It would certainly make it easier for people to understand how that particular functionality works.

@jwilson3, great write up! The only catch that I see is that the way the Radix subtheme pulls in the Bootstrap Sass files, the root.scss files would be prefixed with .ck-content, which can cause the variables in that file to no longer render. So all the color and size overrides that Bootstrap offers, or the variables that are overridden would not appear in CKEditor.

To get around this in one of my Radix themes, I restructured the Radix way of pulling in Sass quite a bit so that the subtheme's myradixsubtheme.style.scsss and ckeditor5.style.scss both pull in those root variables without the .ck-content prefix. So the ckeditor5.style.scss would look a little like this:

// ------------------------------------------------------------------------------------------
// Subtheme CSS overrides for CKEditor 5
//
// This stylesheet is applied to admin pages with a CKEditor 5 WYSIWYG editor. Use it to set
// or override styles of content within the editor.
// ------------------------------------------------------------------------------------------

// ------------------------------------------------------------------------------------------
// Import all the functions and variables.

// 1. Include functions first (so you can manipulate colors, SVGs, calc, etc)

@import '~bootstrap/scss/functions';

// 2. Include any default variable overrides here
@import 'base/variables';

// 3. Include remainder of required Bootstrap stylesheets
@import '~bootstrap/scss/variables';

// 4. Include any default map overrides here
// We don't have any in Westy

// 5. Include remainder of required parts
@import '~bootstrap/scss/maps';
@import '~bootstrap/scss/mixins';
@import '~bootstrap/scss/root';
@import '~bootstrap/scss/utilities';

// ------------------------------------------------------------------------------------------
// Preface all Subtheme styles with .ck-content to restrict them to the content within CKEditor
.ck-content {

  // 6. Optionally include any other parts as needed
  // We've chosen to comment out includes not in use.
  @import '~bootstrap/scss/reboot';
  @import '~bootstrap/scss/type';
  @import '~bootstrap/scss/images';
  @import '~bootstrap/scss/containers';
  @import '~bootstrap/scss/grid';
  @import '~bootstrap/scss/tables';
  @import '~bootstrap/scss/forms';
  @import '~bootstrap/scss/buttons';
  @import '~bootstrap/scss/transitions';
  @import '~bootstrap/scss/dropdown';
  @import '~bootstrap/scss/button-group';
  @import '~bootstrap/scss/nav';
  @import '~bootstrap/scss/navbar'; // Requires nav
  @import '~bootstrap/scss/card';
  @import '~bootstrap/scss/breadcrumb';
  @import '~bootstrap/scss/accordion';
  @import '~bootstrap/scss/pagination';
  @import '~bootstrap/scss/badge';
  @import '~bootstrap/scss/alert';
  // @import '~bootstrap/scss/progress';
  @import '~bootstrap/scss/list-group';
  @import '~bootstrap/scss/close';
  // @import '~bootstrap/scss/toasts';
  // @import '~bootstrap/scss/modal'; // Requires transitions
  // @import '~bootstrap/scss/tooltip';
  // @import '~bootstrap/scss/popover';
  // @import '~bootstrap/scss/carousel';
  // @import '~bootstrap/scss/spinners';
  // @import '~bootstrap/scss/offcanvas'; // Requires transitions
  // @import '~bootstrap/scss/placeholders';
  @import "~bootstrap/scss/helpers";

  // Subtheme Base
  // ------------------------------------------------------------------------------
  @import 'base/helpers';
  @import 'base/elements';
  @import 'base/typography';

  // Subtheme Layouts
  // -----------------------------------------------------------------------------
  @import "layout/page.header";
  @import "layout/page.content";
  @import "layout/page.footer";

  // Subthteme Components
  // -----------------------------------------------------------------------------
  @import "../components/nav/nav";
  @import "components/accordions";
  @import "components/alerts";
  @import "components/badges";
  @import "components/buttons";
  @import "components/cards";
  @import "components/callouts";
  @import "components/form";
  @import "components/item-list";
  @import "components/list";
  @import "components/navs";
  @import "components/office-hours";
  @import "components/pagination";
}

// 7. Optionally include utilities API last to generate classes based on the
// Sass map in `_utilities.scss`
@import '~bootstrap/scss/utilities/api';

My myradixsubtheme.style.scsss, looks the same as above, but without the prefix section. I bet there is a better way to do that so that I'm not maintaining two nearly identical *.style.scss files. 🤔

@ckng, if you use your subtheme's built CSS without any prefixes, and you use the Claro admin theme, you will likely see some conflicts between Bootstrap and Claro because they share a couple of classes. This can cause the form to have some weird layout. It also means that your font in Claro will match your frontend—that might not be an issue depending on your font choices.

+1 on the RTBC. I also had the caching issue, and I *think* it is related to https://www.drupal.org/project/drupal/issues/3370828 🐛 Ensure that edge caches are busted on deployments for css/js aggregates Fixed .

As of Drupal 10.1.2, the cache tag on CSS and JS libraries are only going to get updated if there is a library version change. If the libraries file were updated to have a set version per release (or included in a patch for future release) it should make the cache tag parameter reset—whether for a local build with aggregation turned off or for a production environment with aggregation turned on.

I was able to get around this by including the patch in a build that also updated to Drupal 10.1.3—which has version changes that trigger the new cache tag.

joshuami → created an issue.

After a bit more research with some sites I'm trying to upgrade to D10 with CKEditor5, I think this may be more of a potential data-loss bug than a task.

If a site has used Media Library and embedded media into a list (ordered or unordered doesn't matter), those lists will be corrupted on the first edit of the entity after enabling CKEditor5.

While Document Lists (see my previous comment) in CKeditor5 allow paragraphs and images to be inserted into a list—retaining the nesting of those elements within the list item—you cannot do the same with a drupal-media element. I'm not immediately sure why as you can technically manually add divs. I think it might be because drupal-media elements are not explicitly allowed within CKEditor's DocumentList allowed HTML.

I'm not seeing the same issue with tables. You can have a drupal-media element with a table cell and it doesn't force that to break out of the table cell in the way that it forces the list to break into two list elements.

Can anyone else reproduce this? Do you agree it would be data loss since there is no path to editing the content that has this pattern?

Just tested on a Drupal 10.1 site with the patch against a remote video and a document embed. In both cases, clicking the media item triggers the tooltip instead of triggering the interaction with the media item (playing the video or downloading the document).

Thanks for jumping in on this one @lauriii!

I need to spend a bit of time figuring out how to actually create a merge request in an issue. Adding a patch for now.

joshuami → created an issue.

Adding some use cases to this issue.

1. An editors uses media library to embed a document into an unordered or ordered list.
2. An editor uses media library to embed an image into a list.

CKEditor 5 supports this per https://ckeditor.com/docs/ckeditor5/latest/features/lists/document-lists....

@Wim Leers, do you think we could add 3283173 📌 Document and edge cases in \Drupal\ckeditor5\HTMLRestrictions::getTextContainerElementList Active to this list.

CKEditor 5 seems to support block level elements within a list item or table cell, but Drupal's implementation is stripping this valid syntax per HTML 5.

See https://ckeditor.com/docs/ckeditor5/latest/features/lists/document-lists....

Or am I thinking of this incorrectly, it's technically not blocked upstream, it's blocked in our implementation. 🤔

Thanks, Wim. I posted a comment (and a comment on my comment). I appreciate the link to the specific CKEditor 5 change record → .

Sheepishly, I have to admit that I'm not good at following change records—there are just so many of them.

I read release notes—though not all as I don't tend to read the release notes for a branch I'm not currently using. I like xjm's suggestion to include some reference to these known issues in release notes and to the Drupal 10 upgrade handbook → .

Incidentally, I could only find that handbook by searching, it might make sense to include references to that upgrade process in the next few release notes as well. Upgrade instructions are more important information than linking to the release cycle. (Or at the very least the release cycle should link to those upgrade instructions.)

In the update contributed projects → section of that handbook, it makes it sound like upgraded modules will just be compatible, but I've found several modules that have a version compatible with Drupal 9 or Drupal 10 but not both. I feel like we might need a bit of documentation to help module maintainers understand that it is best practice to have a release that is compatible with both major versions at each end of life transition to aid in the upgrade process.

Maybe we could add a bit to the Drupal 9 compatibility section of the → add a composer.json file → page in the creating modules handbook → .

Another thought that I touched on in Incorrect version of CKEditor being parsed from info.yml files 🐛 Incorrect version of CKEditor being parsed from info.yml files Active , it might be good to link to some documentation that suggests possible upgrade paths in the warning.

Resolving the Composer dependencies to get the contrib ckeditor module is place is not straightforward. At a minimum, a link to the change record → , the composer issue 🐛 Incorrect version of CKEditor being parsed from info.yml files Active , and the deprecated and obsolete extensions page → would be helpful.

From a release standpoint, it is even possible to get this sort of warning included in Drupal 9 at this point? Or is it only possible for us to alert the developers after they've tried to upgrade to 10 or run Upgrade Status → checks?

+1 to adding some sort of notification to status report that would show for both Drupal 9.5 and Drupal 10 installations.

The change record by itself is not adequate to alert less experienced devs that they have a blocker to upgrading to Drupal 10 when using core's ckeditor module versus the contrib module.

My only recommendation would be to display the information as a warning rather than an error. I know that the clients I've helped establish CI/CD practice have tests that fail if "no errors found" is not in the status page. Could this be an error in Drupal 10 and a warning in Drupal 9? It would allow continued development not related to CKeditor and Core upgrades without breaking typical site-specific tests.

I'm trying to help a couple clients through their CKEditor 5/Drupal 10 upgrades and have come to the conclusion there are two paths:

Option 1: While on Drupal 9, upgrade to CKeditor 5, then upgrade to Drupal 10.

Option 2: Upgrade to Drupal 10 using the contrib ckeditor module, then upgrade to CKEditor 5 at a later date.

Either option is going to require some custom work to replace functionality from CKEditor 4 contrib modules (ckeditor_acr 1.0.1, ckedtior_config 8.x-3.1), some custom CKEditor 4 widgets (for inserting a phone number, email address, columns, accordions), and some code that relied on the MediaEmbedDialogue form and the ability to edit a media item inline and change attributes.

Of the two approaches, I would prefer option 2. Upgrading core to Drupal 10 is a larger risk. The smaller risk of not having our CKEditor 4 functionality replicated is not ideal—replicating the functionality is a fair bit of work—but at least it would *just* be CKEditor.

I was just about to post that there does not seem to be a valid way to add the contrib ckeditor to the latest Drupal 9 release, but then I happened on a successful order of operations.

Running composer remove drupal/ckeditor_acr or composer remove drupal/ckeditor_config gives me an error like ckeditor_acr is not required in your composer.json and has not been removed.

If I manually remove ckeditor_acr and ckeditor_config from my composer.json, and run composer update --lock, the files for those two modules are removed. When I then try to composer require drupal/ckeditor:'^1.0', it adds the dependency to the composer.json file, but not to the composer.lock file. A new composer install gives the error:

- Required package "drupal/ckeditor" is not present in the lock file.
This usually happens when composer files are incorrectly merged or the composer.json file is manually edited.
Read more about correctly resolving merge conflicts https://getcomposer.org/doc/articles/resolving-merge-conflicts.md
and prefer using the "require" command over editing the composer.json file directly https://getcomposer.org/doc/03-cli.md#require-r

It turns out that I also needed, to remove the following:

  "provide": {
    "drupal/ckeditor": "*"
  },

This snippet was recommended in 3309026 → to stop ckeditor_config's requirement of drupal/ckeditor from downloading the contrib module unexpectedly. I was able to require, enable, and reconfigure ckeditor_acr and ckeditor_config after successfully switching to the CKEditor 4 contrib module.

If anyone has a simpler upgrade path, please share. If anyone thinks this comment belongs in another queue, let me know.

The improvements to images embedded via CKeditor 5 are impressive, but all other media types suffer a little bit from an upgrade to CKeditor 5.

More use cases for linked media within CKeditor 5 are embedding documents, videos, and audio that are all interactive. I'm not sure the media module is the correct place to address whether media is clickable within the text editor. Right now the touch target for showing the contextual toolbar on a media item is a thin line around the image, document, video, etc. If you miss that line and click the media, you get one of two experiences:

Unlinked Media

If the media is an image without a configured linked display, you get the contextual media toolbar.

This is pretty much what an editor would expect.

Linked Media

If the media is a remote video, clicking on the media will play the video.

If the media is a document, clicking on the media will download the document.

For any media type other than image, the loss of an "edit media" button that has a decent touch target is a huge loss. The loss of an embed dialog that allows you to configure and possibly replace the media seems to also be the result of only looking at media management from an image perspective and not considering the embed complexities for other media types.

It would be a better experience, if a user could interact with a preview of the media entity where the entire embedded item is a trigger for the contextual toolbar. This would be similar to the link contextual toolbar which opens when you click the link rather than taking you to the link destination.

Updating the issue summary with the additional use cases.

For my client's use case, this is fixed in Drupal 10 with #2807629. If you need it for D9, use the patch from comment 104.

I can verify this patch does indeed fix the action to use the correct permission. The tests are failing because the test doesn't expect the permission in ViewsBulkEditModifyEntityValuesTest.