Media Browser Library/My Files does not adhere to image field resolution settings.

Hello kaidjohnson

Your patch works really good! The 'library' tab in the media browser shows - indeed - only the images with the allowed min/max resolutions.
But I noticed another issue:
At filtering those images (with the exposed filters on top of the view, for example; to filter at name) the restrictions are lost again. The filter is correctly applied (when I searched 'flower', an image with title 'flower' is indeed shown) but i can also search on (and select) images which are bigger or smaller than the min/max resolution.
Maybe it has something to do with the fact that it is not applied trough ajax, but i don't know. I searched in the media module, but i could not find anything yet. I hope you or someone else can help me a bit with this?

(I am new here, so maybe this is not the right place to ask this question; please then tell me where to put it.)

Thanks in advance!
Geertje

All,

I applied a fix for _Geertje's issue, but it's quite dirty. Maybe someone haves a better solution for it.
Also I fixed an incorrect join in the patch from comment #6. It was joining the field_usage table so files that weren't used in a node where not loaded in the library.

Jeroen

For testing we've prepared the following:

1. Setup a basic Drupal site;
2. Added Media module;
3. Created image field and selected the Media Browser widget;
4. Uploaded images with the following sizes (20x20, 20x50, 50x80, 80x120, 100x100, 100x150, 200x200)

Then we've tested the following use cases:
Min and max resolutions empty

• We were still able to select all the images.
• Test = OK

Set min resolution to 50x50

• Images with resolution 20x20 and 20x50 where not visible anymore
• Test = OK

Set max resolution to 100x100

• Images with resolution 80x120, 100x150 and 200x200 where not visible anymore
• Test = NOK

It looks like the max resolution check is not working.

Maybe the link can help to continue testing:
Simplytest (valid untill 15/03/16 11:30 +2)

Thanks for the review Nico.
There was a typo in an if statement.
Your issue should be solved in this patch.

The newest patch fixed the issue.
Thnx for the quick reaction.

This is a nice patch! Thank you for submitting this.

Any idea if this will get contributed? It's a small changeset that provides good value.

Could this patch be committed please?
i am still patching media module with each update, for 2 years now.

Kaare what is your opinion on this patch?

I haven't tested this, but considering the life-span of this and @nico.knaepen's review and its follow-up fix, I assume it works as expected. Only a couple nits regarding the actual code:

1. +++ b/media.media.inc
   @@ -62,9 +62,34 @@ function media_query_media_browser_alter($query) {
   +      ->where("CAST(REPLACE(REPLACE(file_metadata_width.value, ';', ''), 'i:', '') AS UNSIGNED) >= :min_width", array(':min_width' => $min_resolution[0]))
   +      ->where("CAST(REPLACE(REPLACE(file_metadata_height.value, ';', ''), 'i:', '') AS UNSIGNED) >= :min_height", array(':min_height' => $min_resolution[1]));
   

   My instant reaction was tthat "why do string manipulation in SQL", but this is the only way to do it during query. And both CAST() and REPLACE() is supported by all major databases so this is good.

2. +++ b/includes/MediaBrowserView.inc
   @@ -45,6 +45,24 @@ class MediaBrowserView extends MediaBrowserPlugin {
   +        $form['min_resolution'] = array(
   

   $form isn't initialized, and further down it might be sent to drupal_render() as such. This should be initialized as empty.

3. +++ b/includes/MediaBrowserView.inc
   @@ -45,6 +45,24 @@ class MediaBrowserView extends MediaBrowserPlugin {
   +      $build['#markup'] = str_replace('</form>', drupal_render($form) . '</form>', $build['#markup']);
   

   Add a test around this, e.g if (!empty($form)).

   I haven't investigated deeply if this is the only way to extend this form. No way to hook into the form using any *form_alter() tricks? The solution seems hacky, but I'm fine with this if no other solution exists.

So overall it's almost good, but I think this should go to the 3.x branch.

ok, moving to the 3.x branch

Recent versions of media have resolved most of peoples concerns and is compatible with entity translation, multilingual and various advanced configurations. Due to the high volume of inactive and most often irrelevant issues we are Closing this as (outdated). If for whatever reason this issue is important to you AND you still have issues after checking the media recipe documentation, then let us know and we will review your concerns.

Otherwise, see the recipe documentation for how to configure media and for troubleshooting tips OR refer to the media_dev distribution if you want to see a working media setup.

As mentioned, feel free to make some noise in this issue if you still feel it is important to you or someone else.

Thanks,

Media team