Image entities/fields embedded using Entity Embed cannot be linked in CKEditor

This doesn't work in D7 either.

One way could be to provide such a formatter for image fields which links images to other URLs.

Although I'm not sure whose job it should be to add such a formatter - we can add one in entity_embed but wouldn't it be better if that gets added to Core?

You would need an Image field formatter that does what you'd like. It might be worth writing a patch for core's Image field formatter to provide a third option for instead of linking to content or file, it provides a 'Custom' link where you can type whatever to link it to.

That sounds reasonable.

"pure images" (i.e. not embedded using Entity Embed) can be linked since a few days before Drupal 8.0.0 — see #2510380: Images cannot be linked in CKEditor.

Which makes me wonder whether @jzech meant:

1. Select an <img>, click the "Link" button, and end up with <a><img></a>, or:
2. Select an <drupal-entity>, click the "Link" button, and end up with <drupal-entity><img></a>

?

If it's the first: Drupal core supports that now, and that'd actually be out of scope here.

If it's the second: the Entity Embed plugin for CKEditor will need to explicitly support that, but it should definitely be possible.

The second option above seems like an excellent thing to add after a 1.0 release :)

Updating the issue title to match my understanding. The current title is very vague.

I guess it was kind of both. Both drupal-entity does not work and until recently, a normal image also didn't work. That was fixed, so this is now only about drupal-entity.

Okay, cool :)

Quote from "Problem/Motivation":

It needs to be added to the source code manually.

This does not work on my site...

1. Edit node, insert embedded image
2. Open source code, add a tag around the drupal-entity tag
3. Save -> the image link works!
4. Open Edit mode once again
5. Open source code. The link is not there anymore!
6. Save node without any changes -> The image is not linked anymore!

Do I miss something here? Or is this a known behaviour?
It would be so nice to have the possibility to link embedded images...! Thanks! :-)

Suggestion at #5 works perfect - thanks!

There is also a "Link formatter" in the Field formatter module that might do the job too.

Organizing tags a bit.

It would be nice to document possible solutions in our D8 guide.

Sorry, could someone explain how to get this working? I'm probably misunderstanding something fundamental, but where would I go to change the field formatter for embedding image media? Or changing "core's Image field formatter" as described in #5? This is the one thing stopping us from adopting Media. Any help is greatly appreciated!

It doesn't work AFAICT. You still can't embed an image and link it to a webpage using Entity Embed in CKEditor.

I have created an ugly, ugly, ugly workaround:

1. [Once] Apply the attached path to the Media entity module, not this module.
2. Embed an image using the CKEditor button (as normal) and select to link the image/media to "Media entity".
3. Select View source, and locate "image_link":"media"
4. Replace it with "image_link":"http://example.org" (to link to example.org).
5. Stop viewing source, and either save or preview. Voila! The image is linked to example.org.

I told you it was ugly.

This topic has been dormant for a few months, and I was wondering whether there's any progress?

What would be the easiest way for me to make an embedded image "linkable" right now? I've looked through the above options, but I'm afraid it hasn't helped me yet...

I am also interested in any update on this.

I feel this is a really important feature which is currently missing from Media

Here's a patch that adds the ability to make any embedded entity into a link in a clean way (note: currently tested only with images; ymmv depending on the contents of the entity, particularly with stuff like iframes).

In the meantime, I've also created a module that does exactly the same thing, here:

https://www.drupal.org/project/entity_embed_link

DO NOT INSTALL BOTH THE MODULE AND THIS PATCH. THEY DO THE SAME THING.

The patch in #20 solves the issue for me, and works against 8.x-1.0-beta2.
Thanks @BartK!

Thanks for the patch, BartK! Worked for me too.

Patch #20 worked for me too. Thanks BartK.

Thanks Bartk. This patch works for me too. I have also altered form to add basic integration with linkit by

/**
 * Implements hook_form_FORM_ID_alter().
 *
 * Add support of linkit feature for URL field.
 */
function mymodule_entity_embed_dialog_alter(&$form, FormStateInterface $form_state, $form_id) {
  if (isset($form['attributes']['data-entity-embed-display-settings'])) {
    $editor_storage = \Drupal::entityTypeManager()->getStorage('editor');
    $editor = $editor_storage->load('rich_text');
    $linkit_profile_id = $editor->getSettings()['plugins']['linkit']['linkit_profile'];
    $form['attributes']['data-entity-embed-display-settings']['link_url']['#type'] = 'linkit';
    $form['attributes']['data-entity-embed-display-settings']['link_url']['#autocomplete_route_name'] = 'linkit.autocomplete';
    $form['attributes']['data-entity-embed-display-settings']['link_url']['#autocomplete_route_parameters'] = [
      'linkit_profile_id' => $linkit_profile_id,
    ];
  }
}

This does not support linkit attributes plugins.

Mostly Drupal Coding Standards issues.

Otherwise patch applies cleanly to 8.x-1.x.

1. +++ b/entity_embed.module
   @@ -6,6 +6,8 @@
   +use Drupal\Component\Utility\UrlHelper; ¶
   + ¶
   

   Needs trailing whitespace removed.

2. +++ b/entity_embed.module
   @@ -31,6 +33,7 @@ function template_preprocess_entity_embed_container(&$variables) {
   +  $variables['url'] = isset($variables['element']['#context']['data-entity-embed-display-settings']['link_url']) ? UrlHelper::filterBadProtocol($variables['element']['#context']['data-entity-embed-display-settings']['link_url']) : '';
   

   This seems a little long to be using a ternary operator.

   They're useful for short, concise conditions but become a little unreadable like this.

3. +++ b/src/Form/EntityEmbedDialog.php
   @@ -160,6 +160,7 @@ class EntityEmbedDialog extends FormBase {
   +    ¶
   

   Needs trailing whitespace removed.

4. +++ b/src/Form/EntityEmbedDialog.php
   @@ -439,6 +440,22 @@ class EntityEmbedDialog extends FormBase {
   +    ¶
   

   Needs trailing whitespace removed.

5. +++ b/src/Form/EntityEmbedDialog.php
   @@ -439,6 +440,22 @@ class EntityEmbedDialog extends FormBase {
   +        '#description' => t('The URL you would like this item to link to.  Leave blank for none.'),
   

   Double space needs removing.

6. +++ b/templates/entity-embed-container.html.twig
   @@ -6,10 +6,11 @@
   +<article{{ attributes }}>{%if url %}<a href='{{ url }}'>{{ children }}</a>{% else %}{{ children }}{% endif %}</article>
   

   Space between `{%` and `if`.

Here's an updated patch with the above issues resolved.

Thanks @BartK. I am working on the integration of the entity_embed and linkit 8.x-5.x, and reviewed this code again.

1. +++ b/src/Form/EntityEmbedDialog.php
   @@ -440,6 +441,22 @@ class EntityEmbedDialog extends FormBase {
   +      if (isset($form['attributes']['data-entity-embed-display-settings']['image_link'])) {
   +        $form['attributes']['data-entity-embed-display-settings']['image_link']['#type'] = 'hidden';
   +        $form['attributes']['data-entity-embed-display-settings']['image_link']['#value'] = '';
   +      }
   

   I could not find where 'image_link' is used in other places of the module. Could you add a comment about this, please?

2. +++ b/templates/entity-embed-container.html.twig
   @@ -6,10 +6,11 @@
   -<article{{ attributes }}>{{ children }}</article>
   +<article{{ attributes }}>{% if url %}<a href='{{ url }}'>{{ children }}</a>{% else %}{{ children }}{% endif %}</article>
   

   I am wondering if we could add {{ url_attributes|default('') }} to let other modules to inject attributes in element. So, we could avoid to override this template again when extending URL attributes in other modules.

1. If you're embedding an image, Drupal adds a 'Link image to' dropdown to the form. The options in this dropdown are "Nothing" (self-explanatory), or "File" (which links to the image file itself -- functionality that this patch covers). The "Link image to" dropdown would only serve to be confusing in this case, since it doesn't affect the functionality added by this patch, and might allow the user to accidentally put a link within a link. I've added a brief comment to the code explaining this.
2. Easy enough. Done.

Thanks @BartK.

I have added a new module to integrate with Linkit 8.x-4.x in https://www.drupal.org/project/entity_embed_linkit in case anyone interested in.

I tried using this patch but the link isn't using any of the classes or structure that Drupal provides so it makes it totally inconsistent and unpredictable. The link should be a render element, the url should be a Drupal\Core\Url object, and the link attributes should be passed in the #options array.

Sorry, I don't have time to continue making changes to this. Hopefully someone else can pick it up.

Updated patch that fixes #34

If this issue fixes #2764353: How to link entity to alternate resource and #2773489: How to link an entity? we should perhaps close the others as duplicate?
Does it?

If I'm reading the test results correctly -- and I could well not be -- the failure is simply caused by a missing route that is only used by the test. Otherwise, #36 seems to be working fine for me.

One possible issue I can see is that this results in a link option for every entity type. This won't make sense for some entities -- an embedded video, for example, or a custom entity type that contains link fields of its own (say, a Contact entity that includes email, web, and tel links).

Should the option to allow an entity to be linked be selectable somewhere? Perhaps the entity type form, or on the display mode?

I'm experiencing same as like #11.

Spoke too soon. Had manually patch applied and had missed a line.

OK, here's a problem I've encountered: if the link field is given an internal path (using, e.g., "/about-us" or "internal:/about-us"), the entity doesn't render. "internal:/about-us" works fine if UrlHelper::filterBadProtocol() is removed. And it kind of seems like using just the path without a protocol should also work, but it doesn't.

Any suggestions?

Confirming comment #43, I encountered the same issue.

This tries to solve #43 by setting "mailto" and "internal" as valid schemes before creating the URL.

1. We should add some validation to the link_url field.
2. What about using the '#type' => 'url' form element for link_url? See \Drupal\link\Plugin\Field\FieldWidget\LinkWidget::formElement

I have added an autocomplete to the patch of @wrd.
I have used code from \Drupal\link\Plugin\Field\FieldWidget\LinkWidget.
How do you find this idea?

Patch #47 working great.

Using Patch #47

I found that setting UrlHelper::setAllowedProtocols() (line 38 of entity_embed.module after applying the patch), resulted in other links in the menu which are not included in the protocol list throwing an exception. Namely it was a tel: link in the main menu.

In the attached patch I have simply removed that line as it is unnecessary and should use the global settings.

It would be great if this could work with Linkit 8.5

+1 for the #52, thanks.

#52 breaks when selecting a node with entity_reference_autocomplete. This results in a link_url in the 'node/7' form, which errors:

InvalidArgumentException: The URI 'node/7' is invalid. You must use a valid URI scheme. in Drupal\Core\Url::fromUri() (regel 281 van /Users/ruud/Groowup/minkema.nl/web/core/lib/Drupal/Core/Url.php).

I've added an UrlHelper::isExternal() check to template_preprocess_entity_embed_container which prepends "internal:/" for when it is an internal link. I don't know if this is the correct solution to the problem (only developing on Drupal 8 since 3 months), but it works for me.

Sorry, here's a new patch.

Added "'#maxlength' => 2048," to #60 to support longer links.

Test fails look unrelated.

I found a trick for images:

• I checked "link to image" checkbox in the view mode,
• I unchecked "Generate automatic URL alias" in the media form
• I chose my view mode when inserting with the WYSIWYG

Testing patch #63 works well apart from when entering URLs such as "/node/123". When the URL is parsed the host and path are incorrect.

I'm not sure what the best fix is here. I can replace $link = 'internal:/'.$link; with $link = 'base:'.$link; which fixes the issue with paths like "/node/123" but removes aliasing for entity routes:

function template_preprocess_entity_embed_container(&$variables) {
...
  if (!UrlHelper::isExternal($link)) {
    $link = 'internal:/'.$link;
  }
...

I've amended patch #63 to trim any preceding slashes on internal links:

function template_preprocess_entity_embed_container(&$variables) {
...
  if (!UrlHelper::isExternal($link)) {
    $link = 'internal:/' . ltrim($link, '/');
  }
...

#68 seems to work as intended. Thank you!

Just closed #2764353: How to link entity to alternate resource as a duplicate. #2773489: How to link an entity? was already marked as a duplicate a long time ago.

Bumping priority since this is something many people are clearly running into.

#2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin should land first, will at minimum simplify this, and may even solve this for us.

I started working this, and compared this with how Drupal 8's drupalimage plugin does this. It achieves this mostly thanks to https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/image2/plugi... doing the bulk of the work.

If I port key bits of what that does over to the drupalentity CKEditor plugin and widget (reusing the same code is impossible because CKEditor image2 code is assuming it's dealing with <img> all over the place), I can get it to work. But … one key assumption rooted in the inception of this Drupal module prominently gets in the way: the <figure> tag is not added on the client-side, but on the server side: the Entity Embed module sends some HTML to the server and asks it to pass it through the filter system, which results in <figure> and so on being added on the server side. This means the client side has to treat it as one immutable blob. Which results in not just the embed being linked, but also the caption itself turning into a link.

So alas, this CKEditor plugin must implement it differently; it has no choice but to decorate the original HTML and ask the server to re-render it.

Attached is the patch for this failed attempt. And here's a screenshot showing how it breaks:

Thanks to the work in #73, I ended up with #2544020-18: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin. That made #2282957-13: Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog) stupidly easy.

But it also made this possible, and much is inspired by and very similar to image2 and drupalimage.

Blocking this on #2282957: Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog), which in turn is blocked on #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin.

This is basically fully working (albeit with rough edges):

One of those rough edges is that it currently requires a core patch, because drupallink is hardcoded to interact with drupalimage …

A bit in the parts documentation was not yet clear, and much more importantly, #2544020-25: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin made it possible to avoid fetching a new preview when that's not necessary; this takes advantage of that for linking support.

This patch is relative to #2544020-25: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin and #2282957-19: Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog).

I've been working on cleaning this up further, and I've got it working quite nicely. For a long time, I thought we could just not integrate with the existing drupallink command since core/modules/ckeditor/js/plugins/drupallink/plugin.js has a hardcoded integration with drupalimage.

But I found a way around that!

🥳

So the core patch that #74 refers to is no longer necessary!

The parts.link stuff was incomplete, and … turns out we don't actually need it! This was just a necessity in the drupalimage plugin based on its overall DOM-centric approach. The approach in #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin — validated by the CKEditor team — does not rely on that. So we can make this completely functional with a much simpler approach.

Note that due to pre-existing focus issues (which also cause #3060245: [upstream] CKEditor's "undo" functionality not working correctly for embeds: offers multiple undo steps where there should be only one), the unlink button state is not updated after decorating an embedded entity with a link. Explicitly focusing it fixes that though. We'll need to fix that in a follow-up. I spent hours trying to figure out a work-around, but … turns out that the existing drupalimage plugin has the same problem 🤦‍♂️

This is now working 100% the same as the link support in drupalimage!

Alas …

var originalGetFocusedWidget = CKEDITOR.plugins.drupalimage.getFocusedWidget;

fails when the drupalimage plugin is not loaded. (Yay for test coverage!)

I guess … that … if that doesn't exist, we can just define it? 😏

#2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin and #2282957: Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog) just landed, let's test this.

The combined patch was green already, the non-combined one just came back green too!

🚢🥳

I've credited everyone who participated in this issue. For a while, it went a direction different than what Drupal core did. It was blocked on people finding the time to dig into CKEditor to figure out how to achieve that. All patches prior to #73 tried to achieve it using Twig templates and additional dialog options. That meant they weren't truly integrated with the rich text editor experience, at least not to the same extent as the image uploading and linking experience in Drupal core was.

As you can see in #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin and #2282957: Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog), it also took me a lot of effort to dig into the CKEditor code base to make this happen, and I actually had worked on CKEditor plugins before (albeit five years ago).

I recognize this may cause some inconvenience to those who have already adopted one of the patches prior to #73. But those never got committed, so it never was actually supported by the module. Rather than sticking to that approach, I believe it's better in the long run to match what Drupal core does, which also yields a significantly better content authoring experience.

I did make this an RC1 blocker (see #2577891: Entity Embed 8.x-1.0.0-rc1 release) because this issue had by far the highest number of followers (currently 83!), and because it is necessary for feature parity between embedding an image "directly" using core (drupalimage CKEditor plugin) and embedding a media entity that happens to represent an image.

Looking forward to your feedback when you try RC1 :)

Hey Wim, just wanted to say THANK YOU for picking this up and investing a lot of time into this issue. The sulution is great and another wonderful step in Drupal 8 progress!

THANK YOU! GREAT WORK! :)

@Anybody: I'm assuming this means that you actually updated to RC1 and used it, and most importantly … were satisfied? :)

Exactly @Wim Leers, it works great! :)

One little point: The update text might be improvable, it was a bit strange...
"entity_embed 8004 hook_update_n Installs the fake entity type."

Thanks! ;)

Oh, that'd good feedback! Could you please open an issue for that so we can fix it? (That update hook is unrelated to this particular issue, but it shipped with RC1, and indeed should be clearer.)

I believe there is some broken logic in this patch...although I'm new to this issue so I might be missing something.

js/plugins/drupalentity/plugin.js lines 73 - 90 (on RC1 which includes the patch)

      var originalGetFocusedWidget = null;
      if (CKEDITOR.plugins.drupalimage) {
        originalGetFocusedWidget = CKEDITOR.plugins.drupalimage.getFocusedWidget;
      }
      else {
        CKEDITOR.plugins.drupalimage = {};
      }
      CKEDITOR.plugins.drupalimage.getFocusedWidget = function () {
        var ourFocusedWidget = getFocusedWidget(editor);
        if (ourFocusedWidget) {
          return ourFocusedWidget;
        }
        // If drupalimage is loaded, call that next, to not break its link command integration.
        if (CKEDITOR.plugins.drupalimage) {
          return originalGetFocusedWidget(editor);
        }
        return null;
      };

It's possible for line 87 'return originalGetFocusedWidget(editor);' to get called even if 'originalGetFocusedWidget' was never defined as a function since CKEDITOR.plugins.drupalimage is set in the else (line 78) even if it wasn't previously set. I'm not familiar enough with what this section is trying to solve to provide a solution that won't break something else, but maybe changing line 86 to:

if (originalGetFocusedWidget) {

would do the trick since it would be null if CKEDITOR.plugins.drupalimage isn't loaded.

I can open this as a new issue if necessary, but I thought those who knew most about this patch would still be watching here.

Thanks!

#87: Well-spotted. We still need JS test coverage for that edge case; that wasn't done by #3060396: Add CKEditor Widget JS test coverage. It'd be wonderful if you could create a new issue for it! 🙏

Thanks everyone who worked on this!

I opened #3061449: Prevent drupal links being added to embedded entities that contain links when rendered as a follow-up improvement.

Apologies if I'm missing something obvious -- but: is there a configuration change needed to get the link field added to the embed wizard? Do I need a separate patch? I have the latest release (8.x-1.0-rc2). I am using am custom embed button in ckeditor. No link field. Thanks!

You have to enable the drupalink plugin in your editor. For example, if you're using full_html, edit that editor and drag the link button to the allowed buttons.

Thanks oknate. If anyone stumbles across this who was using the old patch, note that the workflow is now different. You need to add the drupalink button to your editor. Then, embed the media (don't expect the link field in this wizard), click on the image and then click on the drupalink button. If you have the linkit button already, you will end up with two link buttons in ckeditor.

I'm not having any luck with the new workflow. Clicking the embedded entity has no apparent effect, and then following up by using the link button simply inserts the link above the embedded entity.

I had both LinkIt and Advanced Link installed, but have uninstalled them for testing; no change, however.

Back to use the patch from #47
Not sure if this could be in a new feature issue.

A follow up issue as a request for a new feature.
#3070343: Allow to add a Drupal core link with Link Widget for Image entities using Entity Embed Dialog in CKEditor

@wrd, try this patch: https://www.drupal.org/node/3068469

The patch is already applied in the module but in my instance but it doesn't work. Drupal version: 8.6.16

Hmm I'm also not able to get this to work. Using core's link dialog, nothing happens after inserting the link. The link is not inserted at all. Using LinkIt's link dialog, the link gets inserted above the embedded entity. What's strange is that I believe this was working previously. Some module or core update may have broken it.

Edit: After triple checking all my module versions, I have this working again. I think I needed to update the entity embed module to 1.1 in conjunction with Drupal 8.8.

Re #31
If you created a bunch of content with patch #31 - here is a Backward compatibility layer patch https://gist.github.com/podarok/47b9e38df6570a31d1ea87fea250d6c2 that provides content managers with a good error message for old content and keeps old untouched content working in 1.1 entity_embed

Just in case anyone is looking for LinkIt version 8.x-5.x compatibility: I think I found the solution directly in the LinkIt module.
Leaving the issue here for reference: https://www.drupal.org/project/linkit/issues/3290717

Hello,

I have rerolled entity-embed-link-2511404-68.patch.

Thank you!

We had some content on one of our projects using patch #31.
I rerolled the backwards compatibility patch from #104 to apply to 8.x-1.3.

Is it possible this is still broken in 2023? I'm using what seems to be a pretty straightforward configuration of CKeditor link with the entity embed. Adding a link to a drupal-media tag doesn't work, the link is placed above the media in the html. Am I doing something wrong or is this really still not working?

@rschwab are you using CKE5? It seems to no longer work with that :(

Backwards compatibility patch for 8.x-1.5

Found a small bug where it would set an error on the values array from the form_state; fixed it by pointing the error to the form field instead

old:
$form_state->setError($entity_element, $this->t('Link to is deprecated. Remove its value and use WYSIWYG Link button functionality over embedded entity.'));

new:
$form_state->setError($form['attributes']['data-entity-embed-display-settings']['link_url'], $this->t('Link to is deprecated. Remove its value and use WYSIWYG Link button functionality over embedded entity.'));

Attached the wrong patch file, oops!