Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.So I had a thought about adding links to images when embedding in the WYSIWYG
What if we add a link field to the Media browser's final page where you select the display mode, alignment, Alt Text and Title text that could be a URL. That way while you are finalizing you image you can add the link there and then it could be rendered outright. Then we could in theory add LinkIt to this field if the site has it enabled, or some admin page that would allow them to select either text field or linkit as the option.
Original idea came from this issue Linked images from the Media module in WYSIWYG having A tags stripped when rendered to template
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | add_link_field_to_embed_page-2881204-19.patch | 4.21 KB | emerham |
| |||
| #12 | Screenshot from 2017-06-01 11-22-44_blurred.png | 109.17 KB | emerham |
| #7 | Screenshot from 2017-05-31 09-39-20.png | 83.41 KB | emerham |
Comments
Comment #2
emerham CreditAttribution: emerham as a volunteer commentedIf there is interest in this we will commit time to work on it.
Comment #3
joseph.olstadsounds great, improved linkit integration would be super.
Comment #4
emerham CreditAttribution: emerham as a volunteer commentedSo I was able to turn media embedded in media_wysiwyg into links but I had to touch file entity in order to have the template write the link HTML. I really wanted to avoid doing this but I was unable to think of another way.
Comment #5
joseph.olstadDo you have the patches for file_entity and media? can we have a look?
Comment #6
joseph.olstadare you using file_entity 7.x-2.0?
Comment #7
emerham CreditAttribution: emerham as a volunteer commentedI am using file_entity 7.x-2.0 (dev).
I've add both modules patches as well as an image showing the field on the embed form.
I'm still looking into linkit options...
Comment #8
joseph.olstadok, the file_entity patch looks reasonable , I think we can commit that and push a new release with that.
The media patch looks reasonable as well.
Probably commit this in the next few weeks.
Thanks.
Comment #9
emerham CreditAttribution: emerham as a volunteer commentedI appreciate your optimism, but I want to do some security testing on it first, make sure no XSS, try to get linkit in as an option.
Comment #11
joseph.olstadsounds good, also have to review the test results, one fail.
Comment #12
emerham CreditAttribution: emerham as a volunteer commentedBy Modifying the file entity template i'm getting some bad rendering when editing. CKEditor is not rendering the images fully and stopping right after the
<a>. I can still rightclick on the media object in the editor and get back into editing the file. When hitting save on the embed model the image is then properly rendered in CKEditor.I have not had any issues when the page is rendered.
Comment #13
emerham CreditAttribution: emerham as a volunteer commentedgot that issues sovled, submitting for testing again
Comment #14
emerham CreditAttribution: emerham as a volunteer commentedLeft an alter in patch 13 that I ended up not using. Adding patch without the alter
Comment #15
emerham CreditAttribution: emerham as a volunteer commentedChanging status to get auto tests running
Comment #17
joseph.olstadIn the patch, what is the
//$external_url....
?
Commented out, is this just leftovers?
Comment #18
emerham CreditAttribution: emerham as a volunteer commentedyeah it was left in by accident.
So I just setup my own testing box and ran the tests with my patches and they all pass....
Comment #19
emerham CreditAttribution: emerham as a volunteer commentedTrying again for tests to pass
Comment #20
joseph.olstadso two patches, patch #7 and patch #19 ?
I'll have a closer look over the weekend maybe
Comment #21
emerham CreditAttribution: emerham as a volunteer commentedthe file entity patch in 7 and the media patch in 19. If you want I can make a co-issue over in file_entity and post the patch there.
Comment #22
joseph.olstadsure please do make a co-issue , and this one as the parent issue. Thanks :)
Comment #23
emerham CreditAttribution: emerham as a volunteer commentedremove file_entity patch from listing here
Comment #24
joseph.olstadpaste this in any comment block :
square bracket open #2883529 square bracket closeand get this:
#2883529: Linking Issues - related to 2881204
Comment #25
joseph.olstadtested, works great, thanks!
Comment #28
joseph.olstadrequires latest file_entity 7.x-2.x dev build and latest media 7.x-2.x dev or latest 7.x-3.x dev
Comment #29
brockfanning CreditAttribution: brockfanning commentedI think this is a great idea for a feature, but would it be possible to limit this to the 7.3 branch?
Also it seems like this is intended only for images, but is being applied regardless to all file types. Am I correct about that?
Comment #30
joseph.olstadYa it seems to apply to all file types, it (the option) appears on media_youtube video files, which is probably not desired in that case, but probably doesn't hurt either.
Comment #31
joseph.olstadI was about to tag a release for this.
@brockfanning, could you formalize your concerns in the form of a patch?
such as something optional.
I've tested out this functionality, seems to work fine.
Comment #32
joseph.olstadIf you want this functionality, it is in file_entity 7.x-2.1 and media 7.x-3.0-beta1
setting this to needs review for 7.x-2.x dev , @brockfanning, can you please review this for us?
Comment #33
joseph.olstad@brockfanning, can we deal with your concerns as a new issue?
I made an oops on the 7.x-2.4 release, ended up tagging the wrong branch. Aside from that, I've tested the 7.x-3.x branch , and lots of people are using it without issues, so as the 7.x-3.x functionality and improvements have had some bake time, I think its ready to take it out of the oven and merge into 7.x-2.x.
I'm cutting a release today.
Comment #34
brockfanning CreditAttribution: brockfanning commented@joseph.olstad: If it's going into 7.x-2.x, then I'll start a separate issue to make it optional. The reasoning is that I have sites where I've already dealt with the linking problem in a different way, and would not want to suddenly change the workflow (or worse, have 2 different workflows for the some purpose) for all my users. I wouldn't be able to update to a new 7.x-2.x version until this feature became optional, either by a patch or new release.
I think it would also be nice to prevent the option from displaying on non-image file types, though it's not as critical as the above issue. I haven't looked too closely at the code, but it doesn't seem like this should be that hard.
Comment #35
emerham CreditAttribution: emerham as a volunteer commented@brockfanning If there was a config option to enable this in the admin side would that help with the feature upgrade?
We don't enable embedding of other media types into the WYSIWYG which is why I never thought of testing other media types for the embeder. I did think about setting up a config variable to enable/disable this functionality but thought it would be better if it just was always on..
Comment #37
brockfanning CreditAttribution: brockfanning commented@emerham: Yes, the config option is what I would need. In recent history when we've added changes of behavior to Media we've given them config options that default to off (so that sites won't experience any unexpected changes) and then provided a message in a dummy hook_update_N function that alerts site builders that the new functionality is available.
Comment #38
joseph.olstadhere is the followup issue:
#2884412: add the config option for add embed link to make it optional
Comment #39
joseph.olstadfixed, see followup issue
#2884412: add the config option for add embed link to make it optional
Comment #42
jds1This link field is not working for me. Client reported an issue so I upgraded to the latest versions of Media (7.x-2.8), Media CKEditor (7.x-2.1), and File Entity (7.x-2.2). When I save a node the image is completely stripped out. Don't even see it when inspecting. I've messed with CKEditor settings and other options to no avail. Is anyone else having this issue?
EDIT: not sure why I haven't been able to add a comment but Emerham's suggestions totally worked below. Follow these steps if you're having issues: I needed to change some things around in the Media Browser/Text Format settings. Thank you, Emerham!
Comment #43
emerham CreditAttribution: emerham as a volunteer commented@if-jds I just created two new d7 sites and I am running the latest tag releases for all media/ckeditor related modules.
My settings are:
Media browser:
For CKEditor I am using the CDN and version //cdn.ckeditor.com/4.6.2/full-all
Media Plugin is enabled on the CKEditor text profile as well as for the text format.
The first filter on text format is Convert Media tags to markup
I just embedded and created multiple images and links to external/internal pages and my images all showed up.
Comment #44
jds1@emerham – this was SUPER-helpful. I needed to tweak some of the Media Browser/Text Format settings and I got it. Client site was not updated to latest versions but I was able to get this working locally and can now proceed.
Thank you!