Determine an upgrade path from CKEditor image button to media library

My perspective: Standard should remove DrupalImage and stop allowing <img>, and add DrupalMediaLibrary, the media_embed filter and allow <drupal-media>.

Awesome! I got here from #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library -- MUCH EXCITE.

One "concern or something", only because it's just come up for us in the last few weeks (so it seems EXTREMELY IMPORTANT ;-) ) -- if the recommendation ends up being to remove the "old" button (having two image buttons would indeed be confusing), just want to make sure folks with existing sites (and folks migrating content from D7 to D8 who might have simple <img> tags all over the place _raises hand_) still get to experience Media library + WYSIWYG Joy, without excessive (and maybe preventable?) pain.

...and/or maybe this is more about a desire for a path to convert <img> tags to <drupal-media> tags...? I'm not sure -- but if this "concern or something" belongs elsewhere (or nowhere), just say the word.

Thanks!

(EDIT: oops, used html tags in my comment)

still get to experience Media library + WYSIWYG Joy, without excessive (and maybe preventable?) pain.

They'd continue to have the old button but also get the new button.

A mitigation I can think of is to allow the "old" button (DrupalImage) to become configurable: we can make it only show up if and only if an <img> is already present. I think that would get us the best possible experience without making it impossible to edit existing content's non-media images.

more about a desire for a path to convert

We've at least historically never done such update paths in Drupal core because it's possible that a single Drupal site has millions of pieces of contents with tens of millions of revisions. Drupal's update system is not built for that!

Perhaps we can automate it as long as the amount of content is small enough. But then it's going to be one hell of a bikeshed to agree on where that limit lies. Plus, it depends on how powerful the server is how long each conversion would take. Which is exactly why we've never done such update paths in the past.

Plus, not every site will even want to update!

I know, it's a less-than-ideal answer. 😔

I think there's some good progress happening here towards a reasonable solution.

A mitigation I can think of is to allow the "old" button (DrupalImage) to become configurable: we can make it only show up if and only if an <img> is already present. I think that would get us the best possible experience without making it impossible to edit existing content's non-media images.

That sounds like a sensible idea, although within the context of manipulating an existing image, would it be necessary, I mean generally to manipulate an existing image you would right click on it and use that context menu to access the properties, would you still get that with the button not present?

That sounds like a sensible idea, although within the context of manipulating an existing image, would it be necessary, I mean generally to manipulate an existing image you would right click on it and use that context menu to access the properties, would you still get that with the button not present?

I'm glad you asked precisely that!

Let me start by ensuring we have the same understanding. Today, it's possible to edit an <img> (i.e. trigger EditorImageDialog) in three ways:

1. Double-click the image
2. Right-click the image, click "Edit image" in the context menu
3. Select the image (by clicking or tabbing to it), now click the "Image" button (keyboard users: press the enter key)

I recently found out that two very smart, long-time Drupal contributors and users both knew only about two of the three. And they each didn't know about a different one.

Which then leads to the question: what if a person only knows about the third one and is a mouse user? We'd break their workflow.

Which then leads to the question: what if a person only knows about the third one and is a mouse user? We'd break their workflow.

That's very clear and indeed a valid concern, thanks for explaining that :)

This is potentially a big important deal (and is a tricky problem space).

Wim Leers had previously written on this issue:

My perspective: Standard should remove DrupalImage and stop allowing <img>, and add DrupalMediaLibrary, the media_embed filter and allow <drupal-media>

I discussed this approach with @wim-leers and @phenaproxima on Slack:

I wrote: "Sounds very easy. It would be easy to land that in 8.8, if we can reach consensus. This would only affect new installs."

But then @phenaproxima explained that this is a long way off.

In order for any of that to work, Media Library would have to be enabled in Standard, and that’s a long way off.

So, it looks like for now, implementing an upgrade path is blocked (but not determining the eventual upgrade path). It's blocked until it's possible to add Media and Media Library to the Standard profile.

So perhaps a sub-issue to add documentation or some other interim solution should be added?

And we'll leave this open to discuss the eventual plan?

Here's an initial patch and test coverage.
It just enables media_library and media by default and replaces the drupalimage with druplamedialibrary, and enables the media embed filter. I left img in the allowed_html, for BC purposes.

When I was testing, I had removed those 'allowed_media_types', which are from #3073535-15: Allow only a subset of the media library to be exposed to CKEditor. Here's a combined patch with #11 and #3073535-15: Allow only a subset of the media library to be exposed to CKEditor. Let's see what fails!

If for some reason #3073535: Allow only a subset of the media library to be exposed to CKEditor never goes in, here's a version without the config it adds ('allowed_media_types').

Fixing tests

Fixing tests

Marking postponed per

In order for any of that to work, Media Library would have to be enabled in Standard, and that’s a long way off.

… but I can't even point to an issue, because there's no issue listed in #2834729: [META] Roadmap to stabilize Media Library to enable media_library.module by default in Standard!

Patch review:

1. 👏🥳 This looks great, and the functional test is a nice touch. I'm not sure we usually do this, but it definitely doesn't hurt, and we could still choose to omit this later on. It's great to have this written at a time when it's all still readily available in our head :)

+++ b/core/profiles/demo_umami/config/install/filter.format.basic_html.yml
@@ -48,3 +49,11 @@ filters:
+    weight: 100

+++ b/core/profiles/demo_umami/config/install/filter.format.full_html.yml
@@ -33,3 +34,11 @@ filters:
+    weight: 100

+++ b/core/profiles/standard/config/install/filter.format.basic_html.yml
@@ -42,3 +43,11 @@ filters:
+    weight: 100

+++ b/core/profiles/standard/config/install/filter.format.full_html.yml
@@ -33,3 +34,11 @@ filters:
+    weight: 100

🤓 Nit: why such super high weights? The existing weights are significantly lower.

The answer to why I used super high weights: I didn't even think about it. I was cutting and pasting. I will correct those.

Also, I think I might have some changes to demo_umami in there, and they should come out, as that would be a separate issue and will probably land sooner.

re: #21, I made a working patch #3086965: Allow embedding media in CKEditor in Umami

Ideally we'd figure this out before Drupal 10, because Drupal 10 will drop support for CKEditor 4. See #3231364: Add CKEditor 5 module to Drupal core.

The CKEditor 5 module already has the infrastructure it needs for this: it can provide an automatic upgrade path.

Up until now, this issue has been saying we need the media and media_library modules to be enabled. But … we could also just do that conditionally as a first step:

For Drupal 9|10 sites without media/media_library that switch to CKEditor 5

Keep the current behavior.

For Drupal 9|10 sites with media/media_library that switch to CKEditor 5

Switch to Media automatically!

This seems like a golden opportunity to at least move a big chunk of Drupal sites in the right direction!?

Thoughts? 🤓