Hi,

The media-field is defined as having 3 properties: fid, title and data. The default widget is only capable of setting the fid property, which is a reasonable trade-off, as most only needs this.

However, I just created a new widget where I'm able to set title and a description (saved in data) as well as fid. I then went about theming the output so I could write out these two extra properties. This however, proved to be impossible! Here's the reason:

The hook media_field_prepare_view throws away the information!

The hook is given the field-data in the &$items variable, which is looped through to find fids, and then replaced with the loaded media entries! Formatter hooks are thus unable to act upon anything but the media entities referenced by the fields.

I've created a very simple patch that adds the field-property to each loaded media-instance with the original information of the corresponding $item. This way the information is retained and can be accessed by formatters and possibly from the theme. (Perhaps the property should be called something else? instance perhaps?)
The patch is very simple, and there's probably a more clever way of achieving the same. But it does the job.

What's the opinion on this? Shouldn't this information be retained? Otherwise, why does the two extra properties exist on the media field?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rvilar’s picture

It seems that the patch is broken. How do you create it?

http://drupal.org/patch/create

fangel’s picture

I just did a diff media.field.inc.orig media.field.inc.mine > patch. I'll create a "proper" patch according to the official guidelines tomorrow morning now.

But disregarding the patch, what's your opinion on this matter of providing the full field-value to formatters and not just the referenced media?

fangel’s picture

FileSize
551 bytes

Okay, a better attempt at creating a patch..

fangel’s picture

FileSize
561 bytes

Okay, you can't change which files are attached when you edit comments, so I'll have to create a new comment for my third attempt.

EDIT: okay, I can't seem to get it to not say ignored in the status. The patch should still work if you use patch media.fields.inc media-field-information.patch to apply it.

fangel’s picture

Nobody that wants to comment on whether or not it's intended that information is never available to formatters? Regardless of my poor patch-producting skills, this is an issue that should be at the least discussed..

JacobSingh’s picture

Hi fangel,

Apologies for the cruft here. There was once a plan to add the $data attribute to allow overrides of the values attached to media entities. (so you could use a different title than the one on the media entity). This has kinda been scrapped, or at least back-burnered.

If you add fields to a media entity, and configure their formatters, they should just show up. Am I missing the issue?

fangel’s picture

Hi Jacob.
Sorry for the slow reply - I was off snowboarding in Whistler :)

Yes, you are sorta missing the issue. I want the extra fields on the "link" between a Node and a Media entity - not on the Media entity.. E.g. I want to say "when this Image is attached to this Gallery it should show up with the title 'XYZ'"

JacobSingh’s picture

Yeah, we're on the same page... I think this issue is a bit thorny. I'd prefer to implement editing fields inline when choosing media first before we build the interface for overriding them... But do you have a lightweight solution which would work in the interim even w/o a UI? Does it need to be in core media?

fangel’s picture

Well, yes, it requires changes to core media, but the UI doesn't need to be in core media. The reason for this is the media-reference field view prepare method (media_field_prepare_view() in media.fields.php:111-147).
This method is the first one to be called when the reference field gets loaded. It receives the data from the field ( the (fid, title, data)-tuple), and from this returns a list of media-entities. In this process, only the (fid)-part of the tuple is used and the other to parts (title & data) is thrown away. So even if 3rd party media-reference widget is produced that has the UI for setting the entire (fid, title, data)-tuple and not just (fid), these values cannot get extracted by any method and hence cant be used in theming.
So IF media_field_prepare_view() in some way or another retains the title and data-values from the field, and makes these available for later processing, formatting and display the UI can be provided by a 3rd party module. — My patch does this by adding a field member to the media-entity that contains the (fid, title, data)-tuple of the link, which I don't know if is either the right way or the right name for such a member.

As noted in the comments on that function, some sort of resolution to #879034: Modules have no chance to intercept prior to hook_field_prepare_view() is sought. It relates slightly to this, in that it deals with the fact that there is no way to intercept, change or override the processing done in hook_field_prepare_view().
However, even if this issue is fixed, it doesn't auto-fix this issue because media_field_prepare_view() completely overrides the &$items variable with the results from media_load_multiple, so even if you could inject data in prior to the call of the hook, any injected data would be wiped by the hook.

JacobSingh’s picture

#1062304: Fill in the available fields when creating media in the WYSIWYG

Is the issue I was referring to as the one we should do first.

Solving the issue of adding $data to the field and making it useful is a big one... Maybe too big for me to think about and concentrate on right now, but some patches to start the discussion would probably help.

fangel’s picture

I'm not saying you should work on a interface or anything. I'm merely asking for the hook to be adapted to not remove data that is already stored. This would enable further development to begin - even in a outside-of-Media context. But as it is now, with the data being scrubbed, not even outside-of-Media development can try to add this functionality.

My suggestion is to retain the data by adding a property to the returned Media object with the data (that's the 4 line patch I tried to attach). It basically takes the original $items-array and adds as properties to the Media-object $items-array you replace it with.

JacobSingh’s picture

Status: Active » Needs review

@effulgentsia Committed #879556: Clean up and optimize media_field_prepare_view() which was the last commit there.

Would like his comments.

Status: Needs review » Needs work

The last submitted patch, media-field-information.patch, failed testing.

katbailey’s picture

@fangel, just wondering how you went about creating your widget in the first place. I'm trying to do the same thing, i.e. capture at least a title attribute for each media asset in a multivalue media field. However, because the element is not defined as a fieldset but just as a field I don't see how it's possible to capture anything other than the fid. Can you explain roughly how your widget works?

fangel’s picture

@katbailey:
First, I created a new widget, by implementing hook_field_widget_info() and hook_field_widget_form(). In the widget-form hook, I created a form consisting of a fieldset with a text- and a media(/fid)-field inside. This is saved correctly. If you need to add in extra data, I added a #validate function to my form-element that serialized them into the data field.

I then patched Media with the above patch, so the title and data properties is presented to formatters, and then I added a formatter via hook_field_formatter_info() and hook_field_formatter_view() that used the title and data array, as well as the media-object to render correctly.

HOWEVER, seeing as this issue was getting no-where, I've since created my very own field-type (via hook_field_info()), that contains title, fid and description. This way I could do my own hook_field_prepare_view() where I didn't throw the information away and then add a widget and formatter in the same way as described before. The only difference is that this doesn't require patching the Media-module.

Did that help?

barbi’s picture

subscribe

katbailey’s picture

Thanks @fangel! I had been trying something like your first approach yesterday but it wouldn't save the media item, I assumed because it wasn't expecting a fieldset but a field, so I'm obviously missing something there. At any rate, I certainly don't want to be using a dead-end patch so it probably does make sense to try the other approach, it just seems like such a crazy amount of work for the sake of getting titles on media fields :[
Anyway, thanks again for your response.

fangel’s picture

@katbailey: I wholeheartedly agree! It seems excessive to create a new field, just because Media throws away information. However, this is the case, and this issue seems to be moving nowhere.

Acquia who are the main developers of Media, is using it to power Media Gallery (for Drupal Gardens etc), and they've taken a different approach and embeds the Media (now File) entities into the Node-form via Multiform. This might explain why this is going nowhere, as their own modules enables you to put titles on the Media-entities. HOWEVER that approach is only one-title-per-media-entity, as opposed to one-title-per-use which is what this issue is about.

paranojik’s picture

FileSize
1.19 KB

The patch in #4 does not account for the same media being selected in multiple fields on the same node. Attached is a patch that fixes this.

paranojik’s picture

FileSize
821 bytes

@katbailey,
I found a way to achieve media titles which is much simpler than the one @fangel proposed. Code is attached.

fangel’s picture

@paranojik: Great work. Seems like the shift to using the File entity instead of the Media entity finally made this possible without patching stuff.

paranojik’s picture

Status: Needs work » Needs review
FileSize
780 bytes

Rerolled...

slashrsm’s picture

Status: Needs review » Reviewed & tested by the community

Looks fine. We could also achieve the same thing, if we would pass field data to media formatter. It should be possible (without cloning file objects), but it would be uglier IMO.

gsquirrel’s picture

#22: media_field_title_reroll.patch queued for re-testing.

aaron’s picture

#22: media_field_title_reroll.patch queued for re-testing.

ParisLiakos’s picture

Status: Reviewed & tested by the community » Needs work

patch no longer applies, please reroll

Chris Matthews’s picture

Status: Needs work » Closed (outdated)

Closing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team