๐Ÿ‡บ๐Ÿ‡ธUnited States @joshuami

Portland, OR
Account created on 16 February 2009, about 15 years ago
#

Recent comments

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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. ๐Ÿ˜

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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>.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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 ๐Ÿ› CKEditor5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Active , 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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

joshuami โ†’ changed the visibility of the branch 3396628-tighten-sourceeditingredundanttagsconstraintvalidator to hidden.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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. ๐Ÿ˜…

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

+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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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?

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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!

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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....

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

@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. ๐Ÿค”

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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 โ†’ .

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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?

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

+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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States joshuami Portland, OR

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.

Production build https://api.contrib.social 0.61.6-2-g546bc20