If you select "Small" for display of an image field, nothing is displayed because that formatter isn't valid:

Undefined index: #formatter i theme_styles_field_formatter() (line 11 of .../sites/all/modules/styles/themes/styles.theme.inc).

I think the problem is the that in the function media_enable in media/media.install, the view_mode_settings for image media_small isn't set to a file style. Unfortunately, the obvious fix

$view_mode_settings['image']['media_small'] = 'styles_file_small';

this doesn't fix it because there is no small default style - see the function file_styles_styles_default_styles in styles/contrib/file_styles/includes/styles/file_styles.styles.inc

These two modules should agree on having small or medium (or both) as defaults.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hansfn’s picture

JacobSingh’s picture

Yes, it's true. Here's the issue though:

It's hard to agree on generic "view modes" for media. An image can be "thumbnail", "web" "print", etc right? An Audio file could be "sample", "low-res", "high-res", etc...

So Preview is used for the browser (and could be used elsewhere), Small and Large are generic and represent a derivation of the original generally. And original is of course, the original.

I wouldn't want to remove "small" from media, because I think it would be used a lot. However, it does present a UX challenge. Should we set "small" to the generic link style for now?

hansfn’s picture

I understand that it's a problem to agree on generic "view modes" for media. My reason for this bug report is just that these two modules should agree on either just small, just medium or both. If not, the UX is terrible.

mattcasey’s picture

Jacob - Speaking theoretically, if you think you can judge what people want for Large, why can't you judge what they want for Small?

I just thought "Small" was broken until I noticed I could change the style formats. IMHO, this is a 'bug,' not a feature, as it requires additional configuration after install before it can be used.

Taxoman’s picture

Should we set "small" to the generic link style for now?

+1 for that

This should perhaps also be described in the readme file (or just place a short comment there with a link back to this discussion here?)

JacobSingh’s picture

I think you can. The reason "small" doesn't have default formatters is because core only ships with a few image styles. In a sense, this is a styles module problem to fix. We don't want the media module declaring new formatters because then we'd have to do it for several file types.

So the fix is probably to hide the view mode or... ?

drew reece’s picture

I'm seeing this too…

How do you manually add a small style to make this work? I added a file style at admin/config/media/file-styles, called it small, but it has no effect, I can't assign an image style preset to this style either. Do we need to hack the module to make 'small' work?

Is there anyway the media module can use the image styles provided by the styles module, it feels like it is duplicating the feature of setting up 'image scaling presets'.

effulgentsia’s picture

I think we should add media_medium, remove media_small, and write an update function to move from the latter to the former.

Image module by default comes with "thumbnail", "medium", and "large". File Styles module, by default, adds "square thumbnail", and then makes file styles for each of these four, plus a 5th one for "original". Media module defines a default view mode "preview", instead of calling it "thumbnail" or "square thumbnail", and I think #2 provides a good answer for why that is. And Media module also adds view modes for "small", "large", and "original". Why "small" here instead of "medium"? I see no reason for this, and believe it might be a historical glitch from perhaps when Image module and Styles module used the word "small" instead of "medium".

Any objections to above?

Is there anyway the media module can use the image styles provided by the styles module

See #1026790: Allow view modes to be dynamically defined.. But regardless of that issue, I think it makes sense for us to fix our default names to map to these other modules.

drew reece’s picture

I'm not sure I can take in what is going on in
#1026790: Allow view modes to be dynamically defined but it sounds like changing to media_medium is good enough if the formatter will work.

+1

JacobSingh’s picture

The reason we have "small" and "large" is that view_modes are per entity type, not bundle. So If you have an audio file, should it be "medium" and "large". Granted, small and large is also a little strange for this case, but at least it makes some sense. Medium and large is like Starbucks coffee sizes :)

Anyway, do whatever you think is clearer, but that was the original motivation; to provide view modes which would be comprehendible across different file types.

effulgentsia’s picture

Medium and large is like Starbucks coffee sizes

I'm not sure "small" and "large" makes any more or less sense than "medium" and "large". Both imply 2 different sizes, where one is bigger than the other. For whatever reason, D7 core developers working on Image module chose "medium" and "large". I don't know why, and don't care enough to find out. I think in this case, we should just be consistent. Though maybe someone wants to grab the drupal.org/project/starbucks namespace and change all the names to "tall" and "venti"? :)

effulgentsia’s picture

[EDIT: double post. ignore this one]

RobW’s picture

To expand a little on effulgentsia in #8: If the object is to work towards media-independent names for multimedia styles, I think we should define guidelines for what certain terms mean (in Media or Styles? I am thinking this is a solely Media issue now that Media module is no longer dependent on Styles). It would be my hope that a well reasoned naming scheme for Media could eventually be adopted by other modules, working towards some sort of Drupal-wide standardization of terms.

My thoughts on image/audio/video are something like:

  • Link - no matter what media type, a link to the original file or source.
  • Preview - a time-truncated preview of media types with duration (for instance a 20 second audio or video clip), and a scaled and cropped version for images
  • Small / Medium / Large - well, these are the subjective ones. Do we base this on image dimensions and video and audio length, or by file size, where an image and video would be scaled, and audio and video would be quality-adjusted (bitrate or compression)? In the interests of semantics I would lean towards the latter, or a combination of the two: Medium for audio might set a 96kbs bitrate and truncate the file if it reaches 1mb. Or this might be a place to introduce parallel terms like "Full", "Partial", and "Compressed" to deal with the file-size/quality versus dimensions/duration ambiguity.
  • Original - present the file as it was uploaded or connected

Drifting a little off topic, but thought I'd post as long as I was thinking about it.

effulgentsia’s picture

Title: Problem with "media_small" formatter » Default configuration of hiding file in "Small" view mode is unintuitive
Component: Code » User interface
Priority: Major » Normal

The initial bug report of a PHP notice when a media field is set to use "Small" was fixed as part of #1086958: Switch from Media Entity to File Entity.

#1026790: Allow view modes to be dynamically defined. is fixed, so with a module like Display Suite, you can create additional view modes, like "Medium" or whatever else you want, and Media module now provides a UI for the media formatter to use that view mode, and the "Image" formatter within "Manage file display" for a given file type lets you route that to any Image style you want.

You can also configure Media's "Small" view mode to make the File visible, and then choose an Image style (or any other file formatter) for it.

So, all that's left in this issue, I think, is the UI problem of a default Media module installation creating a Small view mode and making the file hidden in that view mode. Just now, I added a code comment linking an existing @todo about that to this issue.

Therefore, I'm downgrading this issue to "normal". It would be great to get more UI input on this from people. Should we remove the default Small view mode entirely, since now administrators can make custom view modes? Should we link it to the thumbnail image style? Or to the medium image style? How are Media plugin modules dealing with something other than images implementing Small, and how can we best standardize our terminology for best UX?

#13 is a good start (thanks, RobW). To it, I would add:

  • Should "Link" be a link to the original file *inside* or *outside* the context of a Drupal page (e.g., if the file is a YouTube video, should it be to the media/FID page that embeds the full size video, or should it be to the http://www.youtube.com/... page)? Currently, it's the latter, and because of that, is not the view mode used for listing files on admin/content/media, since those links should be to Drupal pages.
  • "Preview" is what is currently used in admin/content/media/thumbnails, in the media browser, in media/FID/edit, in the media field widget, and other similar places where a preview is needed.
  • "Large" is the current default view mode for media fields and wysiwyg insertion.
  • "Original" is the view mode used on the media/FID page.

As can be seen from above list, "Small" is the only view mode created by Media module, but that has no meaning to Media module itself, which I think is part of why there's confusion about what role it should play.

drew reece’s picture

If display suite is required to make the small format work why does media create it as an option in the first place?
Media creates an option that doesn't work and requires an additional module to configure. It wouldn't be so bad if display suites wasn't so complex.

Here is an outline of what is involved in making 'small' do something with display suite (mostly for my reference).

Install Display Suite.
Go to admin/structure/ds/layout
Find the type you want to manage (eg Image), click 'Manage display'
Click the 'Small' secondary tabs at the top,
Set the file format to visible, save.
Click the manage File Display tab, Select 'Small' in the secondary tabs.
Pick a file style, save.

Now the small formatter can actually do something when a user inserts something with it selected.

The module is very promising, but why do admins need to jump through hoops to make it do what it is supposed to do?
How does any of this tie in with the file styles at admin/config/media/file-styles - it doesn't so you now have one place to specify the 'small' preset (display suite) and another for every other media preset (admin/config/media/file-styles).

RobW’s picture

@drew, I believe you can do something with small without display suite. In the current media dev:

  1. Navigate to Administration->Configuration->Media->File types.
  2. Click on "Manage file Display" for Image.
  3. Under "Enable displays" check "Image".
  4. Under "Display Settings" you should now be able to select any image style, including custom styles.

I haven't checked the stable releases, but they may work the same way. Hope that helps.

drew reece’s picture

@RobW
You are correct, I have the dev versions, & it is how you describe, goody.
I was wrong display suite isn't required to make the small format work. I still find it a little odd that media installs a format in a 'non-working state'.

Thanks.

effulgentsia’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev

I still find it a little odd that media installs a format in a 'non-working state'.

I agree, which is why this issue is still open. The question is, what is a "working state" that makes sense (see earlier comments)? Or, should we remove "small" altogether? Moving this issue to 2.x, but I'm open to backporting a solution to 1.x once we have one.

effulgentsia’s picture

Title: Default configuration of hiding file in "Small" view mode is unintuitive » Revamp file view modes
Priority: Normal » Major
Issue tags: +Media Sprint 2011

Discussed this in the Media sprint. Here's what we came up with:

  • Have File Entity module implement a "Full" view mode: #1291428: Add a 'full' view mode and use it for file/%file pages
  • Have Media module add a "Teaser" view mode.
  • Retain the Media module's "Preview" view mode.
  • Deprecate the Media module's Link, Small, Large, and Original view modes. Retain them for existing sites that use them, but not add them in new sites, and do something in the UI to give the site administrator information about how to move away from the deprecated ones. We don't need Link as a view mode, because that should just be its own file/media field formatter. We don't need Original, because where it makes sense to display the original in the context of a Drupal page, then Full can be used, and where the original shouldn't be displayed in the context of a Drupal page, then the Full view mode can display what's appropriate for in-page-context, and link to the out-of-context original. Finally, we decided to remove Small and Large, because simple use-cases can be covered by Teaser and Full, and for more nuanced use-cases, people can use Display Suite or other modules that provide a UI for administrators to add whatever view modes they want.

Any feedback on this plan?

effulgentsia’s picture

Just to clarify, once #19 is implemented, new sites will just have Preview, Teaser, and Full. Preview is what's used in administrative interfaces (e.g., in the media browser's Library tab, and the preview shown by the media widget when editing the file field of a node). Teaser and Full could be used as needed to customize pages for site visitors, similarly to how Teaser and Full are used for nodes.

effulgentsia’s picture

Title: Revamp file view modes » Revamp file view modes: add Teaser, remove Link, Small, Large, Original
effulgentsia’s picture

Title: Revamp file view modes: add Teaser, remove Link, Small, Large, Original » Revamp file view modes: add Teaser, deprecate Link, Small, Large, Original
drew reece’s picture

#19 seems like a good way forward.
Reducing the number of styles is not a problem since more can be added, 'not broken by default' will be great :)

Site admins can be left to wrestle with the naming of additional styles that get used across media types.

rickvug’s picture

#19 is very sensible. Having a minimal amount of built in view modes means less to configure and understand for beginners and less cruft to disable for advanced users.

aaron’s picture

we just need to preserve the old view modes when updating.

amateescu’s picture

Status: Active » Needs review
FileSize
3.21 KB

Here's a patch per @effulgentsia's suggestions on IRC.

mstrelan’s picture

Subscribe

idflood’s picture

Patch looks good. I've quickly tried to update a previous installation and the deprecated view modes are kept like they should. I've noticed some typos so here is a reroll

RobW’s picture

Sounds great, the move to more semantically meaningful labels divorced from what the actual presentation of those settings on individual sites is a really good thing, IMO. Labels based on context, not on what they may be configured as.

As an aside, is there any change to the recommended way to add more formatters (Display Suite/Styles/Media itself)?

robhamm’s picture

Sorry--I had a stupid question here, and it looks like I answered it myself. Sorry.

effulgentsia’s picture

Thanks amateescu and idflood for the patch, and others for the feedback.

Upon more thinking about this and a conversation in IRC, I think it's best to move Preview and Teaser into File Entity: #1296268: Add Preview and Teaser view modes.

Once that's done, then media_preview can be migrated to preview, media_small can be migrated to teaser, and media_large can be migrated to full. That then only leaves media_link and media_original that need to be retained on existing sites.

Here's a patch that enhances #28 with this. I tested it a bunch, but it could use more. Once it's ready, it should be committed after #1296268: Add Preview and Teaser view modes, but as soon after as possible, since the update function here will override preview/teaser/full settings that someone might have put into file entities if they're running the branch tip of File Entity. I think that's acceptable though, since there's a different expectation of reliability of database updates from branch tip to branch tip. Release to release is what we need to worry about more.

effulgentsia’s picture

As an aside, is there any change to the recommended way to add more formatters (Display Suite/Styles/Media itself)?

My recommendation for adding administrator-defined view modes via the UI is to use Display Suite. You can also do it in code just by implementing hook_entity_info_alter() (which is what Display Suite does). All the formatters (the "Rendered File" formatter for file and image fields and the "Media" formatter for the legacy "Multimedia asset" field) now use formatter settings for connecting to the file's view mode, and they automatically pick up on any view modes added via hook_entity_info_alter().

RobW’s picture

Awesome, thanks for the reply, effulgentsia.

virtualgirl’s picture

Category: bug » support
Priority: Major » Normal

I will go elsewhere if this is a low priority, ...
Is it possible to add a video popup in colorbox in the media module without having to go through the colorbox-load options in the css. Thank you for your consideration Media is a pleasure!

bojanz’s picture

Category: support » bug
Priority: Normal » Major

Please don't hijack issues.

Dave Reid’s picture

pbuyle’s picture

Here is a re-roll of the patch from #31. I seems to do the job, but I didn't test deeply. The best would be to have a Media 1.x install using view modes to migrate.

RobW’s picture

Ah, never mind, wrote too soon.

RobW’s picture

When I apply #37, I get default, link, and original view modes, but no teaser or full.

RobW’s picture

This ended up being my fault, missed some configuration (http://drupal.org/node/1296268#comment-5955546).

Testing now and working well.

mrfelton’s picture

Really looking forward to seeing this get in. File view modes, formatters, render styles etc has been a big source of confusion up until now. The discussion in this ticket really helps to clarify why things are as they currently are, and the changes implemented here and in its counterpart #1296268: Add Preview and Teaser view modes seem like a very good solution that should simplify the setup and bring file view modes more inline with Core's idea of view modes for Node types.

As an asside, you can also use the Entity view modes module to add additional file view modes if you don't want to use Display Suite or custom code implementations.

mrfelton’s picture

Patch updated to apply against latest media code.

mrfelton’s picture

Status: Needs review » Needs work
+++ b/media.installundefined
@@ -709,3 +702,185 @@ function media_update_7201() {
+  // We still have the old media_link and media_original view modes that must be
+  // supported for now.
+  // @see media_entity_info_alter()
+  variable_set('media__show_deprecated_view_modes', TRUE);

Why is media__show_deprecated_view_modes set to TRUE? Why must these depreciated view modes still be supported? The comment in media_entity_info_alter indicates that they only need to be supported from user's coming from Media 1.x, yet we are setting this to TRUE even for users upgrading from previous 2.x versions.

RobW’s picture

+1 for #43. 2.x is still in unstable, so this type of change shouldn't be a big deal. Maybe we could test if there are any formatters set in the deprecated view modes and only hide if they're empty.

RobW’s picture

Status: Needs work » Needs review
FileSize
16.75 KB

Merged changes / re-rolled against latest dev.

Status: Needs review » Needs work
Issue tags: -sprint, -Media Initiative

The last submitted patch, file_view_mode-1051090-45.patch, failed testing.

RobW’s picture

Status: Needs work » Needs review

Passes on my local install. I think test bot is having some issues.

I have the worst luck with test bot.

The last submitted patch, file_view_mode-1051090-45.patch, failed testing.

RobW’s picture

#45: file_view_mode-1051090-45.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +sprint, +Media Initiative

The last submitted patch, file_view_mode-1051090-45.patch, failed testing.

RobW’s picture

Title: Revamp file view modes: add Teaser, deprecate Link, Small, Large, Original » Revamp file view modes: migrate media_small to teaser, media_large to full, media_preview to preview; deprecate link & original

The patch isn't passing D.O. tests right now because of #1774482: Modules with no tests shouldn't actually fail.

It is valid however, applies well and passes local installation tests. Don't let that pink warning scare you away.

RobW’s picture

Semi off topic cross post: I think there's a problem somewhere between this issue and the related file entity issues (#1296268: Add Preview and Teaser view modes, etc). When I try to define default file formatters in a module, either none get through, or only the ones for the original media_XXX view modes do. Cross posting because re-defining the view modes is only useful if the new ones are equally functional. See http://drupal.org/node/1320308#comment-6436464 for more info.

[Edit] The problem was on my end, just a little unfamiliarity with CTools.

RobW’s picture

Status: Needs work » Needs review
FileSize
17.79 KB

Added the removal of file_entity_help() entries for "Manage file display", which should be handled by file entity now.

Jackinloadup’s picture

Fixed 2 whitespace issues in #53. No other changes.

RobW’s picture

Whitespaaaace! (shakes fist in air)

Thanks.

Devin Carlson’s picture

The Media update functions are now up to 7203 so the update function will no longer run.

Rerolled to use media_update_7204.

Devin Carlson’s picture

Component: User interface » Code
Category: bug » task
Status: Needs review » Reviewed & tested by the community

Tested alongside #1296268: Add Preview and Teaser view modes by configuring the file display of a number of file types, adding media which is set to display using the configured displays and then applying the patch in #56.

The patch successfully deprecated the link and original view modes and migrated all of the configuration options for media_small, media_large and media_preview to the new teaser, full and preview view modes. Media which was set to display using the deprecated link and original view modes also continued to work properly.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, media-file-view-mode-1051090-56.patch, failed testing.

Devin Carlson’s picture

Status: Needs work » Reviewed & tested by the community

Moving back to RTBC. This will fail without #1296268: Add Preview and Teaser view modes (both should be committed at the same time).

madar’s picture

I applied #56 against media-7.x-2.0-unstable6 with some offset warning

patching file includes/media.admin.inc
patching file includes/media.filter.inc
patching file includes/media.types.inc
patching file includes/media.variables.inc
Hunk #2 succeeded at 147 (offset 4 lines).
patching file media.install
Hunk #2 succeeded at 703 with fuzz 2 (offset -7 lines).
patching file media.module
Hunk #1 succeeded at 66 (offset -1 lines).
Hunk #2 succeeded at 1046 (offset -31 lines).
patching file modules/mediafield/mediafield.module
patching file tests/media.test

I applied http://drupal.org/node/1296268#comment-6445656 too from #1296268: Add Preview and Teaser view modes.
File preview on edit form working as expected after setting up 'Manage File Display' of image file type.
Also works on media_gallery multiedit page with multiform module.

RobW’s picture

I've been using this alongside #1296268: Add Preview and Teaser view modes for a few weeks now. Very RTBC.

RobW’s picture

Issue tags: -sprint, -Media Initiative

Status: Reviewed & tested by the community » Needs work
Issue tags: +sprint, +Media Initiative

The last submitted patch, media-file-view-mode-1051090-56.patch, failed testing.

ParisLiakos’s picture

needs a reroll for sure, doesnt apply just tried

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
17.27 KB

lets see

Status: Needs review » Needs work

The last submitted patch, 1051090-media_file_view_mode-56.patch, failed testing.

ParisLiakos’s picture

Status: Needs work » Fixed

ok, seems good, commited.
thanks everyone:)

Status: Fixed » Closed (fixed)
Issue tags: -sprint, -Media Initiative

Automatically closed -- issue fixed for 2 weeks with no activity.

sheldonkreger’s picture

Added related issue.