Cannot put link on an image added with media

I believe this is actually caused by a quirk of HTML, which is that closing </p> tags are optional, and any other opening block-level tag after the opening <p> is interpreted by browsers as an implied closing of the paragraph. For example:
<p><div>Foo</div></p>
can be interpreted by browsers as something like:
<p></p><div>Foo</div></p>

When links get involved, it gets even wackier. From my perspective it seems like the simplest solution is to remove <p> tags from around embedded media. The attached patch adds a new (optional) text filter that can be enabled in your text format settings, and if positioned before the normal Media filter, will remove the paragraphs from around embedded media.

Also, I could have sworn there was an existing ticket about this in the Media queue, but I can't find it at the moment. Moving this one over the Media.

Forgot to move the issue to the media project... Anyone know how to make it run tests against media instead of media_ckeditor?

Posting this again to hopefully apply it to the correct project.

I was able to recreate the issue, but at least for me, applying the patch and enabling the filter on the Filtered HTML Text format did not address the issue. Maybe I'm missing something?

What other filters are enabled, and what is the order? Also can you paste in the relevant chunk of HTML code?

Attached is the current filter order. When I noticed nothing had changed, I dragged the "Convert URLs into links" filter to the bottom, but it had no effect.

<div id="file-1" class="file file-image file-image-jpeg"><a href="http://example.com">

        </a><h2 class="element-invisible"><a href="http://example.com"></a><a href="/file/1">headshot.jpeg</a></h2>
    
  
  <div class="content">
    <a href="/file/1"><img height="220" width="220" class="media-element file-teaser" typeof="foaf:Image" src="http://d7.sd/sites/default/files/styles/medium/public/headshot.jpeg?itok=9enE0-7a" alt=""></a>  </div>

  
</div>

There may be a couple of issues, related to anchor tags being within anchor tags, which makes browsers do wacky things.

1. The file template has that h2 which contains a link. This is baked into file_entity.tpl.php in the file_entity module, unfortunately. The usual way to get rid of that is to override it in your theme, and remove the h2. Note that if you want to override it for images only, the duplicated file would be called "file--image.tpl.php" (not "file_entity--image.tpl.php" as you might expect). It would be nice if we could resolve that without overriding the template -- we could patch file_entity to remove that h2, but that would surely never get committed.
2. The img tag is also wrapped in an anchor tag. This I believe you should be able remove by changing the "Manage File Display" settings on the image file type, under "Structure".

Your goal is to make sure that the normal rendering of the file entity has 0 links in it, so that you can have valid HTML when the whole thing is wrapped in a link.

To clarify, the patch I posted here is only concerned with paragraph tags around the file entity. It doesn't do anything to anchor tags, and doesn't do anything to the markup inside the file entity.

I can confirm that everything is working as expected after applying the patch and then overriding the file_entity.tpl.php! Thanks for figuring out the underlying issue! I kept just seeing workarounds that didn't address the main issue of the invalid HTML.

After applying the patch I get this error:
PHP Fatal error: Cannot redeclare media_wysiwyg_filter_paragraph_fix() (previously declared in sites/all/modules/media/modules/media_wysiwyg/includes/media_wysiwyg.filter.inc:23) in sites/all/modules/media/modules/media_wysiwyg/includes/media_wysiwyg.filter.inc on line 56

My previous configuration works with these versions of the modules:
CKEditor 7.x-1.16
CkEditor Link 7.x-2.3+10-dev
CKEditor Link for Files 7.x-1.3+3-dev
Media CKEditor 7.x-2.0-alpha1 is disabled
Media 7.x-2.0-beta1
Media WYSIWYG 7.x-2.0-beta1
Entity API 7.x-1.6
File 7.44
File Entity 7.x-2.0-beta2+13-dev
File Styles 7.x-2.0-alpha8
WYSIWYG Filter 7.x-1.6-rc2

Let me know if I'm missing a module for linking files to images in CKEditor (standalone).

@NeuQuest, I'm not sure what's happening there, but it seems like maybe the patch was somehow applied twice? It sounds like the same function is appearing twice in the file.

For what it's worth, I've stopped using this patch. Rather than try to support embedded images being entirely wrapped in links, I've added a link field to the Image file type, and am using custom code to output the file entity with a link according to the data in that field.

@brockfanning, does that work with CKeditor though?

I'll re-download Media and apply the patch again to see if that fixes the issue.

@NeuQuest: To clarify my alternate approach: I've got a link field on the image file type. When users see the Media popup as they are inserting/editing an image into Ckeditor, they see that link field and can put a URL into it. Then later, when the image is being rendered, I have custom code that makes the image into a link. Here's a snippet of that code (in my case, the link field is called "field_image_link"):

/**
 * Implements hook_media_wysiwyg_token_to_markup_alter().
 */
function mymodule_media_wysiwyg_token_to_markup_alter(&$element, $tag_info, $settings) {

  $file = $tag_info['file'];
  // We only care about image files outside the wysiwyg here.
  if (!empty($settings['wysiwyg']) || empty($file) || 'image' != $file->type) {
    return;
  }

  // Do we need to wrap the image in a link?
  $wrapper = entity_metadata_wrapper('file', $file);
  $link = $wrapper->field_image_link->value();
  if (!empty($link['url'])) {
    $url = url($link['url']);
    $element['content']['file']['#prefix'] = sprintf('<a href="%s">', $url);
    $element['content']['file']['#suffix'] = '</a>';
  }
}

I should say though, there are a lot of patches needed to get Media+Ckeditor working in general, so I've been trying to document that in #2730285: A working Media + Ckeditor recipe.

In my case, the link is being completely removed, even when using the PHP filter. Thus in my case, the patch wouldn't work in any case since it's not a formatting issue. Perhaps I should file a new bug, but I'm having the exact same issue as stated in the title.

The workaround was to add the image with the image plugin vs the media browser.

I have a page that shows both for reference: http://dev.mysourcewise.com/ (see the public authority images at the bottom of the page).

Here is the pre-rendered markup shown by disabling the editor (or with the PHP filter enabled):

<p><a href="public-authority-services">[[{"fid":"18","view_mode":"hg_media_image_original","fields":{"format":"hg_media_image_original","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","field_folder[und]":"1"},"type":"media","link_text":null,"attributes":{"height":61,"width":411,"style":"width:
  411px; height: 61px; border-width: 0px; border-style: solid;","class":"media-element
  file-hg-media-image-original"}}]]</a></p>

  <p><a href="public-authority-services"><img alt="" src=
  "/sites/default/files/pas-logo.jpg" style=
  "border-width: 0px; border-style: solid;" /></a></p>

You can see for yourself the <a></a> has been removed from the markup.

Ironically, I find if I highlight the image and click the clear formatting icon in the WYSIWYG editor, it works.

I have tested the patch https://www.drupal.org/files/issues/cannot_put_link_on_an-2707107-4.patch and it works well.

Doesn't look like this was committed to the latest dev releases (2016-10-25). It patches without an error, but when I run crush I get this error:

Fatal error: Cannot redeclare media_wysiwyg_filter_paragraph_fix() (previously declared in /home/gsigadmin/d7t/sites/all/modules/media/modules/media_wysiwyg/includes/media_wysiwyg.filter.inc:33) in /home/gsigadmin/d7t/sites/all/modules/media/modules/media_wysiwyg/includes/media_wysiwyg.filter.inc on line 66

Yet when I run the release without the patch I have the same issues as above. It turns what previews fine in CKeditor:

<p class="rtecenter"><a href="/files/docs/zonemap2016-10-17small.pdf" rel="nofollow">Click to view as larger PDF</a></p>

<div class="rtecenter"><a href="/file/659" rel="nofollow">[[{"fid":"658","view_mode":"default","fields":{"alt":"Zone / District Map","title":"Zone / District Map","height":800,"width":434,"class":"media-element file-default"},"type":"media","link_text":null,"attributes":{"alt":"Zone / District Map","title":"Zone / District Map","height":800,"width":434,"class":"media-element file-default"}}]]</a></div>

Into:

<p class="rtecenter"><a href="/files/docs/zonemap2016-10-17small.pdf" rel="nofollow">Click to view as larger PDF</a></p>

<div class="rtecenter"><a href="/files/docs/zonemap2016-10-17small.pdf" rel="nofollow"><div class="media media-element-container media-default"><div id="file-658" class="file file-image file-image-jpeg">

<h2 class="element-invisible"><a href="/file/zonemapjpg">zonemap.jpg</a></h2>
    
<div class="content">
    <img alt="Zone / District Map" title="Zone / District Map" height="800" width="434" class="media-element file-default" src="/files/images/zonemap.jpg" />  </div>

</div>
</div></a></div>
</div></div></div>

@NeuQuest: I don't particularly think my patch in #11 would be appropriate to be committed. It requires too much knowledge of HTML, I think, for the user to know when/if/how it's needed. If it's useful to people, maybe it could be a separate module. But personally I have stopped using it. In fact, I'll change this ticket to "active" to see if we can come up with better solutions or explanations.

I'm not sure why you're still getting that error. Is that function actually declared on both lines 33 and 66, like the error message says? If so, that points to some problem happening while applying the patch, maybe.

But regardless, from what I can tell, the markup that Media is outputing for you actually seems pretty close to OK. If you look close at the markup in the description of this issue, those <a> tags were getting closed unexpectedly. Your output seems much better - your <a> tag is surviving just as would be expected. My suspicion is that you are pasting your markup from a "View source" of the page, and the original issue description's markup was pasted from a browser's "Inspect element" console. That unexpected closing of <a> tags, as far as I know, is something that browsers do as they are trying to render invalid HTML.

So I believe the solution to your problem is to get rid of the <a> tag in the <h2> of your file template. This is what is causing the invalid HTML (a link within a link) which I think is making browsers mangle (close unexpectedly) your <a> tags. As I mentioned in earlier comments, the <h2> link is baked into the file_entity template, so the best way to get rid of it is to override the template in your theme.

I'm leaning towards, as a better solution for this problem, a patch in the file_entity module that adds some logic in the template that would allow that <h2> link to be skipped, so that we can more easily work around this problem from a module, rather than having to tell everyone to fix it in their theme.

I noticed the issue was the <h2> tag also. I'm not sure why the file_entity template is even adding the <h2> tag in the first place since it has an invisible class (for SEO?). I'll override the template and see if that fixes the issue and hoping for a patch to file entity.

This jogged my memory, when using the wetkit distro we came across this problem and fixed it in our implementation of media. It now works out of the box in the wetkit distro.
#2624520: Bean WYSIWYG internal uuid links for images/media/file

This issue comes up when some custom theming breaks default behavior and for the solution please see this comment.

This is the actual commit spoken of in that comment.

The commit was to add a new subtheme tpl, the explanation is in this aforementioned comment.

Here's the patch from #11 (originally #4) re-rolled against media 7.x-2.0-beta6 for anyone who still needs to apply this workaround.

I'm still getting this problem with media-7.x-2.0-beta8, in which it should be fixed. Also using media_ckeditor 2.0-alpha1 and ckeditor 1.17

I've enabled the new "Ensure that embedded Media tags are not contained in paragraphs" filter, put it before the filter as suggested. Still no luck.

I've checked and in the file display settings for image, no a wrappers in there.

Am I correct that I would need to override the default template file and that would fix it? If so, why isn't this done in the media_wysiwyg module? I also couln't find this step documented anywhere, so I believe this can only be considered fixed if it's at least documented.

Upgrade your media_ckeditor to alpha2
Try that first. Let us know if that helps

If not then switch to beta6 that didn't have this patch

related issue ?:
#2357993: Linked images from the Media module in WYSIWYG having A tags stripped when rendered to template

if you are experiencing what @daften describes
Try:
upgrade the media module
upgrade your media_ckeditor module
upgrade your file_entity module

then follow instructions here:
https://www.drupal.org/node/2357993#comment-11788118

if this doesn't help, please followup with a detailed response.

Thanks.

Appears that some people might be still having an issue, see related issues

for the latter part of this thread /issue see this related issue #2357993: Linked images from the Media module in WYSIWYG having A tags stripped when rendered to template

patch #35 in that issue needs a reroll / refactor , this will do the trick.

Hello. So Drupal still has the issue with placing hyperlinks around media entities (images). I chose to comment on this thread because it seems to closest address the issue that occurs when I inspect the page to take a look at what's going wrong. The issue for me is that the

tag is being added around the link. The image seems to be ok as it is surrounded by a div instead. For example:

link to web page

Can anyone help with this? I'm still new to Drupal.