[META] Support WYSIWYG embedding of media entities

Copied more elaborate proposed solution from DrupalCon Dublin media meeting minutes.

Even though it could be more complex, I think the first option it's the best solution in the end. Embedding entities (of all types) is a very valid use case people will be happy with. If entity embed is something that should be in core, doing it now would make a lot of sense.

The most important thing I see right now is a possible upgrade path. Updating the way things are stored in WYSIWYG could be hard, if not impossible. If we are sure people would be able to fully benefit from the entity embed goodness when it does get in, it could probably wait.

I think the issue summary is far too vague for me to give feedback on this. It's not at all clear to me what both option entails exactly. What concrete consequences would there be? Changes, or only additions?

@wim-leers I've updated the summary to be more specific about concrete consequences of both options, based on my understanding of the issue. Based on the tone in the original summary, I would go with option #2, as it will get us the functionality we need without any new APIs to review.

Hi!

I'm sorry to have nothing to contribute to as far as technical choices are concerned, however I can stress the needs "as user" if that could help choosing the development approach.

It's very simple in fact.
1. Being able to create any View to use as "prefilter" (exactly the same as with an entity reference field).
2. Have a "Entity profile" that makes the glue between a specific view and a specific button in CKeditor (a bit like Linkit profile module).
3. When we click on that button, having a popup with a) Reuse Entity b) Create Entity (which redirects to the specific entity form, and on creation submit "autoreturns" the correct embedding code).
4. Embed the result as a code that will be rendered through a filter.

Otherwise said, I really don't think that most projects will need the overwhelming complexity of current Entity Browser configuration + Embed configuration. Especially when nothing seems to work as intended (the module suite of Embed, Entity Embed and Entity Media Browser is an utter mess sadly. Nothing works really as soon as you try using an view and not just autocompletion. Not even the contrib module Entity Media Browser. Only the module contrib Content Entity Browser works as intended imx).

But we certainly do want a simple way to...
1) Embed any entity (media or otherwise).
2) By using a View to (multi)select entities.
3) Select a view mode to render all of them.

Using Views for everything is already the dominant paradigm in Drupal, for good reason.
The richness of configuration options in current Entity Embed module is just a hard try to circumvent the void of proper, sensible default interface.

And while you can already use Views to prefilter results, technically, with an "autocompletion text" field, it quickly shows usability limits once you reach hundreds of content. So it is really necessary to be able to expose a "full-fledged" View with display style (list, table, grid) and exposed filters.

Considering how simple it seems to customize CKeditor with custom modules, the interface currently brought by the Embed contrib module ("Text editor embed button") is not strictly necessary imo (although it is nice to have for sure), as long as Drupal 8 provides good enough functionality out of the box.

IMO, the following would satisfy a majority of usage for small/medium websites.
A. An "Embed content" button in CKEditor, linked to a View restricted to Nodes, in a table kind of display.
B. An "Embed media" button in CKeditor, linked to a View restricted to Medias, in a grid kind of display.
Both providing the option to create a new entity instead of selecting it.
Both exposing basic filters from common data (title, type, changed date).
Both, when the relevant text filter is checked (done automatically if you add the relevant buttons to CKeditor toolbar), would provide a configuration tab at the bottom.
Any user with "Administer formats" permission could, for each...
- change the button tooltip text.
- specify the authorized "view modes" for selected entities (if none selected, all allowed. If one selected, the related option is greyed or hidden for users in the popup)..

It is obviously a solution providing much lesser flexibility than what you can (theorically) achieve with current contrib modules, but should be enough for many sites, since webmasters could adapt the View as needed for now.
Once there is a "native" View display type dedicated to this, you can then provide an additional option in the tab to select the "Content/Media" targeting View you want used.
Anything beyond could be safely left for custom/contrib to take over.

TL;DR
We just need a working solution to embed things (links to nodes, embedded images), bells and whistles are not necessary. The current state of "content reuse", especially "inline", is frankly very hard to deal with (the only workaround for now is using Token Embed Views, not really optimal as you can guess). ^^

Please note that while I cannot do much on development side, I'd be happy to help testing, writing "single use-cases" or otherwise helping in ways that don't require too much technical skills, like contributing to documentation once I understand how to make it work.

Cheers,

Re-tagging.

With @phenaproxima's blessing, I've gotten a start on this. It seemed like people were generally a little more interested in option #1 than #2, but inclined toward 2 because of the difficulty that 1 posed of getting API changes through the approval process. So in the interest of getting inline media entities into 8.5, I thought that I'd start on the text filter without any API additions. If this makes sense, we can add API changes in for 8.6.

So, just to start the conversation, here's a WIP patch that contains a media-specific filter based on the one in Entity Embed. What do folks think?

(p.s. please ignore the half-done test...)

Thanks, @bdimaggio! I will probably make a few changes here, but this is an excellent start. Much appreciated!

Updated this patch. Improvements:
* Each media type gets a CKEditor button, which the site builder can choose to enable at /admin/config/content/formats/[format name]
* Much javascript has been ripped off from Entity Embed to make those buttons display and customize individual media-entity embeds in text fields (they don't yet)
* There's a new "editor_embed" form mode, with only required fields appearing in it. For every new media type that is created, a form display based on this mode is enabled.

Todo:
* Fix javascript as noted above
* When someone adds a new required field to a media type, add that fields to the type's editor_embed form display

Updated patch with the following things.

• Ckeditor button now launches the modal dialog with a media entity add form
• Saving the form in the modal injects the <drupal-entity></drupal-entity> markup expected by the filter into ckeditor source
• Removes the unintentional dependency on a Entity Embed DomHelperTrait and copies the method code directly to the filter

Updated patch.

Cleans up some unused stuff from entity embed.
Adds another method from entity embeds domhelper trait.

Adding in a (hopefully) working test for the media embed filter.

Here's the actual interdiff from #14 to #15.

Since this issue was last touched, a lot of change has occurred:

• Media now has oEmbed support.
• The media library has landed in core, and is the UI we want to use for embedding content into CKeditor.

I think it's time to rescope this issue. Specifically, I think we need to turn it into a meta-issue, covering at least four other issues (and their follow-ups), each of which must be done in order (since each blocks the one after it, I think), all targeted for Drupal 8.7:

1. Implement an input filter which converts <drupal-entity> tags into fully rendered entities, with protection against recursive rendering and support for different view modes and parameters. This should implement no UI at all; it's just the input filter. Once this is in, we can...
2. Implement a CKeditor "widget" plugin which renders a drupal-entity tag in the editor -- essentially a "live preview" of an embedded entity, or at least a visually obvious placeholder for it. Then we can...
3. Add a plugin for CKeditor which allows an author to call up the media library, choose an item from it, and embed it in the editor as a drupal-entity tag. Once that's done, we can...
4. Extend that plugin so that it's possible to edit an embedded entity and change the way it's rendered for that specific embed (e.g., embed the same image twice but with different alt text in each instance).

If this sounds good, I'll make this a meta and open issues.

1. See https://www.drupal.org/project/entity_embed; it already has this.
2. See https://www.drupal.org/project/entity_embed; it already has this.
3. Yep.
4. Not sure about this, but this is IMHO a nice-to-have anyway, we don't need to talk about that now I think.

Oh, sorry, of course you already knew about entity_embed. You were looking for feedback for the sequence of steps, d'uh!

I think this sequence of steps makes perfect sense :)

+1 to #22.[1-3], thanks for putting it that way.

Concerning #22.4, I'm not sure if you mean to edit the embedding meta-data which defines how the entity is rendered (which viewmode to use, alignment, etc), or opening the real entity form? I would definitely try to avoid opening the entity form for edition inline, because that opens the door to issues related to inadvertent edition of entities that are also referenced/embedded from other places.

edit the embedding meta-data which defines how the entity is rendered

That is precisely what I meant.

I have opened four sequential follow-up issues to get this done for 8.7:

1. #2940029: Add an input filter to display embedded Media entities
2. #2994696: Render embedded media items in CKEditor
3. #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library
4. #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`

Each of these issues is part of the MVP for this feature, and each blocks the one after it. So, off we go!

The patch in #11 did no longer apply to the latest commit on 8.7.x. Re-patched it in order to continue working on this issue.

Please ignore #28 .. That was intended for #2940029

During DrupalEurope I have been discussing this issue with Sean. I started working on the child issues and after analysing what needs to be done we noticed that what we are trying to achieve is to bring Entity Embed to core.

I'll try to discuss this with xjm during Drupal Europe and see what she suggests.

Re #32: Indeed, when I read this in the summary of #2940029: Add an input filter to display embedded Media entities:

similar to the way the contrib Entity Embed module works

it made me wonder why core wants to reimplement from scratch something that's so far along already, and which is already doing exactly what we need. Seems way better to put that energy into an official 8.x-1.0 release of entity_embed (for 8.6.x and older) and bring it into core, basically as-is, for 8.7.x and beyond. A similar approach as we're taking for jsonapi, migrate_source_csv and others.

We don’t want to reinvent the wheel here. The idea is, indeed, to port parts of Entity Embed into core. The operative word is “parts”, though. We don’t want to add a new module, nor do we need all of Entity Embed’s API or UI.

So we can and should take some code from Entity Embed, but we should not be afraid to massage, rework, and refactor it before it lands in core.

Just discussed this with xjm and here's what she suggested:

• She likes the idea of #34 and suggests to go that direction
• If we are going to add a new plugin to ckeditor, we need to align with the UX team in order to get their feedback
• We need to create a first prototype of the dialog that we want to display (in CKEditor) for entity embed
• We present the prototype to the UX team and adapt the prototype until they are ok with the design

This is going to be fantastic! I wanted to add support for Media Library to the media_entity_browser module, as a stopgap for this so I've started on https://www.drupal.org/project/media_entity_browser/issues/2999249 (adding a new entity browser and view in a submodule).

It was surprisingly easy so huge thanks for making the Media Library code so easy to reuse!

That whole module will be deprecated once this all goes into core and is stable. But it could be useful as the first prototype for UX purposes?

#33++
#34++

Perhaps surprisingly, I agree with both :)

We need to carefully assess the state and scope of the entity_embed contrib module. If none of it would be "scope creep" for the scope of this issue, then I agree with #33. Otherwise, I agree with #34. To be able to properly answer this, I'd need to really assess the entity_embed module's code base. I've done that in the past but it's been a long time.

#35++

badcamp 2018

Note that the approach that Insert module takes would be nice to continue to allow for (a separate image/media field that can optionally be inserted into a text area); it is currently hampered by core's WYSIWYG image insert dialog.

I'm quoting a comment I made on a child issue:

So, embedding means injecting proprietary tags such as <drupal-media ...>? This is very sad. This kind of markup has no meaning outside Drupal. Other system doesn't know how to deal with tags such as <drupal-media ...>. In the case you want to achieve interoperability this is a NO GO. Why not injecting the rendered media which already have native tags (img, a, etc.) wrapped in this proprietary HTML tag that has all he Drupal metadata (uuid, view mode) as data attributes? The wrapper will be just ignored on other systems but the inner markup can still be interpreted. That said the process of rendering the media will take place on the embedding phase, not in the filter

We came to this point because we wanted to use entity_embed with media in our project, at European Commission. But instead of using MySQL as backend, our entities are using an RDF triplestore backend. The reason is that Drupal, in our case, act as a human connector/interface to the RDF backend. Data can easily used on other systems that know how to read RDF triples. Using these kind of proprietary HTML tags will just not work for us. Maybe we can reconsider the architecture to account also interoperability?

EDIT: So, I'm proposing that when embedding, we end up with something like:

<drupal-media data-entity-type="media" data-entity-uuid="..." data-view-mode="foobar">
   <!-- Begin rendered media entity markup -->
   <article ....>
      ....
   </article>
   <!-- End rendered media entity markup -->
</drupal-media>

Why not injecting the rendered media which already have native tags (img, a, etc.) wrapped in this proprietary HTML tag that has all he Drupal metadata (uuid, view mode) as data attributes? The wrapper will be just ignored on other systems but the inner markup can still be interpreted. That said the process of rendering the media will take place on the embedding phase, not in the filter

What should happen when the embedded entity is updated though? The benefit of doing this in the filter is that you don't have to re-save all content embedding the media when the media item changes. If external systems use the data, you have to make sure the filters are applied. Replacing the embed tag is not the only useful thing filters are doing, so applying the filters is a good idea anyway.

@seanB

If external systems use the data, you have to make sure the filters are applied.

If the external app is accessing data in the way I described in #40, you cannot use the Drupal API because the other system doesn't know how to use it.

Replacing the embed tag is not the only useful thing filters are doing, so applying the filters is a good idea anyway

Each system should take care of doing sanitization on HTML markup and HTML consistency checks. This is the reason why Drupal is filtering when showing the content rather when saving -- because we want to keep the field raw value as it was entered by the user or exchanged (imported/exported) with other external systems. But in this case is not about sanitization or other filtering -- it's about the content itself.

What should happen when the embedded entity is updated though? The benefit of doing this in the filter is that you don't have to re-save all content embedding the media when the media item changes

Well, this is a good question. I'm not so sure that we want to show every time the updated media entity. This is not an entity reference field, it's a markup field, and I would expect that will show the same markup as it was when I saved it. If I want to account the changes made to the media entity after I saved the markup, I simply need to create a new revision (aka resave the entity that contains the textarea). But, yes, I agree that others may want to see the latest version of the media entity every time that entity has been updated. I think this is a business decision and we shouldn't make any assumptions. Probably we can add a option to the embedding plugin that allows us to choose if we're embedding also the rendered media or only the proprietary tag.

Probably we can add a option to the embedding plugin that allows us to choose if we're embedding also the rendered media or only the proprietary tag.

I wonder how often one option is preferred over the other. From what I can tell, all projects I've worked on or heard of have the expectation that embedded entities are updated dynamically. Since this is the behavior Entity Embed has been providing for years, maybe we could leave the other option to contrib in a first moment, instead of trying to solve all of them at once?

@claudiu.cristea

If the external app is accessing data in the way I described in #40, you cannot use the Drupal API because the other system doesn't know how to use it.

It would be possible to apply filters before saving to the backend. Filters are the Drupal way of processing text input for use in a browser, but in this case it would be possible to create a filter that processes the input for usage in other systems.

Probably we can add a option to the embedding plugin that allows us to choose if we're embedding also the rendered media or only the proprietary tag.

I think this is a good point. Authors could get surprised if parts of their content is changed by someone else when the media item is updated. Not really sure what the best way would be to fix that. I guess it all depends on how the media is added to the content and if users know they are embedding something or not. To be fair, if you have a URL to an image and the image on that URL is replaced, the same thing would happen.

@claudiu.cristea re: #42:

I'm not so sure that we want to show every time the updated media entity.

That's funny, b/c that's exactly what I would expect, and why I want a level of indirection via media entities in the first place. ;) Instead of directly referencing an img URL everywhere, I want to reference "media item #1234" and then if that item ever changes, I know that all the places that reference it will automatically get the new version.

I think this is a business decision and we shouldn't make any assumptions.

Very fair point. However, "don't make any assumptions" might mean incredible complexity here, since these are two pretty fundamentally different models of how media entities and embedding should work. As @marcoscano points out in #43, entity_embed has worked this way for years, and it seems like the vast bulk of the Drupal ecosystem shares this mental model, not the "only update the embedded markup if I re-save it" approach you're proposing. I'd vote for letting core continue to make this assumption (at least the "80%" use-case, if not the 98% or 99.9% case), and saying that people who want different behavior should implement it in contrib.

@seanB #44:

To be fair, if you have a URL to an image and the image on that URL is replaced, the same thing would happen.

Excellent point.

Cheers,
-Derek

Thanks @dww! I think we should continue with the current plan to implement a <drupal-media> tag.

I can see the need to convert this tag to a rendered entity before storing it, and opt out of updates, but that doesn't seem to be a standard use-case. To support this, a filter can be applied in hook_entity_presave(), which could be a pretty useful contrib module.

I hope I am not sending the wrong message, I think it is really valuable to receive feedback from real-world implementations and to consider if anything we are building here is not blocking custom/contrib modules from doing things.

Moved #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5 and #2560457: Support drag-and-drop image uploads in CKEditor from #2825215: Media initiative: Roadmap since it would be better to outline the plan for WYSIWYG embedding here.

I'm very sad :(((.
I was hoping that the media system will be ready for version 8.7.
I see sadly there is no chance of it.

@Somfai Tibor:

Contrib already has a solution for this. Entity embed is a module that supports embedding of any renderable entity and supports extensive display configuration.

Contrib already has a solution for this. Entity embed is a module that supports embedding of any renderable entity and supports extensive display configuration.

Most of a solution. There are still some issues like #2640398: Allow <drupal-entity> elements to be inline CKEditor Widgets that stand in the way of feature parity with D7.

Getting this into core would be a big win for the community. The number of contrib modules (Entity Embed/Entity Browser/Inline Entity Form/etc) and the amount of configuration needed is pretty daunting for someone new to Drupal. And don't forget - Views, Media Entity, and WYSIWYG/CKEditor all used to be contrib modules. We're better off that they're in core.

Getting this into core would be a big win for the community.

I think almost everyone agrees with that. Per #2825215: Media initiative: Roadmap, it's one of only 4'ish issues left before Media can be enabled by default in Standard profile. That's worthy of promoting it to Major, so doing so.

I think this is more of a feature request than a task, but that doesn't change its importance in any way.

I'd be happy if the community managed to make a decision.
As a starter drupal user, I do not really have the right to speak to the decision.
The media is also an entity, but it is good to separate the two groups.
Browser and embedding could support all entities.
It is not always possible to insert entities or media in separate fields.
It is also necessary to include entities and media in the current content edited by CKEditor. I think this would be a very useful feature in the core. I know it's an external modules. However, it would be good if we could finally make a decision on the subject. I'm very looking forward to the new media system :).

The number of
contrib modules (Entity Embed/Entity Browser/Inline Entity Form/etc) and the
amount of configuration needed is pretty daunting for someone new to Drupal.

Well-put, @Chris Burge -- also Views configurations, which are the opposite of simple :)

For my part, I'm tired of tormenting myself trying to guess which combination of contrib modules and configurations will be most future-proof.

@Somfai Tibor Thank you for commenting, your perspective is extremely relevant and important, and it's wonderful that you took the time to chime in. Forgive the tangent, but for anyone else reading here, please know that plenty of developers say that they appreciate when non-devs chime in to express whether or not some functionality is important to how they use Drupal -- and to share experiences / use cases / potential side effects that they're more likely to encounter :)

please know that plenty of developers say that they appreciate when non-devs chime in to express whether or not some functionality is important to how they use Drupal -- and to share experiences / use cases / potential side effects that they're more likely to encounter :)

+10000000

With the Media Library in 8.7, Entity Browser/Inline Entity Form should not be needed for simple media use case. We are going to try and get WYSIWYG integration in for 8.8, but we can use all the help we can get. See #27 for the issues we need to fix to get this done.

In #27, @phenaproxima wrote this excellent comment:

I have opened four sequential follow-up issues to get this done for 8.7:

1. #2940029: Add an input filter to display embedded Media entities
2. #2994696: Render embedded media items in CKEditor
3. #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library
4. #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`

Each of these issues is part of the MVP for this feature, and each blocks the one after it. So, off we go!

It didn't get done for 8.7. But I'm working on them now. They're the remaining tasks and should be in the issue summary though, so copied them there.

Updating title per the Barcelona Media sprint — see #2940029-25: Add an input filter to display embedded Media entities. Yes, this still needs an issue summary update.

Removed the second proposed resolution. We know what our plan of attack here is.

Moved a couple of related issues out of the IS.

I think the issue summary is now up to date.

🥳🙏

Tagging for DrupalNorth 2019

I wrote a Drupal 7 to 8 migration for this at #3048368: Add migration from media_wysiwyg tokens to entity_embeds.

One of my objectives this quarter was getting this meta issue to a place where it'd be certain Drupal 8.8 would ship with Media embedding. The end of the quarter is here. Time to write a long comment to give you a sense of where things are at. 🤓 I won't be working on this as much anymore next quarter. I'll still be around for rerolls and reviews. The bulk of the work has been done. But this comment should enable core contributors & committers to get a sense of the current status and enable both core committers & contributors to understand next steps.

#2940029: Add an input filter to display embedded Media entities and #2994696: Render embedded media items in CKEditor now have green patches that have been extracted from https://www.drupal.org/project/entity_embed after stabilizing that module first (from 1.0-beta2 in October 2016 to 1.0beta3 in May, via RCs, to 8.x-1.0 today). They have very solid code, informed by by tens of thousands of Drupal 8 sites using them over years. They also have comprehensive test coverage. Those two represent the most complex and highest-risk (in terms of integration with other subsystems, brittleness and potential security vulnerabilities) child issues of this meta. They both belong in media.module.

#2994699: Create a CKEditor plugin to select and embed a media item from the Media Library also has a green patch. This has been far less vetted since it's specifically about selecting media from the Media Library, which is UI functionality that only shipped with Drupal 8.7.0 last month. What matters here is that we offer a UX that is consistent with that of the media reference field, and an experience that doesn't get in the way — forcing the user to think about a view mode, alignment, caption and metadata overrides (alt for example) is bad UX.

Finally there's #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`, which also has a green patch. The above also means that we must use the same affordances for overriding media metadata such as alt and title for example — which media reference fields do not yet support. That's why #2994702 is currently hard to move forward: while it's easy to implement some UX for per-embed overrides in CKEditor, it must be consistent with the UX that #3023807: Override media fields from the reference field will provide. That's why we came up with proposals for this at Drupal Dev Days Transylvania, and these were presented at the UX meeting of June 11 — see comments 9–12 at #3023807 (includes mock-ups and YouTube recording of the UX meeting).
Interestingly, alignment and captions are not overrides, but they're conceptually similar. While they're not about overriding things, they're about creative freedom/visual representation, yet they still apply per media embed. This part has been implemented already, but precisely because of the cognitive relationship with metadata overrides and our unwillingness to add three buttons (a. change alignment/caption, b. override metadata, c. delete) on each embedded media means we have to first agree on an approach before we can proceed.
(Personally, I think alignment/caption + metadata overriding can be in a single dialog titled "Edit embedded media" — this is very similar to how the existing image dialog already works. And that's also what #2994702-14: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`'s patch implements!)

Note that with the current direction of #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library and #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`, these can happen in parallel!

Even without #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`/#3023807: Override media fields from the reference field, it will be possible to enable Media by default in standard: they were marked as "strong should-haves". But "just" #2940029: Add an input filter to display embedded Media entities + #2994696: Render embedded media items in CKEditor + #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library already make it possible to update the text format & text editor configuration in the Standard install profile and allow all future Drupal users to benefit from this out-of-the-box :)

Finally, one thing that came up at the Barcelona Media Sprint in December 2018 and was documented in #2940029-25: Add an input filter to display embedded Media entities: a new embed or embedded view mode for Media entities. I believe this would be a massive improvement: choosing a view mode for embedded media is cool, but how often does it really make sense to embed media in Full or Media Library view mode (the two defaults core ships with)? And even if we add an Embed view mode by default, when does it make sense to select one of those other two?
I totally see how a Tiny embed and Detailed embed could be valuable. But that is a relatively advanced use case — used by perhaps 10 or 20% of Drupal 8 sites. Why not defer that to a media_embed_viewmode contrib module?
This would simplify so much. 🙏

Wanna get a visual sense of the current state? Here you go:

1. 
2. 
3. 
4. Screencast showing the UX on a high-latency connection while actively editing: https://www.drupal.org/files/issues/2019-06-28/media_embed%20cached%20pr...

#2940029: Add an input filter to display embedded Media entities landed. All subsequent patches have been rerolled:

1. #2994696-25: Render embedded media items in CKEditor
2. #2994699-27: Create a CKEditor plugin to select and embed a media item from the Media Library
3. #2994702-23: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`

Added #3067116: text_summary() returns malformed (not normalized) HTML for basic_html and other formats that use filter_html instead of filter_htmlcorrector and #3067124: text_summary() returns a plain string, even if passed a MarkupInterface object as additional required bug fixes to the IS. While those are pre-existing bugs, they are much more easily encountered with embedded media, so I wouldn't consider this meta issue complete until they're fixed. Fortunately, I think they're both pretty straightforward to fix.

Only patch reroll

+++ b/core/modules/media/media.module
@@ -179,6 +182,7 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
+<<<<<<< HEAD

This doesn't seem correct.

#2994696: Render embedded media items in CKEditor just landed! Next step: #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library.

It seems #69 rerolled the earlier patch iterations from a very long time ago. This issue is now patchless and the work has moved to separate issues. See the issue summary. 50% of the patches have already landed, and >80% of the work has already been committed to Drupal core :)

If this issue is intended to be patchless, lets hide all old patches so people won't be confused :)

All subsequent patches have been rerolled:

1. #2994699-45: Create a CKEditor plugin to select and embed a media item from the Media Library
2. #2994702-26: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`

@Martijn de Wit: good call 👍🙏

!!! MUCH EXCITE !!!

.......................
Question: Could y'all recommend an implementation for "right now" that'll be closest to what you're working so hard to get into core?

I was going to use Media Entity Browser, because I'm familiar with it, and, in a recent release, they did work to emulate core Media library UX (so that if you're using core media library for image fields, the CE UX wrt images will be as consistent as possible). They said:

Now that 8.6 is released with Media Library, and until WYSIWYG embedding gets supported, this module could bridge the gap.

Do you folks think MEB is a good option for the time being, or do you have any other recommendations?

Thank you!

The work we are doing in core is based heavily on the Entity Embed contrib module, and they are cross-compatible to an extent (with some important differences -- the core functionality is intentionally more limited in scope than Entity Embed is). So, for the current solution that is closest to core, I would suggest the latest release of Entity Embed.

I would checkout entity_embed + entity_browser + media_entity_browser. Possibly with dropzonejs.

Please know though that Entity Embed uses different markup than the core functionality (intentionally, so that sites on Entity Embed can continue to use Entity Embed). Entity Embed uses <drupal-entity>. Core uses <drupal-media>.

#2994699: Create a CKEditor plugin to select and embed a media item from the Media Library is in! On to the next and final piece.

I don't want to spoil the party, but … in #2940029-97: Add an input filter to display embedded Media entities, we agreed to default to the full view mode. But this doesn't make for the best UX possible. This was first raised by @effulgentsia in #66. Some of the comments from 66 through 97 talk in some detail about this.

To truly consider WYSIWYG embedding of media entities done, I think we should probably also do #3066802: Ensure that embedded image media items do not display at unreasonable sizes by default.

Wouldn't it make more sense to allow users to select view mode for embedded content? Similar to #2061377 just expanded for all media types. Or is it out of scope for now?

@mirom Yep, I agree. #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption` is adding the ability to override metadata. That's where you would also be able to pick a different view mode. I think it's in-scope for #3066802: Ensure that embedded image media items do not display at unreasonable sizes by default to not only add that new view mode, but to also add the 10 or so lines to EditorMediaDialog plus test coverage :)

@oknate just convinced me that allowing users to select a view mode should be done in a separate issue actually, because there are significant UX concerns around that, with a big potential for bikeshedding. Just adding a new view mode and making it the default is comparatively trivial :)

Here's a separate follow-up issue to allow setting the view mode from the dialog:
#3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog

I should've updated the issue summary instead of added it as a related issue. Done.

Adding several follow-ups and related issues from #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`.

Not sure if this should be addressed here or elsewhere, but it'll need to be possible to link embedded entities to other content or external URLs. Currently, I can select an embedded entity, click the Link button, and enter a URL. I'm not seeing any visual indication in CKEditor that it's linked, but when I save and view the node, the entity is wrapped in an A tag -- div and all:

<article class="media media--type-image media--view-mode-full">
  <a href="https://google.com" target="_blank" rel="noopener noreferrer">
    <div data-quickedit-field-id="media/1/field_media_image/en/full" class="field field--name-field-media-image field--type-image field--label-hidden field__item">
      <img src="/sites/drupal88.dd/files/error.jpeg" width="640" height="427" alt="foo" typeof="foaf:Image">
    </div>
  </a>
</article>

...which isn't really ideal, especially if the embedded entity is a piece of content that itself contains links or text.

This was addressed in an Entity Embed issue (Image entities/fields embedded using Entity Embed cannot be linked in CKEditor), though I'm not sure if the same approach works here.

#88: This is definitely the wrong place for that :) You wrote "embedded entity", so you're probably using https://www.drupal.org/project/entity_embed? Please file an issue there!

I'm sorry, I wasn't clear. I'm not talking about the Entity Embed module -- I simply mentioned it because a similar problem has recently been addressed in that module.

What I'm talking about is how this new core functionality works (or doesn't) with the core Link plugin. With my 8.8.x-dev test install, I can now embed media from the media library, which is great -- but I can't use the core ckeditor Link plugin to link the embedded media.

In other words, here's what I'm doing:

1. Install 8.8.x-dev
2. Configure the text editors to enable the new "Insert from Media Library" button
3. Create a new Image entity in the Media Library
4. Create a new Basic Page node
5. Use the "Insert from Media Library" button to insert my image entity into the body field
6. Click the embedded image entity to select it
7. Click the Link button and enter a URL to wrap the image entity in an <a> tag
8. Save the node

The result is the HTML I pasted above. The Image is successfully embedded, but the <a> tag is placed outside the <div> wrapper.

That's the simplest case, really. Imagine I build a more complex media type -- say, an Image with some additional fields such as a date and an entity reference to a Photographer content type. Say I configure the default display for this media type so that the name of the Photographer is linked to the Photographer node. I insert one of these image entities from the library, and then use the link button as described above. What happens then? What should happen then? Is the whole entity going to be wrapped in an <a> tag? That wouldn't work, because there'll be another <a> tag inside of it, on the Photographer field. Will just the image be wrapped in the <a> tag? That's probably the desired result.

Is this all more clear? If not, I can put together some screenshots.

@wrd, will you file a separate issue, including those useful steps to reproduce and screenshots? It sounds like something we should certainly investigate (and thank you for testing it and reporting back!), but this issue is not the place to do it. :)

Will do! Thanks!

Separate issue opened here: Allow users to link embedded media entities in CKEditor

We did a "product management deep-dive" into this feature on the last UX meeting https://www.youtube.com/watch?v=MDrKAH0xaD4 ... my recollection is we identified the following stable blockers:

1) There's an "undefined target_bundles" notice on the media field configuration form. Since you'll get this the very first time you interact with the module, we should fix it. (Hopefully an easy fix.)

2) The description of the Media embed filter is super long and has more twists and turns than a murder mystery. Suggestion was to configure it with "smart defaults" (align, caption, alt, and title all accessible by default, to match what image does), and strike the second part of the sentence.

3) We found a feature regression with regards to images in that you can't link the image to something else... the <a> tag ends up getting wrapped around an <article> tag which is invalid HTML. The discussion eventually landed on #3066802: Ensure that embedded image media items do not display at unreasonable sizes by default moving from a "should-have" to a stable blocker.

4) One other feature regression with regards to images is the inability to resize media once it's been embedded. With images, you get resize handles and can easily smoosh the image down if it's taking up too much room. This is not the case with media. And this is something that is easily a "90% use case" that people will want to do. :\ There was talk of providing a view modes drop-down on the modal with the alignment + caption controls... while this would technically fulfill the requirements, it's a big step down UX-wise. :\ OTOH, to get the same UX as images was cited as a "dragon" (e.g. needs to hook into the breakpoints system, etc.) which if we stick it in front of stability, is likely to delay this feature until 9.1. I think I speak for all of product management that the feature itself is a tremendous improvement over the status quo, and so with regret, this regression was ranked as only a super, super strong should-have vs. a stable blocker, assuming that contrib could conceivably work around it. (However, if anyone has any bright ideas for how we might "MVP" this capability, I would certainly love to hear it! I suggested height/width fields on the dialog, but this was shot down. :\)

Also crediting folks from said UX meeting. 1:20 mins of grilling, egads!

#94.4: I strongly disagree with this. Yes, "this is what people want". But it's the same kind of "people want this" argument that is also used for things like choosing the font, aligning text, … — in short, all the things that allow arbitrary creativity aka "frontpage in the body field". It's how sites end up looking like crap on mobile devices, for example (or more generally speaking: how they end up looking like crap on devices that aren't the content author's device).

The only reason we allowed the resizability of images in CKEditor in the first place is because #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5 and #2822389: Allow responsive image style to be selected in Text Editor's image dialog (necessary for structured content) had not been done yet. The same restriction is not true once #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption` lands — people will be able to choose a different view mode.

I understand the perspective and reasoning in labeling it as a UX regression, but if #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5 had happened, you also wouldn't have had the ability to resize images in CKEditor either.

This is once again about the tension between content authors' level of control and front-end developers' level of control. Drupal has not yet truly solved this. The answer so far is: it's up to the site owner to decide who has the biggest level of control, because for one site it may be the one, and on another site it may be the other.

This is a super hard problem! It almost feels like we need to ask during Drupal installation or perhaps on a per-text format basis what level of control should be granted to the content author vs to the theme.

RE #94.4, my organization worked around this by using view modes that control the size of the image, both with image styles and with CSS. We present view modes "content width", "large", "medium", "small". The editor cannot resize the images using drag handles, but we decided that is OK, largely for the reasons outlined in Wim's first paragraph.

Re #94.4: A workaround if some from of resize controls aren't available for 8.8 could bee to recommend the use of the Responsive Images core module. This will mean that images size appropriately based on screen size and breakpoints. This approach might actually be the desirable approach for the majority of publishing sites using Media, since they would likely want to control the size of images based on breakpoints to ensure they style correctly.

Sorry, I'm fine with a view mode selector as the solution to the problem. But the basic problem of "I want to float this image left and oh shit now all my content is smooshed into a 4 cm column and I have no way to fix it" persists without some means of fixing this. That's why I would prefer it as a stable blocker.

From a UX point of view, however, "select from this list of textual descriptions for relative sizes" is absolute crap. :( So ultimately, we need to aim for a graphical "view mode switcher" (keep the drag handles, but they snap to the most appropriate view mode and preview the image inline, see e.g. SquareSpace) but that's not at all fair to throw in front of the "path to stability."

But, if we can solve the basic "4cm" problem somehow, without introducing a huge "dragon" in the way, we really, absolutely should. It's going to bite basically everyone.

Adjusting issues linked in the IS per @webchick's summary in #94. I will also file issues for point 2.

Opened #3077756: Improve description of the Media Embed filter in response to #94.2, and making it a stable blocker.

Oh hai.

So @effulgentsia, @phenaproxima, and I had a call where we discussed #94.3 & #93.4, and think we came to a conclusion without a lot of extra work for anyone. (Fingers crossed!)

#3066802: Ensure that embedded image media items do not display at unreasonable sizes by default was already a stable blocker, but there was some question in there as to the "spec" of what this should entail. Covered that (hopefully) in #3066802-33: Ensure that embedded image media items do not display at unreasonable sizes by default and #3066802-34: Ensure that embedded image media items do not display at unreasonable sizes by default. If there are any other questions/clarifications needed, just ping me!

For the sizing issue, a GREAT suggestion from @effulgentsia was to hard-code the width of embedded media at e.g. 75% of the CKE viewport. Then you could never get into the situation where you have individual characters along the side that you can't really edit because the image (or whatever) is too large.

That hard-coding (assuming it passes @lauri's review) would fall into the "Must have" bucket, since you effectively prevent people from editing content if they get into this state otherwise. But making it user-configurable in some way (@effulgentsia suggested a "25%/50%/75%" selector, which actually sounds nice) can be a "should have" follow-up. And ultimately, we want to go for something like this https://support.squarespace.com/hc/en-us/article_attachments/202981268/B... that snaps image sizes to predefined image styles, but that is definitely not Drupal 8 material. ;)

Also, hm. The sizing concerns in #94.4 might be overblown. This is as bad as I can make it with Image module, which is still passable (though obviously ugly and annoying).

I unfortunately can't test with the Media align patch right now (long, annoying story), but assuming it looks no worse there (I was remembering a

s
k
i
n
n
y

column of text with like one or two characters per line) then NEVERMIND, we can definitely strike it from the stable blockers, and I'm REALLY sorry for the distraction. :(

For the sizing issue, a GREAT suggestion from @effulgentsia was to hard-code the width of embedded media at e.g. 75% of the CKE viewport.

Just to clarify, my suggestion is to hard-code the max-width, not the width. In other words, to add CSS along the lines of max-width: 75% on media that's aligned left or right. It's fine for the media to be smaller. It's just that if you're explicitly choosing to align it left or right, then you're choosing to put something else beside it, which means leaving some amount of room for that something else.

The sizing concerns in #94.4 might be overblown.

No, they're not. With #2994702-120: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption` applied, if you add a Media image larger than the CKE width, and then click the pencil icon and select Left for the alignment, then you only get room for one letter per line to the right of it. And even if the image src is only 480px wide (per #3066802-33: Ensure that embedded image media items do not display at unreasonable sizes by default), you still run into that exact same issue if you shrink your browser window to leave <480px for the CKE area. So if we don't want super skinny columns of text, we need some CSS to constrain the maximum width of aligned media.

• Added issue for #108 and #106:
  #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75%
• Also, I added an issue for a regression I found while writing tests:
  #3078161: DrupalMediaLibrary plugin breaks things when adding a new text editor
• The fix for the regression is included in the latest patches for #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`

#106 + #108: Once again @effulgentsia saves the day with his incredible conensus-building skills and uncanny ability to find something that satisfies all needs 😁This indeed makes it so that the more width-constrained CKEditor <iframe>-mode viewport can never become filled with images too wide for that viewport (as in, too wide for it to still be a usable experience).

Note that we need to handle this not only for images, but for any media. But that's a mere/simple selector problem, so I'm not worried. :)

Adding #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75% as a should-have. See #107 for fun demonstration of this bug.

As a side note, #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption` landed!

Removing #3077756: Improve description of the Media Embed filter from the issue summary, since it was fixed by #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`.

Adding follow-up issues:

• #3071713: Make error messages for embedded media themeable
• #3068786: Allow customization of missing media indicator returned by MediaEmbed filter

Adding #3069525: Refactor to remove drupalunlink event listeners for registered linked widgets as a follow-up.

I un-postponed #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog. Either that one or #3066802: Ensure that embedded image media items do not display at unreasonable sizes by default could be committed next. They are independent of one another in their current forms.

#3066802: Ensure that embedded image media items do not display at unreasonable sizes by default has landed!
🎉
Adding a follow-up #3081303: Allow media embed width constraint to be configurable, which is a follow-up of #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75%

Shouldn't #3066802: Ensure that embedded image media items do not display at unreasonable sizes by default also be a must-have?

See #2834729-198: [META] Roadmap to stabilize Media Library

Per discussion with @webchick, updating the first must-have to an either/or.

The summary was updated so that there's a conditional must-have:
Either #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog or #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75%

I copied the conditional should have over from that issue to this issue.

Re #3078287: I'm not following a goal of that story - I was under the impression that the media will use some view mode. Does that story mean that the view mode will be ignored? If yes, will be there some way to turn off that behavior?

I found a bug in #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`:

#3081983: Double Alignment classes on widget wrapper for embedded media in CKEditor

#122: A view mode is used and respected, but in a CKEditor <iframe> instance the viewport is rather narrow, so some view modes may result in such a wide rendering (as in, number of pixels wide) of the media that it impedes usability. So that issue is choosing to sacrifice some fidelity (not that it's got perfect fidelity otherwise — it's an approximate preview already anyway!) in order to improve usability.

Here's another bug I found that's closely related to one Wim found yesterday:

#3081988: Disappearing edit button and caption on media embed when using dialog

There's a bug fix for this in the latest patch for #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog. So if that gets committed, we may not need a separate bug fix.

Since #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog is delayed, I suggest we work on getting these two bug fixes in:

• #3081988: Disappearing edit button and caption on media embed when using dialog
• #3081983: Double Alignment classes on widget wrapper for embedded media in CKEditor

SHOULD-HAVE: Either #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog or #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75%. Both would be great, but either one is sufficient to make media embedding usable enough to allow Media Library to be stable.

Who wrote this? What's the rationale? They're about completely independent concerns. @webchick said this in #3074608-68: Optionally allow choosing a view mode for embedded media in EditorMediaDialog, which surprised me, but apparently this meta is saying that. Why? I explained in #3074608-69: Optionally allow choosing a view mode for embedded media in EditorMediaDialog why this is incorrect as far as I know.

I moved the bug fixes I added the other day under the bug fixes sections.

Regarding #127:

The Either/or comes from @effulgentsia in conversation with @webchick in this comment: #2834729-198: [META] Roadmap to stabilize Media Library.

Now that one of these issues is committed, where do we want to move the other one in list?

Adding ✅Next to issue in the issue summary for #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog.

🎉

Moving this issue to the list of follow ups.

#3081303: Allow media embed width constraint to be configurable

I had added it to the "in order" list, so I think it's OK for me to move it. With the current plan to add the code that sets this (outside of the WYSIWYG) to the themes, it probably doesn't make sense to make this configurable, à la layout builder. It will be controllable at the theme layer.

It might still make sense for the WYSIWYG, but this issue isn't import enough to be on the main list, IMHO.

Regarding #127:

The Either/or comes from @effulgentsia in conversation with in this comment: @webchick #2834729-198: [META] Roadmap to stabilize Media Library

Now that one of these issues is committed, where do we want to move the other one in list?

I still think the "either/or" is wrong. We want both.

I still think the "either/or" is wrong. We want both.

I agree we want both. The either/or language was added to the "must-have" list in #2834729: [META] Roadmap to stabilize Media Library, because even though the two issues address different things, @webchick was satisfied with either one of them as sufficient to address her concern of the Media Library not being sufficiently usable to mark it Stable. Although #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75% is still needed to cover all permutations of image style configurations and browser window widths, at least #3074608: Optionally allow choosing a view mode for embedded media in EditorMediaDialog now provides a workaround. For example, let's say a site configures "Large" to be 960 (or some other relatively wide number), and let's say that's appropriate for images displayed on their own line. If a site now wants to also make use of left or right aligned images, they can configure a different image style (e.g., "Medium"), and corresponding Media view mode, to be a size that's appropriate for that and allow that to be selected by editors.

We absolutely still want #3078287: Constrain the width of aligned images, media, blockquotes etc to a max of 75%, so that aligned images display well for viewers and editors at different browser widths, without requiring an extra step on the part of content authors. But neither @webchick nor I think that we should hold up marking Media Library stable on that.

Now that one of these issues is committed, where do we want to move the other one in list?

I updated the list to put them on their own lines, and retained the "SHOULD-HAVE" label on the one not yet done.

#133: Thanks! 👍🙏 That makes perfect sense :)

Adding #3073535: Allow only a subset of the media library to be exposed to CKEditor to "Additional related and follow-up issues".

It was never added to any of the lists on this page, AFAIK.

Added a new follow-up. #3085436: Center alignment of media embeds doesn't work in Bartik

Adding under "Additional related and follow-up issues". Given that we're embedding with center-alignment by default, someone might want to adjust the priority higher, or move it to one of the other lists.

#3073535: Allow only a subset of the media library to be exposed to CKEditor and #3076773: Edit Media button within WYSIWYG should include media label for screen readers landed, updating the issue summary. 🎉

#3071713: Make error messages for embedded media themeable landed! 🎉

I started a sandbox throwing out an approach to handling embedding media inline. The gist is to extend much of the work already done to embed media from the media library and add a widget and plugin for a "drupal-inline-media" tag. Inspiration coming from #2640398: Allow <drupal-entity> elements to be inline CKEditor Widgets. I am curious what the community would like to see as an approach to address such an issue.

All must-haves have been completed. The most important should-have is #3078287 and I last bumped that two weeks ago: #3078287-111: Constrain the width of aligned images, media, blockquotes etc to a max of 75%.

Adding this as a should have. #3092795: [regression] Restore styling for embedded media edit button in Seven theme

Sorry if that's presumptuous. Feel free to move around.

Adding two new issues under "Additional related and follow-up issues":

• #3100066: "Convert line breaks into HTML" filter should exclude <drupal-media> tag
• #3100470: EditorMediaDialog triggers an "undefined index" notice for data-view-mode

I just reported #3102011: Media Library's embed modal is almost unusable with less than 600px browser viewport width.

Please consider assessing this issue and add it to the matching section.

Added #3117172: Allow CKEditor styles for <drupal-media>.

I think it's time to close out this meta in favor of #2825215: Media initiative: Roadmap. I've moved all issues listed in the issue summary to that one.