Contrib.social

Adding links around embedded media through CKEditor might lead to invalid/complex markup when rendered

Open on Drupal.org →
Created on 16 August 2019, almost 5 years ago
Updated 21 May 2024, about 1 month ago

Problem/Motivation

The Drupal link CKEditor button can be used to add links to embedded media items. However, we can't guarantee that the rendered representation of that media item will be a single HTML element, or one that can be safely wrapped with <a> tags. As a result, editors can create links that have arbitrary HTML elements inside them.

Steps to reproduce:

1) Go to simplytest.me and spin a vanilla install of Drupal 8.8.x. Log in with admin/admin, install the "Media" and "Media Library" modules.
2) Go to /admin/config/content/formats/manage/basic_html?destination=/admin/config/content/formats and drag the "Insert from Media Library" button into the CKEditor toolbar. Enable the "Embed media" filter, and click "Save configuration".
3) Go to /admin/structure/media/manage/image/display and drag the "Authored by" field up into the visible fields area. Click "Save" to save this view display settings.
4) Go to /node/add/article to open the article node form. In the wysiwyg, click the "Insert from Media Library" button. Upload a file into the upload zone, give it an ALT text. Save and insert. At this point you will have something like this:

5) Click the media item to select it, then click the "Link" button from the CKEditor toolbar. Give it a random URL (such as https://drupal.org)
6) At this point, if you view the source markup you will have something similar to this:

<p>text before media</p>
<a href="https://drupal.org">
<drupal-media data-align="center" data-entity-type="media" data-entity-uuid="94a3ff34-a958-427c-ac51-127259cbc1e2"></drupal-media>
</a>

<p>text after media</p>

7) Fill in the required fields and save the node.
8) Inspect the markup of the node body, you will find something like this:

Proposed resolution

TBD

Remaining tasks

- Discuss and agree on a reasonable approach
- Implement it
- Review & commit

User interface changes

TBD

API changes

TBD

Data model changes

TBD

Release notes snippet

TBD

Original report from @wrd :

With the progress being made on #2801307: [META] Support WYSIWYG embedding of media entities → , I've been playing with the new capabilities and have run into what seems like an important issue for typical users: after embedding some media entities, particularly images, a user will probably expect to be able to link the media to another page. At present, this doesn't really work.

  1. Perform a Standard install.
  2. Enable the Media and Media Library modules.
  3. Edit the Basic HTML editor. Enable the "Embed media" filter, remove the "Image" button, add the "Insert from Media Library" button. <drupal-media data-entity-type data-entity-uuid> is added to the allowed HTML tags automatically.
  4. Create a new Image media entity in the Media Library.
  5. Create a new Basic Page.
  6. Use the "Insert from Media Library" button to embed an image in the body field.

  7. Click on the embedded image to select it.
  8. Click the Link button to link the image to a destination. I'll link it to https://drupal.org.

  9. When done, note that there's no visual indication that the embedded image has been linked to anything.

  10. Save and publish the node, and view it.

You'll see that the image is, indeed, linked to https://drupal.org. However, if you inspect the element, the <a> tag has been wrapped around the entire media entity:

<article data-history-node-id="1" role="article" about="/node/1" typeof="schema:WebPage" class="node node--type-page node--view-mode-full clearfix">
  <header>
    <span property="schema:name" content="Test Page 1" class="rdf-meta hidden"></span>
  </header>
  <div class="node__content clearfix">
    <div property="schema:text" class="clearfix text-formatted field field--name-body field--type-text-with-summary field--label-hidden field__item">
      <p>This is a test. </p>
      <a href="https://drupal.org">
        <article class="media media--type-image media--view-mode-full">
          <div class="field field--name-field-media-image field--type-image field--label-visually_hidden">
            <div class="field__label visually-hidden">
              Image
            </div>
            <div class="field__item">
              <a href="https://drupal88.dd:8443/sites/default/files/2019-08/pears_1.jpg">
                <img src="/sites/default/files/2019-08/pears_1.jpg" width="640" height="480" alt="Pears" typeof="foaf:Image" />
              </a>
            </div>
          </div>
        </article>
      </a>
      <p>This is only a test. </p>
    </div>  
  </div>
</article>

While we can wrap block elements in <a> tags in HTML5, the result here is problematic. The image itself is (by default) linked to the raw image file, so we've got nested hyperlinks, which AFAIK isn't permitted in HTML5. And browsers (Safari, anyway) will ultimately render this as multiple separate <a> tags, most of them empty or wrapping whitespace.

It seems like the expected default behavior for a typical user would be that after using the Link button, there would be some kind of visual indication in CKEditor that the entity is linked, and the rendered page would simply have the image linked to the destination specified using the Link button.

However, to complicate matters, we have to anticipate the possibility that the media entity being embedded is more complex than just an image. For example, what if I have a custom media type that looks like the Image type, but has a few more fields:

  • Copyright (date field)
  • Location (geolocation field)
  • Photographer (entity reference field)

...and a default view mode that displays the image, the copyright date, the location, and .

If I embed this entity, and then use the link button, the desired behavior is for the image to link to the URL entered using the link button, while leaving the link to the Photographer node untouched so that it still works.

I'm not sure if this more complex case can be handled in any reasonably generalized fashion that'll work for most users. However, the more basic behavior of simply linking an embedded image entity should probably work out of the box, shouldn't it?

I hope this is useful! Let me know if I can clarify anything.

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
Media  →

Last updated about 13 hours ago

Created by

🇺🇸United States wrd

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇳🇮Nicaragua edysmp Nicaragua

    #28 Fixes my issue. Now media images can be linkable. Thanks!

  • Comment about 1 year ago →
    🇺🇸United States bkosborne New Jersey, USA

    I wonder if anyone has tested how this works in CKEditor 5? Maybe it's easier to accomplish there?

  • Comment 10 months ago →
    🇺🇸United States scotwith1t Birmingham, AL

    Still seeing problems with this in CKEditor 5 with D10.1.2. While adjusting the order of the filters does link the image (Yay!) the markup is still broken.

    This is compounded and becomes particularly visible when using ExtLink module, where the broken markup results in multiple external link icons being added.

  • Comment 9 months ago →
    🇺🇸United States cewernlund

    Just wanted to say the comment in #28 worked for me. I was thinking something in the Text Editor format might be the culprit and this proves it. Many thanks for reporting it!

  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.2 & MySQL 8
    last update 9 months ago
    30,358 pass, 2 fail
  • @jayhuskins opened merge request.
  • Comment 9 months ago →
    🇺🇸United States jayhuskins

    Here is a patch for 10.1.5 that simply removes the ability to link media.

  • Comment 9 months ago →
    🇺🇸United States scotwith1t Birmingham, AL

    I don't know about others but, to me, removing the ability to link media altogether is not a viable solution. Linking media is a critical function for many sites.

  • Comment 9 months ago →
    🇩🇪Germany marcoka

    @scotwith1t yes, i agree, of course that is an important function. Linking media is used everywhere :)

  • Comment 7 months ago →
    🇺🇸United States loze

    I think all the options available in ckeditor5 (edit, link, align, caption, etc.) should all be configurable on the media bundle, or view mode.

    Is that possible?

  • Comment 7 months ago →
    🇺🇸United States scotwith1t Birmingham, AL

    @loze While that would definitely be a great feature, it doesn't really fit into the scope of what's being addressed in this issue. See https://www.drupal.org/project/media_embed_view_mode_restrictions → contrib module which was created out of #3308069: Provide per-bundle configuration for embeddable view modes → as a potential solution (tho it doesn't have a D10 release yet).

  • Comment 7 months ago →
    🇩🇪Germany rgpublic Düsseldorf 🇩🇪 🇪🇺

    Well, the main problem seems to be (IMHO) that way to many field templates (e.g. core/themes/stable9/templates/field/field.html.twig and many more) in Drupal contain a div tag when they should better use a span tag, I guess. A span tag would be more correct because a whole field, for example, is an inline element. There can be multiple fields in a row. As long as they are all using div tags, they can never appear inside a p tag :-(

  • Comment 6 months ago →
    🇺🇸United States millerrs

    I get the same broken markup in #43 with 10.2.1. My filter order is the same as suggested in #28.

    While I have limited knowledge of CKEditor's mechanics, the proposal in #18 to utilize an attribute for the link appears feasible and aligns with the methods used for alignment and other options.

  • Comment 5 months ago →
    🇺🇸United States chuck_theobald

    On Drupal 10, CKEditor 5, fully updated Media tooling:

    FWIW, the issue I am seeing is a link with no content, followed by the image.

    Seen in CKEditor Source mode:

    test media image link

      →

    Delivered to the browser:

    test media image link

    →




    Moving "Convert line breaks into HTML (i.e.
    and

    )" to last in the processing order fixed the linking issue. My filter processing order:

    Convert URLs into links
    Advanced Insert View
    Restrict images to this site
    Align images
    Caption images
    Embed media
    Display embedded entities
    Linkit URL converter
    Correct faulty and chopped off HTML
    Convert line breaks into HTML (i.e.
    and

    )

  • Comment 5 months ago →
    🇺🇸United States mortona2k Seattle

    I ran into a similar issue as above.

    I have a media view mode that displays an image linked to the file.

    When it's embedded, the link is stripped.

    The issue was with the order of filters, after moving things around the link worked.

  • Comment 4 months ago →
    🇩🇪Germany drubb Sindelfingen

    I did something similar like in #28 or #54. While the image is correctly displayed and linked, there are still additional empty links before and after the image. Something like

    <p><a></a></p>
<p><a>Rendered image</a></p>
<p><a></a></p>

    It's weird!

  • Comment about 1 month ago →
    🇺🇸United States caspervoogt

    I was running into issues with linked media images not rendering as linked images. Disabling the 'Convert line breaks into HTML' filter fixed that for me. It was not enough for me to move it to the end of the list of filters. I also tested putting it first. Only disabling that filter solved it for me.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024