With Media WYSIWYG enabled - "Contextual links" are shown for anonymous users

Hi mori,
yes, I can confirm this. Because yesterday I tried to update our default installation profile to use latest dev versions of File Entity and Media. After updating on a test site I had the same problem.

I've also tried on a clean install and again had the problems. Next, I reverted File Entity to 2.0-beta1, like we had before (and left Media at the latest dev), and the problem was gone!

I will try to find out, which change could have caused this.

I've moved this issue to File Entity module

Sorry, my fault - moving it back to Media, because it also happens with beta1. I Just tricked myself before...

I've found the problem! It was introduced by https://www.drupal.org/node/2194821

Attached is a patch, which removes the contextual links from output rendering. However, there's still the problem, that the full template file is used, including all the unnecessary divs.

I've just seen that there's a comment with a sample implementation of hook_token_to_markup_alter(), that removes all this stuff - haven't tried it so far, but looks promising for a workaround as long as this problem exists.

added https://www.drupal.org/node/2408629 as related issue

#11 is working for me, thank you!

Sorry, typo.

I can't comment on either approach however I'm using the patch in #15 and it stopped my unwanted contextual links.

I searched for exactly opposite solution: to enable users to use contextual links on the embedded media (wysiwyg). The problem was that the contextual links where improperly displayed (as reported by mori). Finally I managed to get it working, without any patching or hooking -- the problem was in input filter configuration, which, by default, removes div tags. Maybe it is not the best solution, but at least I can easily enable: h2, img, div tags, so that the file entity is output is ok.

Edit: Once the input filter is configured, the contextual links are displayed if the user has proper permissions.

I've applied patch at comment #15.
It is working fine.
Thank you...

Patch #15 working for me.

Hi, i've applied patch at comment #15.
Its working fine.
But when i tried to add caption for image field at admin/structure/file-types/manage/image/fields, It didn't display caption field. Because this batch was removed view mode. So it is happening. To display caption for a image, then you don't apply this batch.
Now the caption is displayed well under image.
So you have to find another perfect solution for avoid displaying of contextual link.

Concur with comment #22 - patch #15 removes the contextual links, but also removes the display of any other fields associated with your media content. Patch #11 looks more focused towards simply unsetting the contextual links...

I haven't actually looked into the problem properly but just by looking at these patches neither of them really seem right.

If a user doesn't have the permission to use contextual links, then they shouldn't be showing in the first place. We shouldn't have to make some hack to strip them out.

My question is what are we doing wrong in the rendering of the file entity so that these contextual link permissions are not being honored?

Shouldn't contextual_preprocess() handle this at some point in the chain?

Ideally we would want entities in WYSIWYG to display working contextual links like they would anywhere else.

Also the issues linked as the cause of this one has been reopened to possibly revert that change.

Still seems like a bug that the permission isn't used properly in the rendering though.

Ran into the same issue.
Is it that input filters aren't configured right as suggested in #19
Is it a code regression from a previous commit #2194821-21: Embedded media objects should honor display suite settings
Is the fix patch in #11 the right way to go or covering for a problem elsewhere?

We need to check the for current user ID before hiding the contextual links.
check attached patch.

These contextual links don't show to other than anonymous which is the odd piece here.

Patch in #27 works for images, but not for video. With an embedded video, contextual links are visible to anonymous users.

I recommend mglaman's workaround: #2408629-3: Media filter always renders full template output

You should also keep an eye on the following issue, as this is the root of this problem: #2194821: Embedded media objects should honor display suite settings, which will either be reverted or a solution for backwards compatibility must be found (see comment #21)

@agoradesign Yes, the two issues linked to in #30 are useful. The first link seems to be reverting the change while elsewhere that thread seems to be embracing the change.

Also, when the contextual links appear (see attached screen capture) they appear as menu links not the way contextual links typically appear as a cogwheel.

Contextual links are not just shown to anonymous users but to other users who should not see them.

I agree with comment #24 by @rooby it is not ideal to unset the variable but we should be looking at the root cause.

I believe that rendering contextual links is always bad for embedded entities within text fields. Afaik the output of text filters is quite heavily cached, and I don't think that there's a possiblity to control caching of filters by role. So I fear that showing the contextual links could lead to big problems anyway.

For what it's worth, the patch over in #2194821-28: Embedded media objects should honor display suite settings has reached RTBC status and fixes this contextual links thing...

... Except that I've just noted back there that it breaks my embedded images so they don't get their view mode-defined fields (caption, in this case) and wrapper. So, all or nothing at the moment: either you get the full entity rendered, but it comes with contextual links (current dev versions of everything, no patches), or else you can get rid of the contextual links, but you also lose all your fields (patch over in #2194821-28: Embedded media objects should honor display suite settings).

Yeah at first glance (without actually testing anything) I can't say I like the look of that patch in the other issue.

I've composed a longer summary post to help newcomers understand how all of these issues are tied into one another, but don't know where to post it. In the meantime, after a day and a half of testing and reading:

@agoradesign's patch in #11 seems to be the only one in this thread that offers a suitable workaround for the contextual links issue, until something else happens in the other issues. All it does is unset the #contextual_links parts of the render array, after it comes back from file_view() in media_wysiwyg_token_to_markup(). Yes, it removes contextual links for everyone, but there are limited alternatives that don't either scupper the way forward or have negative performance impacts.

@ro0NL's patch in #15 just does what the apparently very wrong patch over in #2194821-28: Embedded media objects should honor display suite settings does: completely eliminates any view-mode-based field rendering for embedded file entities. file_view() no longer gets called (so the contextual links are no longer rendered), but then field_attach_prepare_view() isn't put back, so we never get the fields.

@mqanneh's patch in #27, which tests global $user won't work as it stands, since file_view() - the generic renderer for entities - is called , and it essentially renders and caches the links based on the permissions of the user who initially created/embedded the content: the user creates content, embeds an entity, hits 'save', and lands on the node_view(), which kicks off the whole rendering process based on that user's permissions, and then... the now-rendered token is cached for a Very Long Time since media_filter_info() does not declare an explicit 'cache' => FALSE, caching occurs by default (see the D7 API docs for hook_filter_info()), and thus those contextual links are being shown to everyone. You can choose to implement hook_filter_info_alter() and turn caching to FALSE for the media tags filter, but then you're hit with the full ..._token_to_markup() pipeline every time someone views the node (depending on your overall site caching regime).

Whew!

I really don't see a solution to the contextual links issue other than #11, if the changes committed over in #2194821-8: Embedded media objects should honor display suite settings stand (with whatever configuration options might arise - see #2194821-21: Embedded media objects should honor display suite settings).

Hope that helps - and if I'm wrong on any of the details, please correct me - this has been more of an effort for me to understand than it was to be authoritative (which I most definitely am not).

I agree that #11 seems closest to the right solution to me however I still have my concerns from #24.

Like I say, run up a hook_filter_info_alter() and turn off caching of the media tag filter, and I'll bet the problem will go away (without #11). I'm just reluctant to do this for the site I'm working on, until I can confirm the performance impact - and in any case, this would need to be configurable.

Yeah that sounds like a very likely explanation.

Confirmed: you can get your contextual links to appear correctly for the right users if you implement something like the following (BUT SEE THE WARNING FOLLOWING):

function MYMODULE_filter_info_alter(&$info) {
  if (isset($info['media_filter'])) {
    $info['media_filter']['cache'] = FALSE;
  }
}

Note that you'd also have to visit your text formats and hit 'save' again in order for this cache change to take effect.

WARNING: This has serious performance implications for larger sites. I had thought that perhaps only individual filters were cached granularly. In fact, any text format that uses even a single filter with ['cache'] = FALSE is itself no longer cached at all. Which makes sense on reflection. So, if you have many filters configured to apply for your content's text format (i.e., Filtered HTML), and you do the above, all of those filters are re-run whenever that entity is rendered.

So: Not such Good News for those of us who would still like contextual links on our embedded media objects, but at least we know why this is happening.

I haven't looked into the feasibility but if there was a way to have cache per role for input filter caching that should be a viable solution.

Yes, that would indeed be interesting and potentially useful for other cases... One the sites I work with allows images to be uploaded/embedded, but we struggle with approval status (do we have permission to publish this picture? especially for minors). I was thinking of using an 'approved for publishing' status flag on images, but if the embedded media tag is so heavily cached, I wouldn't be able to show the image in place (with a big red border, perhaps) for content moderators... [Might have to consider using the Paragraphs module instead. But that's an aside.]

The patch in #914382-184: Contextual links incompatible with render cache for Drupal core has relevance here.

We could possibly do something similar. In theory though that would be at the file_entity level not the media level.

The problem is still there. Will a patch be commited soon?

Thx.

There is no patch that properly fixes the problem yet.

Sorry, to be more constructive:

The main problem is that the contextual links are being cached based on whoever views them first.
If that person has access to the contextual links then they will be in the cached markup for the next person, regardless of whether they are supposed to see them.

Because the currently proposed patches are really a temporary workaround until the real cause of the problem is fixed I think #11 is the best solution because it is simple and it changes the way things currently work the least, which are desirable attributes for a workaround.

Personally I think the most desirable real solution is to print the entire file entity always and then people can configure/theme their entities to display as they want. For example if you don't want all the wrapper divs in WYSIWYG then configure/theme a display mode that has no wrappers and use that for WYSIWYG.

Then the problem remains to be role based text filter caching although I have not yet looked into the feasibility of doing that.

So if we're going to commit something to work around this annoying problem in the short term we just need to decide which approach we prefer then propose the maintainer commit that until we have a better solution.

My vote is for #11 (which still applies cleanly to latest dev).

I'm for #11 as well - and am using it in production without complaints - until there's a way to cache the filters by role.

Patch #27 not working. Contextual links are displayed for anonymous and authenticated users.

Patch #11 working good but hide contextual links for all users.

Patch #11 working good but hide contextual links for all users.

That's intentional because it is a temporary work around, not the real fix.
It is still better than the current bug.

Oh ok rooby, you are right, thanks for explain.
What is the interest of the patch #27 (which checks whether the user is anonymous) we can hide this file/patch?

@comexpertise: see my comment at #37 for a critique of the patch at #27 and why it won't work unless there's a deeper change in how core caches filters (see @rooby's comments #42-#44).

The same as patch #11 can be achieved through hook_media_wysiwyg_token_to_markup_alter, fot those who don't want patch their file or want to have some more flexibility on dealing with media_wysiwyg formating.

I've taken the patch in #11 and implemented it as a #pre_render callback instead, so that if a site wants to re-enable contextual link output, it can easily do so by just removing the pre-render callback using media_wysiwyg_token_to_markup(). That way we haven't *removed* the contextual link data completely. Ideally we just disable the filter cache for the media filter and remove this, but I'll leave that for discussion in #2017761: RFC: Disable caching for media filter.

Moved the #pre_render addition to where file_view() is called since this wouldn't be necessary if the legacy file viewing method is used.

Even better, I can just conditionally add this pre_render callback only if $filters['media_filter']['cache'] has not been set to FALSE (which is the default). That way, if we do decide to disable the cache for the media filter, contextual links will automatically work as designed.

Committed #59 (with fixed ending newline) to 7.x-2.x. Thanks everyone for your help in fixing this!